This repository was archived by the owner on Sep 11, 2024. It is now read-only.
Improve performance and stability in sticky headers for new room list #4931
Conversation
This file contains hidden or 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
It should be in all major browsers as of years ago, and we use it unguarded elsewhere in the app. The performance benefits of it appear to be worthwhile enough to keep it, though it's not a perfect solution.
We now use offsets and scroll information to determine where the headers should be stuck to, still supporting the transparent background. Some scroll jumps were originally introduced as part of the change in numbering, so they have been fixed here. By proxy, some additional scroll jump/instability should be fixed as well. This has a lingering problem of still causing a huge number of no-op UI updates though, which will be dealt with in a future commit.
The layout updates are anecdotal based on devtools flagging the values which are "changing" even if they aren't. The scrolling feels better with this as well, though this might be placebo.
turt2live
commented
Jul 8, 2020
| header.style.width = `${headerStickyWidth}px`; | ||
| header.style.top = `${rlRect.top - HEADER_HEIGHT}px`; | ||
| // When an element is <=40% off screen, make it take over | ||
| const offScreenFactor = 0.4; |
There was a problem hiding this comment.
@bwindels this number is fairly arbitrary, but should match the old 60% value with the older math. Might need some tweaking on the refresh.
|
This PR does fix the overscroll fighting at the top of list (element-hq/element-web#14377), but it introduces a similar overscroll fighting issue at the bottom of the list. |
bwindels
approved these changes
Jul 9, 2020
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.
For element-hq/element-web#13635
For element-hq/element-web#14035
Fixes element-hq/element-web#14379
Fixes element-hq/element-web#14371
Fixes element-hq/element-web#14354
Might help with element-hq/element-web#14232
Might help with element-hq/element-web#14253
Might help with element-hq/element-web#14377
Might help with element-hq/element-web#14273
It's best to review this commit-by-commit.