Surfacing interesting inline comments threads

[donokuda]

Opening up this discussion based on some discussion in #1025

The tooltip that's used for previewing inline comments could be improved, so let's have a discussion on how we can improve it and decide on a plan moving forward.

The original purpose for the tooltip was stated as:

a quick way to find comment threads of interest

Given that, what makes a comment thread interesting to a developer? Particularly one who is either looking for feedback or giving feedback in a pull request. Few things that came to my mind:

• A new comment thread was created: If I'm looking for feedback, then any new thread means that I might
• A new comment to an existing thread was created:
  • If I'm a pull request author, then a new comment from someone else is interesting to me because it might influence what changes I plan on doing next.
  • If I'm a reviewer and someone has commented on my thread (whether it's the author or not), I might be interested in the thread because it could be a response that requires a reply from me.
• A thread has a lot of participates
  • Could this indicate that a lot of people have varying opinions on something? If I'm the author, is this a thread that I should be focused on the most?

These were the first couple that came to my head. I'll most likely add more as I think of them. I find it useful to state these down so we can start digging into exactly what type of thread is useful and how we can surface that to our users. One solution that comes to my mind is calling out (maybe with an indicator) of these threads that might useful/relevant to a developer.

One suggestion I had for improving the tooltip (in #1025 (comment)), is trying to summarize the first and last comment while displaying how many comments aren't shown:

+------------------------------+
|   (First comment truncated)  |
| ~~~ 5 comments not shown ~~~ |
|   (Last comment truncated)   |
+------------------------------+

I was suggesting that we show the first comment (and truncate for space) since I assume the pull request description would be the best place to understand why the PR is needed and what is being changed. The last comment is assumes that it would be the best place to understand the next step of the feedback process. Last, showing the amount of comments not shown (or some indication with the amount activity) could indicate that there's might be a lot of opinions in the thread worth reading.

☝️ All that said above, I'm suggesting this approach purely based on assumptions about what developers would need if they were to hover over the 💬 in the margin.

To move forward with designing, I'd like to better understand what developers are looking for when they are an author of a pull request (and getting their code reviewed) or when they are responsible for reviewing code. What are examples of comments in a thread that help you understand what action to take next, and where do they appear (at the end, in the middle, etc?)

[donokuda]

☝️ All that said above, I'm suggesting this approach purely based on assumptions about what developers would need if they were to hover over the 💬 in the margin.

Just to clarify, what I mean by this statement is that I'm also not 100% convinced this is the best solution and that discussing some of the questions I have will help guide me to either feeling more or less confident about that approach (as well as help brainstorm other potential solutions.)

[jcansdale]

@donokuda,

I've captured some hopefully pretty representative threads from the Inline Reviews PR (#1004). I'm using my default window layout and have captured most of the screen to put them in context. Each thread is shown as a tooltip and then a peek window.

1. Here is the longest thread I managed to find. I think threads that don't fit on the screen are a tiny minority. I haven't seen one in the wild and had to come up with a contrived example when testing overflow behavior.

2. This seems to be a more usual length for threads that actually have replies.

3. There are lots of threads (maybe 2/3) that only contain the initial comment. They generally point out a typo or code formatting issue. On github.com I'm tend to respond with a 👍 and not bother with an actually reply.

I'll leave observations for follow up comments.

[jcansdale]

One suggestion I had for improving the tooltip (in #1025 (comment)), is trying to summarize the first and last comment while displaying how many comments aren't shown

Looking at the examples above, I don't think showing the first and last comment would be enough context for (1) but it would kind of work for (2). I'd still prefer to see the whole thread, especially since it doesn't take up that much space.

Looking at the peek view, even these relatively modest length threads are looking cramped. It takes a fair bit of stretching, jiggling and scrolling to read the thread. When I change to (3) the peek window is now taking up too much space.

[donokuda]

@jcansdale Thanks for your insights on comment thread lengths as well as the peek window.

Looking at the peek view, even these relatively modest length threads are looking cramped. It takes a fair bit of stretching, jiggling and scrolling to read the thread. When I change to (3) the peek window is now taking up too much space.

It does feel like to me that we're trying to fix the shortcomings of our peek window approach by using a tooltip. Rather than focusing on the tooltip, what are your thoughts on exploring how we can improve the peek view first before we touch the tooltip? There's a chance that if we fix the issues around the peek window, then there will be no need for the tooltip in the end.

My understanding is that you want to be able to read a comment thread to understand what actions you need to take next as a PR author, but the peek view doesn't have enough space for the content. Is that correct?

[jcansdale]

@donokuda,

My understanding is that you want to be able to read a comment thread to understand what actions you need to take next as a PR author, but the peek view doesn't have enough space for the content. Is that correct?

Often all I want to do is read the comments without changing context. I don't even want to open and close a window. All I want to do is read the comments and get on with whatever I was doing. Sometimes I might know which comment I want to read, but other times I might quickly look through a bunch of comments.

It would certainly be useful if I could pin a comment thread (maybe open it in a tool window). That way when I modify or delete lines near the comment glyph, it won't suddenly disappear (which is what happens now and is a tricky problem to fix). This is however too heavyweight most of the time, when I just want to read and get on with it.

It feels like I'm working in a few different modes:

1. Browsing and reading comment threads
2. Working on code related to a particular comment thread
3. Reviewing code or contributing to a discussion

I've found the tooltip to be an efficient and effective way to do (1).
At the moment (2) is lacking (due to the disappearing comment issue).
The peek window works okay for (3).

[jcansdale]

Rather than focusing on the tooltip, what are your thoughts on exploring how we can improve the peek view first before we touch the tooltip? There's a chance that if we fix the issues around the peek window, then there will be no need for the tooltip in the end.

It's definitely worth looking at ways to improve/alternatives to the peek view. I'm skeptical that I'd want to do without the tooltip, but you never know. 😉

One obvious alternative would be to open the comment thread on the GitHub tool window. This would give us some extra space and it wouldn't interfere with code I'm likely to be working on/changing.

What do you think?

[donokuda]

Often all I want to do is read the comments without changing context. I don't even want to open and close a window. All I want to do is read the comments and get on with whatever I was doing. Sometimes I might know which comment I want to read, but other times I might quickly look through a bunch of comments.

Ohh, okay! Yeah, I agree that the tooltip could be a way to preview a thread quickly.

It feels like I'm working in a few different modes:

1. Browsing and reading comment threads
2. Working on code related to a particular comment thread
3. Reviewing code or contributing to a discussion

These breakdowns of the "modes" you work in were super helpful for me to understand why you would want a quick way (which the tooltip could provide) to look at threads. Thanks for this!

It's definitely worth looking at ways to improve/alternatives to the peek view. I'm skeptical that I'd want to do without the tooltip, but you never know. 😉

One obvious alternative would be to open the comment thread on the GitHub tool window. This would give us some extra space and it wouldn't interfere with code I'm likely to be working on/changing.

What do you think?

I'll try out a few ideas for the tooltip first to make it a little more lightweight (such as removing some decoration like the boxes around the comment) and let's evaluate from there. I feel confident that will help number 1. ✨ The suggestion for opening the thread might help for number 2.

Thanks again for the insights @jcansdale! 🙇

[shana]

I have big concerns over the UI as it is right now. The main purpose of using the peek window is so that the comments can be seen together with the code, and the tooltip goes against this concept entirely.

The example screenshots on the issue body only work because they're in a diff view in side by side mode where a file has been added, so there are no relevant code to be seen on the left side, which is what the tooltip is covering. In any other scenario the tooltip would cover meaningful code and would be detrimental to the experience.

There is no way for the user to know how to go from the tooltip to the peek window - as soon as the tooltip shows up, it becomes the primary source of information, given that it's showing everything and not a summarized view of the thread. This makes it harder to train users in how to interact with inline comments.

There's also all the other problems that were mentioned in the original PR. I've reverted the PR so we can continue iterating on it in #1091 (same branch)

For reference, here are various UX guidelines for Visual Studio and windows apps in general that might come in handy:

https://msdn.microsoft.com/en-us/library/windows/desktop/dn742443(v=vs.85).aspx
https://msdn.microsoft.com/library/windows/desktop/dn742479.aspx
https://msdn.microsoft.com/en-us/library/mt162310.aspx