This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
BACAT Scrolling #2842
Merged
BACAT Scrolling #2842
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
as it's used in PersistentElement which could be used at various places
…ur restore position logic. Disable it like this.
as it's used in PersistentElement which could be used at various places
…ur restore position logic. Disable it like this.
…x-org/matrix-react-sdk into bwindels/timeline-nativescrollbars
these variables are now relative to bottom of timeline, before it was the top
it's only called from one place
instead of setting a min-height on the whole timeline, track how much height we need to add to prevent shrinking and set paddingBottom on the container element of the timeline.
see if that avoids jumps
as BACAT scrolling relies on CSS from riot, which is not included in the karma tests, we're removing these tests in favor of later adding them to the e2e tests.
dbkr
approved these changes
Mar 29, 2019
|
Thanks for the quick review Dave! Merging now. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related ticket: element-hq/element-web#8565
This implements a new way of restoring scroll position, dubbed BACAT (Bottom-Aligned, Clipped-At-Top) scrolling.
The motivation for the changes is having noticed that setting scrollTop while scrolling tends not work well on at least Chrome on macOs. Also see element-hq/element-web#528. The approach taken instead manually sets the height of the timeline in 200px increments, with the timeline being aligned to the bottom of the container element. The relatively tiny 200px height comes from the fact that we don't want the user to ever scroll off into pages of whitespace and we have no way of preventing overscroll, so by keeping it to 200px and putting a spinner 100px up from the top of the page, we avoid death by whitespace.
By changing the height of the container, we can compensate for anything that grew below the viewport to maintain what's currently visible in the viewport without jumping. For anything above the viewport growing or shrinking, we don't need to do anything as the timeline is bottom-aligned. We do need to update the height manually to keep all content visible. To maintain scroll position after the portion above the viewport changes height, we need to set the scrollTop, as we cannot balance it out with more height changes. We do this 100ms after the user has stopped scrolling, so setting scrollTop has not nasty side-effects.
As part of this PR, the timeline now also uses native scrollbars instead of gemini. Gemini gave us a way to observe the size of the scrollable area, which I replaced with the
ResizeNotifier, which determines that e.g. if the left panel sizes was changed, the timeline must have also changed size. It it also notified if the window resizes. This is passed everywhere where it's needed by means of aresizeNotifierprop.There are a lot of commits in this PR, if it's really hard to review, I'm happy to squash the commits to make it a bit easier.
Some things remain to be done in subsequent PRs: