Comments panel redesign

[laurentlb]

This is a followup to the discussion in #139524. Planned API changes means that more information can be surfaced on the UI (in particular, timestamp and thread status). At the same time, we want to reduce visual complexity; it can already be difficult to navigate through the set of comments and find which ones are relevant.

I'd like to explore here designs that remove intermediate replies in a discussion thread. Rationale: reading a full discussion in the comments panel is not really feasible (because comment snippets are truncated and lack styling). If someone wants to read the discussion, they should open it. We need to show enough context so that a user can find the thread they are interested in.

Design 1.

Here, we show only the first and the last messages in each thread. The first message is useful for the context. The last message helps the user know what is the state of the thread, and if there was a message since they last read it.

We could add "..." somewhere to make it clearer that intermediate messages are not shown.

Design 2.

Here, we don't show the first message and we focus on the last update. We give information about hidden messages: how many messages are not shown and who where the authors. This design makes it more obvious that the full thread is not shown.

Other thoughts:

• Being able to hide the resolved threads would be very useful. This would be similar to the filter we already have in the Problems panel (e.g. Show warnings can be unchecked).
• It would also help if we had control on which thread is expanded. The API only lets us expand/collapse a thread in the editor view; maybe we could have the same for the Comments panel? This feature is important IMO if we keep a design that lists all intermediate replies.

(mocks designed by @albertelo)
cc @alexr00 and @isidorn

[alexr00]

@laurentlb thank you for opening this issue, you have read my mind. We have been discussing a design similar to design 2. I think we will leave the twistie/expander off of each comment thread though.

Regarding hiding resolved threads: I want to add a menu contribution point to the comments panel comments/view/title which would allow actions to be contributed to the comment panel in this area:

This would allow extensions to contribute actions can be used to expand/collapse specific comments.

We should also mirror the editor comment expand/collapse state in the panel.

[laurentlb]

This feature already exists. We have been able to add icons there in our extension using:

      "view/title": [
        {
          "command": "critique.publishComments",
          "group": "navigation",
          "when": "view == workbench.panel.comments && config.critique.enableComments"
        },
        // ...
      ],

[alexr00]

One less thing to do then!

[alexr00]

Work in progress update:

Any feedback is welcome!

EDIT: there was bug where the single comment was missing. Updated the image with the fix.

[albertelo]

Thanks! A couple of questions:

• which comment is displayed under the main thread? The middle row suggests it's a second reply from a thread of three comments.
• are threads expandable? There is no chevron, but the first two nodes have a nested comment
• when multiple users have replied to a thread, will their names be displayed?
• To make content easier to parse, would it make sense to make the time stamp and the description smaller? Like how the directory ".github" is displayed?

[alexr00]

which comment is displayed under the main thread? The middle row suggests it's a second reply from a thread of three comments.

It's the most recent comment on the thread.

are threads expandable? There is no chevron, but the first two nodes have a nested comment

Threads are not expandable. There was a bug that caused the third node to be missing it's comment snippet line.

To make content easier to parse, would it make sense to make the time stamp and the description smaller? Like how the directory ".github" is displayed?

Here it is with everything but the usernames in the smaller font:

And here it is with just the timestamps and the comment counts in the smaller font:

Not sure which I prefer.

[isidorn]

I personally do not like using different font sizes one next to the other, it can look ugly for the eye and we do not do this anywhere else in the workbench to my knowledge (I might be wrong).
We had similar challenges in the breakpoints view and we landed on always the same font, but use different colors and shapes to show what is more and less important. So can we use some other decoration and not font size to make it easier for the user to parse this information.

Example

[albertelo]

I like the idea of the timestamp as supplementary information because the nodes can become complex depending on the comment. A smaller font supports rather than compete for attention. Something along the lines of the second image alexr00 posted aligns with what I was imagining, but I agree it looks a bit odd when the nested node has the username and timestamp in a smaller font. Perhaps the timestamp in a smaller font could work. That said, if this treatment is not used anywhere else besides the file nodes (file name is bigger than the directory), I'd side with consistency.

I'm happy to keep discussing this proposal:

Might we worth exploring how it looks with more usernames. Likely we'll need a treatment for collapsing or limiting how many usernames are displayed.

One thing we've discussed within the team is that it's not easy to understand that the comment shown is the last one posted in the thread. In one thread we had discussed using "..." I don't have a great solution in mind, I thought about an icon like this one:

but that feels too abstract. A tooltip could alleviate this a bit, but has discoverability shortcomings.

[miguelsolorio]

Did a few design variations on the panel here to (hopefully) make the threads a little more clear. I think showing the preview gets a bit in the way of just parsing the number of threads and some social platforms will try and summarize the number of comments:

[laurentlb]

I'm afraid it will be difficult to distinguish the threads. It's common for a single person to leave a bunch of comments; if all the lines are similar, it can be confusing.

When I see the last comment, I can more easily know the state of a thread and what I need to do: Is there a question? Is there a suggestion to apply? Did someone add a response since my last comment?

[hermannloose]

I think it's also important to keep in mind the persona of the user: are they the reviewer of the change, or the author of the change? In our particular case, the focus is on change authors, since reviewers are less likely to review changes inside the IDE, relying instead on the code review tool, with test results, static analysis etc.; the author meanwhile is likely to make changes in response to reviewer comments, hence their integration into the IDE.

I am not a heavy GitHub user these days, but I assume for GitHub pull requests it might be similar?

As a change author, a significant function of the comments panel is to be a TODO list. To orient myself, it helps to see the first message of a thread from my reviewer(s) and also the last message from them. The last message on its own might not have enough context by itself to remind me what it was about, since the reviewer writes it assuming the previous messages on the thread as available context. Example: the reviewer saying "Looking better now, but can you inline this above?" is unlikely to bring back my memory of what the thread was originally about.

As I address threads through code changes, I might check things off this list purely through status changes, i.e. the resolved / unresolved state from #127473. Note that the distinction is achieved through icons and color in Design 1 and 2 above, but disappeared in the most recent designs!

In addition to changing the status of a thread, depending on the culture around my specific code review tool, I might also leave a short message like "Done.", "Acknowledged." (e.g. in response to a general mentoring suggestion that didn't trigger any particular code changes) or "Good catch, thanks!". My tool might even force me to enter some content to post a reply changing the thread status.

If we only show the last message on a thread, including if it came from the change author, these won't be helpful for orientation again.

---

As an aside, regarding the point directly above: if CommentAuthorInformation had a boolean field like isChangeAuthor or isCurrentUser that extensions could populate, VS Code could explore various things to yet more clearly communicate status, e.g. highlighting the user's avatar differently ("this is you"), changing messaging ("3 replies" -> "2 replies, last from you") etc.

This could also inform filtering options mentioned by @laurentlb in the original post, e.g. showing only threads where the last reply is not from me.

Let me know if I should file a separate issue for this.

For us specifically, since our code review tool distinguishes between pending replies ("drafts") and published replies, the equation of which replies to surface / style in which way is a bit more complicated still, but I understand that we do not have that distinction at the moment in VS Code and it is somewhat out-of-scope here.

---

Another option to provide context—mostly orthogonal to my points above—is to include a code snippet of the line the comment was left on. CommentThread.range can however span multiple lines, possibly many lines, so that would have to be considered.

[alexr00]

This could also inform filtering options mentioned by @laurentlb in the original post, e.g. showing only threads where the last reply is not from me.

Yes, let's start a new issue for this. It's a good feature request, and we should make sure the UX changes we make here don't prevent us from adding it later, but it should be added separately to facilitate our API process.

[alexr00]

Here's where we're at so far. The arrows will instead be the nice "reply" icon from @misolori's mocks instead of the current icon.

• The comment preview is the original comment in the thread.
• If a comment thread only has one comment then it only gets one line.
• The icon helps distinguish a thread with multiple comments. When we add resolved/unresolved this may no longer be true, but its ok to use different icons in that case.
• We show the original timestamp of the thread and the timestamp of the last reply.
• Smaller font is used for timestamps to match the smaller greyer font in the resource folder.

Once we have the "reply" icon I'll merge this change so we can start getting user feedback. We can still iterate on it as much as we want for the rest of the February 2022 milestone.

[albertelo]

One nit:
The icons used in this proposal feel ideal for the task:

Are they implemented already? I couldn't find them and the latest image uses the straight arrow. If they're not, perhaps design + implementation is needed?

[alexr00]

The arrows will instead be the nice "reply" icon from @misolori's mocks instead of the current icon. The have just not been added yet!

[laurentlb]

In the last screenshot, I think it would be useful to add the username of the last comment, e.g.

4 comments Last reply from albertelo, 1 hour ago

I often want to know if someone else has responded after me.
Apart from that, I think it looks good!

[miguelsolorio]

@alexr00 added the new icon as indent since we already had a reply

[alexr00]

Here's what I merged: