Skip to content

Correct suggestions for multiple comment boxes

Thomas Randolph requested to merge tor/defect/correct-suggestion-line into master

What does this MR do and why?

For #330996 (closed)

This MR fixes the lines used for suggestions (and the multiline header) so that every discussion form has the correct values.

Problem

The previous implementation sets a single global variable that has the current comment range/position. If multiple comment forms are open, the most recent multi-line form will always be the lines used (e.g. for suggestions).

Solution

This MR just moves the local question "what lines is this comment form related to" to be local state.

There are two basic parts:

  1. When the view is interacted with, a single "range" property is updated, and when the form is opened, that range is sent to the form and disconnected from live updates. This keeps each form isolated once it's been opened.
  2. When the user manually changes the start line using the multi-line header (whether it started as multi-line or not), the note form is updated with the new range so that suggestions update appropriately.

Screenshots or screen recordings

There are a number of different scenarios that can appear with this defect, since it's basically just "any discussion after the first will use the most recently opened."

However, here are two common scenarios:

  1. two multi-line discussions are opened
  2. a multi-line discussion is opened, followed by a single-line discussion
Before After
Scenario 1
Scenario 2

Also note in Scenario 2 (before), the multi-line header was incorrectly showing the start line of the above multi-line comment even though the open discussion form was a single-line (comment bubble click) on a later line. This is a symptom of the same problem, but it's notably wrong - and corrected in this MR.

How to set up and validate locally

  1. Create an MR with a file that has at least 5 lines of changes
  2. Create a comment by dragging the comment bubble from line 1 to line 2
  3. Click the insert suggestion button (note that the lines should reflect lines 1 & 2)
  4. Create a comment by dragging the comment bubble from line 3 to line 4
  5. Click the insert suggestion button (note that the lines should reflect lines 3 & 4)
  6. Click the comment bubble on line 5
  7. Click the insert suggestion button (note that the line should reflect line 5)

Without the changes, the following happens:

  1. This still works
  2. This still works, however, on the open comment box created in step 3, the Insert suggestion button now inserts lines 3 & 4
  3. This suggestion text is the lines from the most recently opened discussion form. In this case, lines 3 & 4.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Thomas Randolph

Merge request reports