Improve scroll positioning when elements change on a page.
Review Request #9142 — Created Aug. 17, 2017 and submitted
Review Board has a lot of bits of UI that dynamically grow or shrink as
the page is loading. Diff fragments, for instance, dynamically load
after the page has otherwise loaded, which results in a page jump
(sometimes a significant one) when browsing through the page or after
having navigated to the anchor for a comment or review. There are also
elements further up the page that react as changes are made further down
(like the issue summary table when resolving/reopening an issue).Some of this has been worked around in the past (the issue summary table
specifically), but the problem was pretty apparent in some browsers when
working with anchors on the review request page.This change introduces
RB.ScrollManagerView
(accessible via
RB.scrollManager
), which aims to fix the jumpiness as parts of the UI
update. When a view is replacing/showing/hiding a bit of UI, it can now
callRB.scrollManager.markForUpdate
, passing in the element. After the
update, it can callRB.scrollManager.markUpdated
. This tells the
scroll manager to schedule a scroll position fix, taking into account
the changes made to that element (and any others updated before the
scheduled position fix takes place). The scroll position will be
adjusted to compensate for the new heights/offsets in the updated
elements, keeping the current view of the page stable.It also handles provides functions for navigating to an element or
position on the page, wrappingwindow.scrollTo()
. This will take into
account an offset that views can adjust. This is intended for things
like the review draft banner, which would normally overlap the top of
the content being navigated to. The review request page has been updated
to use this whenever navigating to anchors.Over time, this will be extended to offer additional functionality
related to the scrollbar and positioning on the page.
Tested this extensively on the review request page in multiple browsers.
Verified that anchor navigation worked consistently and took the floating
review draft banner into consideration.Tested navigating to a page with an anchor and having the diffs load.
Verified that the page did not jump.Tested that changing issue statuses didn't result in page jumps.
Unit tests pass.
Description | From | Last Updated |
---|---|---|
Col: 54 Expected an assignment or function call and instead saw an expression. |
reviewbot |
- Change Summary:
-
Fixed a comma that should have been a semicolon (jshint complaint, but didn't impact the runtime of the code).
- Commit:
-
755ceafd3fec73546e59052b844034d30cc845b8654159619064a66d89a47212142079f863bfb313
- Diff:
-
Revision 2 (+559 -82)
Checks run (2 succeeded)
- Change Summary:
-
- Added assertions to make sure there's always exactly one element provided for the mark functions.
- Fixed a missing line of code for hiding the review draft banner that got deleted.
- Fixed a unit test for the diff viewer page due to the review banner's element not being part of the template (which is now a requirement).
- Commit:
-
654159619064a66d89a47212142079f863bfb313889d176de98f756bb1a1138ffa308fd42f046556
- Diff:
-
Revision 3 (+566 -82)