Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comment UX #269

Open
1 of 4 tasks
schanzer opened this issue Sep 23, 2019 · 9 comments
Open
1 of 4 tasks

Comment UX #269

schanzer opened this issue Sep 23, 2019 · 9 comments
Assignees
Milestone

Comments

@schanzer
Copy link
Member

schanzer commented Sep 23, 2019

  • We should render top-level comments if they are unattached.
  • We should badge nodes which have attached comments (like in Snap!)
  • Right now there is no way to attach a comment to a node. There should be a keystroke and possibly right-click that brings up a comment UI.
  • Relatedly, adding comments at the top level introduces a bunch of problems. In Racket, for example, inserting a line comment and then some text separated by a newline results in the line comment disappearing (since it's a comment, and those aren't visible!) and the text on the new line being rendered as roots. We should sanitize comment input to always produce a single block comment.
@schanzer schanzer self-assigned this Sep 23, 2019
@schanzer schanzer added this to the alpha milestone Oct 20, 2021
@pcardune
Copy link
Collaborator

So I looked at the way Snap does this, and they just have comments always visible, with a button to collapse the comment bubble to a single line:
Screenshot from 2021-11-22 08-45-02

That seems to work fine and we could do something similar. It also makes the UI for adding/editing/deleting a comment really obvious.

However, it breaks down for comments on nested nodes:

Screenshot from 2021-11-22 08-45-13

We could do all kinds of fancy layout magic like they do for graph visualizations, but that would be a lot of effort. Would it be safe to ignore the nested comment overlay problem until a future milestone?

@schanzer
Copy link
Member Author

Can we really not use the existing visual layout for comments, and just allow them to be edited in-place as they are now?

I find Snap's comment layout to be a really mediocre design, and baking the assumption that comments show as top-level blocks into the system feels like something we will likely regret later.

@pcardune
Copy link
Collaborator

I should have been more specific. I was referring specifically to

We should badge nodes which have attached comments (like in Snap!)

The way nodes with attached comments get "badged" in Snap is that the comments are just always visible... there isn't really a "badge" AFAICT.

@schanzer
Copy link
Member Author

I figured that's what you meant. :)

I'm just not a fan of their layout when it comes to comments, but I'm also a pragmatist. I'm not taking a hard line against Snap here -- I'm asking whether our existing layout is really that much work to make editable.

@pcardune
Copy link
Collaborator

It's no problem to make our layout editable. But I'm not sure what the difference is between our layout and Snap's layout other than that our comments are hidden until you focus on the node. Otherwise the layout is exactly the same.

The problem I'm trying to solve is what "badging" is supposed to look like.

@pcardune
Copy link
Collaborator

some other questions about comment behavior:

  • can comments be dragged / dropped from one node to another? If yes, what happens when you drag a comment onto a node that already has a comment: does it append or replace?
  • how does one navigate to/from a comment? Can you use the same keyboard shortcuts for navigating between nodes to access a comment node?

The more I think about it, the more it seems like comments should behave (and be implemented) exactly the same way every other node in the ast is implemented.

@schanzer
Copy link
Member Author

The vast majority of comments are inextricably linked to code. Yes, sometimes there's a giant top-level comment that's more of a note than anything else, but I'd wager at least 95% of comments are about a specific line, expression, or block of code. So my thinking here is focused on those, and not the exception.

I can't imagine comments being dragged and dropped, without their associated nodes being dragged and dropped with them. And if we allowed for this functionality, I feel like we'd wind up getting a ton of blocks that have been moved without their comment coming along for the ride. Treating them just like any node in the tree could lead to bad outcomes.

The way I'm thinking about them, you don't really "navigate to a comment". Instead, you navigate to the expression associated with that comment.

This feels like something we should hash out in our call tomorrow afternoon/evening.

@pcardune
Copy link
Collaborator

My idea isn't to unlink comments from the code, it's just to treat them (and implement them) like a regular ast node, which can appear at the top-level or as a child of another node.

A node with an "attached" comment, is just a node with a comment node in its children. Dragging the node drags the comment and all the other children of that node along with it as per usual. A comment that's not "attached" to a node is just like any other node that doesn't have a parent, and would appear as a top-level node in the editor.

Navigating to a comment node would be the same as navigating to any other node. Drag a comment to the trashcan to delete it, or select it first then delete. Type Enter to edit the comment. If there is no comment to edit, then click on the placeholder to fill one in, or drag a comment from somewhere else to fill it in.

@schanzer
Copy link
Member Author

Ah - so you'd maintain the relationship using a parent-child setup. That makes a TON of sense. Yeah, I'm good with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants