Marked as resolved marking has poor visibility with high contrast color themes

[vahvero]

Problem statement:

Currently, differences between resolved and unresolved icons is relatively poor with high contrast color themes.

compared to default theme

It is difficult to tell conversations apart in high contrast color themes.

Suggestion:

Change the icon of marked as resolved to an unique icon to differentiate the two conversation states.

[alexr00]

@daviddossett do you have any ideas for which icon I could use for resolved comments? I tried the "check" but it doesn't seem quite right.

[hermannloose]

@alexr00 the original mocks from our side in #142081 used the report codicon for unresolved comments and comment for resolved comments, maybe that's an option? Not ideal long-term, if the report icon or comment icon are changed in isolation at some point by someone not aware of their use in the comments panel, but then the only really robust option would be specific icons for resolved and unresolved icons in codicons (or I guess SVGs?).

[daviddossett]

What about something like this? Not yet in codicons but we could add it.

cc @misolori when he's back from vacation.

[alexr00]

@daviddossett it looks like it will fit nicely in with both the comment and discussion icons. Can we add it to codicons?

[hermannloose]

@daviddossett While the icon in isolation looks perfectly fine, I wanted to point out that resolved comments are generally the ones less relevant to the user at a given time, and for workspaces with many resolved discussions, their icon is going to occur more frequently than the one for unresolved comments, so I am a bit worried that the proposed icon for these would be more visually busy than the icon for unresolved comments, which is what the user should focus on?

[daviddossett]

That framing makes sense to me. How about introducing an icon-with-dot pattern as we use that for a "this needs attention" pattern in multiple areas. Resolved comments could default to the regular comment icon.

[alexr00]

I'm happy with that too.

[daviddossett]

Opened a PR to add the icons in the codicons repo. Miguel is on vacation so it may take a bit to land.

[daviddossett]

Ok, added comment-unresolved to codicons 👍

[alexr00]

Thanks for the new icon! Adopted:

[albertelo]

Would it make sense to add the dot treatment to the comment thread icon for unresolved comments in a thread? @daviddossett

[daviddossett]

@albertelo I think that makes sense. I can add the icon if others are also in agreement.

[alexr00]

So long as the icon doesn't look too crowded then it sounds good.

[daviddossett]

Tried this and I don't think it will work quite as well with the current comment-discussion icon. @albertelo do you have any ideas for indicating that a thread has unresolved comments in it?

I wonder if we could instead could consider unread dots that are separated from the icon instead of built-in. Just a thought.

[albertelo]

An idea that comes to mind is changing the position of the message icons:

This way placing the circle is less disruptive:

[hermannloose]

Can we reevaluate whether we actually need two separate icons for singleton comments and comment threads with replies?

The current UI already prominently indicates when a thread has more than one message, with the "[indent] n comments, Last reply from someUser" treatment (which I understand the two icons might slightly predate):

In our experience, most if not all threads will have 2+ replies after the first review round (initial reviewer comments + author reply), meaning we have the busier icon of the two used for most entries in the panel.

I believe it would be completely fine to just use comment and comment-unresolved throughout, what do you think?

[daviddossett]

I was actually thinking about that myself when I saw this and had to think for a second to realize what was going on.

comment-discussion is a fairly busy icon already so I'd be open to such a change.

[alexr00]

👍I'll remove the uses of comment-discussion.