Summary

Update UI for the comments hint.

Review Request #13672 — Created March 27, 2024 and submitted April 12, 2024, 12:39 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

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.

Commits

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

Summary

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

Files

Issues

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?	maubin	April 4, 2024, 10:33 p.m.
This one can be an import type.	chipx86	April 2, 2024, 10:51 a.m.
Unless I missed runtime usage, this can be import type.	chipx86	April 2, 2024, 10:51 a.m.
We should avoid h1, as that's the main title foe the page. h2 should be the (in theory) the review …	chipx86	April 2, 2024, 11:08 a.m.
Let's pull out this.model. We access it a lot.	chipx86	April 2, 2024, 11:08 a.m.

flake8 passed.

JSHint passed.

Change Summary:

Fix a typo.

Commits:

	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.	31d6eb8052f5f315f8ca3ae9cf3716fa9be0c816
	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.	97ddab26f7bf8ac3bdff861403d0de94c70e9849

Diff:

Revision 2 (+644 -366)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.es6.js
	reviewboard/static/rb/js/diffviewer/views/diffCommentsHintView.es6.js
	reviewboard/static/rb/js/reviews/index.ts
	reviewboard/static/rb/js/reviews/models/diffCommentsHintModel.ts
	reviewboard/static/rb/js/reviews/models/diffViewerPageModel.ts
	reviewboard/static/rb/js/reviews/models/tests/diffViewerPageModelTests.ts
	reviewboard/static/rb/js/reviews/views/diffCommentsHintView.ts
	1 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/static/rb/js/reviews/models/diffViewerPageModel.ts (Diff revision 2)
The issue has been resolved. Show all issues
```
This one can be an import type.
```
reviewboard/static/rb/js/reviews/views/diffCommentsHintView.ts (Diff revision 2)
The issue has been resolved. Show all issues
```
Unless I missed runtime usage, this can be import type.
```

reviewboard/static/rb/js/reviews/views/diffCommentsHintView.ts (Diff revision 2)

The issue has been dropped. Show all issues

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.

david April 2, 2024, 11:09 a.m.

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.

reviewboard/static/rb/js/reviews/views/diffCommentsHintView.ts (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's pull out this.model. We access it a lot.
```

Commits:

	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.	97ddab26f7bf8ac3bdff861403d0de94c70e9849
	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.	5ce1f7c84b281fe76b69d62b2a1518309b1206ea

Diff:

Revision 3 (+704 -28526)

Show changes

	package-lock.json
	reviewboard/dependencies.py
	reviewboard/package.json
	reviewboard/staticbundles.py
	reviewboard/reviews/views/review_request_detail.py
	reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.es6.js
	reviewboard/static/rb/js/diffviewer/views/diffCommentsHintView.es6.js
	reviewboard/static/rb/js/reviews/index.ts
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

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

david April 4, 2024, 10:33 p.m.
```
It is not.
```

Commits:

	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.	5ce1f7c84b281fe76b69d62b2a1518309b1206ea
	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

Diff:

Revision 4 (+656 -366)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.es6.js
	reviewboard/static/rb/js/diffviewer/views/diffCommentsHintView.es6.js
	reviewboard/static/rb/js/reviews/index.ts
	reviewboard/static/rb/js/reviews/models/diffCommentsHintModel.ts
	reviewboard/static/rb/js/reviews/models/diffViewerPageModel.ts
	reviewboard/static/rb/js/reviews/models/tests/diffViewerPageModelTests.ts
	reviewboard/static/rb/js/reviews/views/diffCommentsHintView.ts
	1 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.x (e5a1f25)