Allow multiple draft comments on any diff line
What does this MR do and why?
Related to #34662 (closed)
When rendering draft comments (the Review feature) in an MR, show any number of draft comments on a single line.
Caveats
Note two things that are unchanged between master
and this branch, but might seem weird:
- Any comment on an unchanged line always displays on the "old" version (the left side)
- You cannot add a second draft comment as a reply to another (non-draft) comment.
You will see both of these in the videos.
For the former, the second visible comment includes "on new (unchanged) line". I left this comment on the right side.
For the latter, I click multiple times on the comment button in the existing (non-draft) comment. Unfortunately, you can't see my mouse pointer, but the button state does change visually.
Both of these quirks are intended and aren't defects, but they kind of seem like they are on first glance.
Screenshots or screen recordings
I have arranged this table so that the fix is first because - due to the nature of this defect - you won't actually know what is missing when looking at the "Before" until you've seen what it's supposed to look like.
After (this branch) | Before (master branch) |
---|---|
duplicate-draft-comments-fix | duplicate-draft-comments-master |
How to set up and validate locally
- While logged in, open an MR
- Write a comment on a line of code and click the "Start a review" button
- Write another comment on the same line of code and click the "Add to review" button.
Outcomes
master |
This branch |
---|---|
The visible comment is replaced with the new comment. The old comment remains in the review, but it's not shown in the diff, and it can't be interacted with (for example to delete or edit it). |
The new draft comment is added below the existing comment(s) |
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.