Feature Request: Review author flow #2095
Conversation
still thinking about how to structure the explanation
There was a problem hiding this comment.
Left some initial thoughts based on the writeup 😄 Looking pretty promising 👍
I'm wondering a little about the complexity of adding a distinct additional tab for pending reviews. I like keeping them all in one place, separate from the existing reviews, but I do worry about increasing the number of navigation steps needed to reach it; GitHub tab, reviews tab, pending review tab. Can we add a "pending" section to a single reviews tab, instead? It would likely feel pretty crowded, so I'm not sure.
| #### Responding to a comment thread | ||
|  | ||
|
|
||
| User can respond to a comment thread by adding a single line comment (current implementation) or starting a new review. The two buttons should only show up when the comment textbox is in focus, or is _not_ empty. |
There was a problem hiding this comment.
👍
We can fiddle with the "in focus or not empty" behavior once we have an implementation to play with. I'm remembering that we had to scrap the "comment editor that expands when focused" because it made the surrounding UI elements jolt around.
It also feels a little awkward to have two rows of buttons right next to one another. Not sure what else we could do there, though 🤔
There was a problem hiding this comment.
Thanks for tackling this @vanessayuenn! It's super clear and thorough ✨
Left a few questions to ponder, all pretty minor
|
|
||
|
|
||
| ### "All Reviews" tab | ||
| This tab shows all review summaries and review comments, including the ones that are part of a _pending review_ that has not been submitted yet. |
There was a problem hiding this comment.
Hmm... since the pending review will be in the "All Reviews" tab, is it necessary to have a separate "Pending Review" tab?
Quite possibly. I haven't thought too deeply about it. Just wanted to throw out the question.
There was a problem hiding this comment.
My major concerns with keeping both published comments and pending comments in the same space are that 1) it would get too crowded, and 2) there would be no obvious hierarchy in the information presented.
I would argue that when a user is in the middle of reviewing a PR (i.e. have a review pending), the most relevant information to them would be their pending comments, as well as the summary textbox that they need to fill out before publishing a review. I couldn't figure out a way to fit all this along with other published comments, while preserving the hierarchy of importance.
|
|
||
| #### Pending comments | ||
|
|
||
|  |
There was a problem hiding this comment.
Oooh interesting. I kinda like this placement, and I like the arrow that indicates it'll take you to somewhere else if you click it.
Minor UI question...
I think our current implementation has the badge on the top of the comment next to the author name. Should we consider keeping it up there? Or do we want to make it more pronounced like it is here?
There was a problem hiding this comment.
Hrmm yea honestly I am 50/50 on this one. I am open to fiddle with it once we are at implementation stage, and then decide which treatment has better UX.
There was a problem hiding this comment.
Yeah - the existing "pending" badge is at the top. But it also has no click action. I think being actionable would justify moving it below though. 🤔
|
|
||
|  | ||
|
|
||
| The summary section of the Pending Review tab is sticky (although still collapsible), so it stays within view regardless of how long the list of comments below it is. The icon on the left indicates the type of review, which can be selected in the dropdown underneath the text box. The button to submit review will be disabled a review type has not been chosen from the dropdown menu. |
There was a problem hiding this comment.
Does this imply we won't have a default? Looks like dotcom has "Comment" as default (at least for me)
There was a problem hiding this comment.
Hrmm.. I can see the value in both having "Comment" as a preseleted default, and making users explicitly choose the type of review they're making. I think I am indifferent? Do you have a preference for one?
There was a problem hiding this comment.
Preselecting it would reduce the interactions required to submit a comment review, but increase the number of users who submit one by accident or without thinking. I'm kind of indifferent too, but I think slightly on the side of not having a default and making the user make a conscious choice... ?
Co-Authored-By: vanessayuenn <6842965+vanessayuenn@users.noreply.github.com>
That's a very valid point. Another thing I just thought of is that while a user can only have one pending review per PR, it is entirely possible for a user to have a pending review for multiple PRs, which would then result in 2 review tabs per PR -- that can get confusing really fast. I started exploring the option of having "sub-tabs" within a review tab 👇. That way we can keep all reviews (pending and published) for a PR in one place, with a simpler navigation model.
|
Codecov Report
@@ Coverage Diff @@
## master #2095 +/- ##
==========================================
+ Coverage 92.55% 92.55% +<.01%
==========================================
Files 207 207
Lines 12021 12016 -5
Branches 1746 1745 -1
==========================================
- Hits 11126 11122 -4
+ Misses 895 894 -1
Continue to review full report at Codecov.
|
|
|
||
| #### Pending comments | ||
|
|
||
|  |
There was a problem hiding this comment.
Yeah - the existing "pending" badge is at the top. But it also has no click action. I think being actionable would justify moving it below though. 🤔
|
|
||
|  | ||
|
|
||
| The summary section of the Pending Review tab is sticky (although still collapsible), so it stays within view regardless of how long the list of comments below it is. The icon on the left indicates the type of review, which can be selected in the dropdown underneath the text box. The button to submit review will be disabled a review type has not been chosen from the dropdown menu. |
There was a problem hiding this comment.
Preselecting it would reduce the interactions required to submit a comment review, but increase the number of users who submit one by accident or without thinking. I'm kind of indifferent too, but I think slightly on the side of not having a default and making the user make a conscious choice... ?
The first draft of the review author workflow proposal is ready! This is nowhere near its final form yet, but hopefully, it will at least get the conversation started.
View rendered docs/feature-requests/006-pull-request-reviewer-flow.md