Update UI for the comments hint.

Review Request #13672 — Created March 27, 2024 and submitted

Information

Review Board
release-7.x

Reviewers

This change ports the comments hint model and view to TypeScript, and
updates the UI to include the new commit range information. Along with
this, the implementation has been simplified a bit to attach event
handlers individually to each item in the list, rather than having a
generic event handler and setting data on the list items.

  • Ran js-tests.
  • Viewed a diff while having a review that had comments on lots of
    different revisions/interdiffs/commits. Saw that the comment hint
    correctly showed all of the revisions that had draft comments, and
    that clicking on all of them correctly loaded the associated revision.
Summary ID
Update UI for the comments hint.
This change ports the comments hint model and view to TypeScript, and updates the UI to include the new commit range information. Along with this, the implementation has been simplified a bit to attach event handlers individually to each item in the list, rather than having a generic event handler and setting data on the list items. Testing Done: - Ran js-tests. - Viewed a diff while having a review that had comments on lots of different revisions/interdiffs/commits. Saw that the comment hint correctly showed all of the revisions that had draft comments, and that clicking on all of them correctly loaded the associated revision.
6ba2f4f635f172e1b3df75acd7e2eacb94407e4c

Description From Last Updated

Is the babel-plugin-htm dependency update with all the package-lock.json and package.json changes supposed to be part of this change?

maubinmaubin

This one can be an import type.

chipx86chipx86

Unless I missed runtime usage, this can be import type.

chipx86chipx86

We should avoid h1, as that's the main title foe the page. h2 should be the (in theory) the review …

chipx86chipx86

Let's pull out this.model. We access it a lot.

chipx86chipx86
david
chipx86
  1. 
      
  2. This one can be an import type.

  3. Unless I missed runtime usage, this can be import type.

  4. We should avoid h1, as that's the main title foe the page. h2 should be the (in theory) the review request summary. This should probably be h3 or h4.

    1. I agree, but all of our ancient styles make h3 have a crazy margin. I think I'd prefer to basically leave this box stuff as-is until we replace with a modern ink component.

  5. Let's pull out this.model. We access it a lot.

  6. 
      
david
maubin
  1. 
      
  2. Is the babel-plugin-htm dependency update with all the package-lock.json and package.json changes supposed to be part of this change?

  3. 
      
david
maubin
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-7.x (e5a1f25)
Loading...