Add pop-open to the docked file index and improve scrolling behavior.

Review Request #12942 — Created April 6, 2023 and submitted — Latest diff uploaded

Information

Review Board
release-6.x

Reviewers

This change adds an expansion icon to the right of the file index when
it's docked into the review banner. Clicking this will temporarily
expand the entire index, allowing the user to quickly switch between
files without having to scroll back up to the top of the page. There's a
subtle animation when this is opened, though closing happens
immediately.

This change also dramatically improves the behavior of scrolling when
anchors are clicked (filenames or chunk indicators in the file index) or
keyboard shortcuts are used to navigate. This is kind of tricky because
we have a lot of (literal) moving pieces--the page itself scrolls, and
the docked file index can change height. The height calculation inside
the file index has been exposed, and the page view runs the calculation
twice in order to dial in a good target scroll position.

  • Exercised the expansion icon a fair bit. Verified that the animation
    worked as expected and that the index was closed either when the icon
    was clicked again or when the page was scrolled (including clicking on
    an anchor to scroll to another part of the page).
  • Clicked on file index filenames and chunk indicators to scroll. Saw
    that the page always scrolled such that the target index was displayed
    a bit below the bottom of the banner, even when the banner was
    significantly changing size depending on the new scroll viewport.
  • Went through all the diffviewer keyboard shortcuts and saw that files
    and chunks were scrolled nicely.
  • Ran js-tests.

Diff Revision 2 (Latest)

orig
1
2

Commits

First Last Summary ID Author
Add pop-open to the docked file index and improve scrolling behavior.
This change adds an expansion icon to the right of the file index when it's docked into the review banner. Clicking this will temporarily expand the entire index, allowing the user to quickly switch between files without having to scroll back up to the top of the page. There's a subtle animation when this is opened, though closing happens immediately. This change also dramatically improves the behavior of scrolling when anchors are clicked (filenames or chunk indicators in the file index) or keyboard shortcuts are used to navigate. This is kind of tricky because we have a lot of (literal) moving pieces--the page itself scrolls, and the docked file index can change height. The height calculation inside the file index has been exposed, and the page view runs the calculation twice in order to dial in a good target scroll position. Testing Done: - Exercised the expansion icon a fair bit. Verified that the animation worked as expected and that the index was closed either when the icon was clicked again or when the page was scrolled (including clicking on an anchor to scroll to another part of the page). - Clicked on file index filenames and chunk indicators to scroll. Saw that the page always scrolled such that the target index was displayed a bit below the bottom of the banner, even when the banner was significantly changing size depending on the new scroll viewport. - Went through all the diffviewer keyboard shortcuts and saw that files and chunks were scrolled nicely. - Ran js-tests. Bugs Fixed: 2605, 4059
a853f7d65c13fae2401aec0658bdd07ed177e963 David Trowbridge
reviewboard/static/rb/css/pages/diffviewer.less
reviewboard/static/rb/js/reviews/views/diffFileIndexView.ts
reviewboard/static/rb/js/reviews/views/diffViewerPageView.ts
reviewboard/static/rb/js/reviews/views/reviewablePageView.ts
reviewboard/static/rb/js/reviews/views/unifiedBannerView.ts
Loading...