Multi-line merge request comments: 1. MVC Dropdown basic UI on MR comments
Feature flag: multiline_comments
, default to false.
Rollout plan
These are the MRs required to ship this feature and close this issue:
-
1st MR: Add dropdown with full support for inline view: !29516 (merged) - Includes MR Reviews support.
-
2nd MR: Make dropdowns show correct line numbers for parallel view: !33655 (merged) -
3rd MR: Add Docs
Problem to solve
Commenting on a single line is great for simple kinds of code review feedback, but frequently a comment is actually addressing multiple lines in the diff, perhaps a portion of a logic block, a paragraph of prose or an entire function. This forces users to chose a single line to provide feedback, but the feedback may in fact be resolved with a change to a different line.
Further details
Multi-line comments presents a range of challenges that make it difficult to implement in a single iteration. Particularly the complex and substantial changes to the frontend are the primary area of concern.
Proposal
It should be possible to create a comment on a merge request diff that spans multiple lines. It should be viewable by others, resolvable and possible to reply to, including:
- internal APIs to provide this functionality to the interface
- public APIs (including) may not be updated in this iteration, as multi line comments will can be treated as single line comments targeting the last line in the range (where they will be displayed in the interface)
The primary area of scope reduction for the MVC is in the frontend.
As a reviewer, I will be able to create a multi-line comment by:
- clicking on the last line in multi-line range I wish to comment on
- expanding the range from one line, to many lines through a very basic interface
Concept |
---|
Additionally when viewing a multi-line comment:
- there should a visual method for seeing the range to which the comment applies
- this may be a hover state,
- or for simplicity a text description of the range in the discussion interface
Frontend scope
As discussed in the solution validation issue, for the PoC, given the complexity of the implementation of the desired visual representation of overlapping comments, let's keep it simple:
- Basic UI to increase the range of lines when leaving a comment. (example, not final mockup)
- Basic display on hover and when the comment is focused/clicked.
Availability & Testing
- No risk to availability is expected, but we should check how the feature performs on large diffs with many comments.
- Given the complexity of the existing functionality this feature builds on, substantial new unit and integration coverage will be needed to cover known troublesome edge cases. Particularly interactions with expanding lines. Feature tests may be justified for additional confidence.
- We should also confirm there are no unintended changes to the Web IDE.
- No need for a new E2E test, and the change isn't expected to affect existing E2E tests.