New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix timeline jumps #10001

Merged
merged 3 commits into from Feb 11, 2019

Conversation

Projects
None yet
3 participants
@ThibG
Copy link
Collaborator

ThibG commented Feb 10, 2019

This PR attempts to fix most “timeline jumps”, that is, situations in which the scrolling viewport of a timeline move to different toots without user intervention.

Timeline jumps are caused whenever a toot above the current scrolling position changes height after its first rendering, or gets deleted.

Height changes are currently due to:

  1. MediaGallery, Video player and Card components being rendered in multiple steps, because of computations depending on their width.
  2. Card components being displayed while they weren't at first because the status got updated at a later point, at which point the card has been fetched server-side.
  3. Content warnings being folded and unfolded.

I have chosen not to fix 3. for now, as it is caused by user interaction, and thus is a little less jarring than the other issues.

This PR implements two main fixes:

  • Avoid multiple-step rendering by caching the width of MediaGallery, Video and Card items into the ScrollableList component holding them, since all of those will have the same width. This mostly fixes 1. (the first rendering may cause a small jump if the default value turns out to be wrong due to CSS tweaks for instance).
  • Compensate scroll position whenever we know the height changes (fixes 2., Card appearing, or a toot getting deleted). We want to avoid doing that when not necessary because it may cause the scroll to “stutter” a bit. I think my changes perform this as infrequently as possible.

Those are pretty big changes, so they should be thoroughly tested, which I haven't done yet.

@ThibG ThibG force-pushed the ThibG:fixes/scroll branch from 2019aee to 523f251 Feb 10, 2019

@ykzts

ykzts approved these changes Feb 10, 2019

@ThibG ThibG force-pushed the ThibG:fixes/scroll branch from 523f251 to ec1b5d1 Feb 10, 2019

ThibG added some commits Feb 10, 2019

Avoid two-step rendering of statuses as much as possible
Cache width shared by Video player, MediaGallery and Cards at the
ScrollableList level, pass it down through StatusList and Notifications.

@ThibG ThibG force-pushed the ThibG:fixes/scroll branch from ec1b5d1 to 84f7b6d Feb 10, 2019

@ThibG ThibG removed the work in progress label Feb 11, 2019

@ThibG

This comment has been minimized.

Copy link
Collaborator Author

ThibG commented Feb 11, 2019

Tested for a bit, I haven't seen any regression.

@Gargron Gargron merged commit aee93bf into tootsuite:master Feb 11, 2019

11 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: check-i18n Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-webui Your tests passed on CircleCI!
Details
codeclimate All good!
Details

lucida3rd added a commit to lucida3rd/mastodon that referenced this pull request Feb 12, 2019

10006のコンフリクト対応
tootsuite#10006tootsuite#10016tootsuite#10017tootsuite#10001tootsuite#10008、#10007のcommit。
・及びコンフリクト対応。

Gargron added a commit that referenced this pull request Feb 17, 2019

Fix timeline jumps (#10001)
* Avoid two-step rendering of statuses as much as possible

Cache width shared by Video player, MediaGallery and Cards at the
ScrollableList level, pass it down through StatusList and Notifications.

* Adjust scroll when new preview cards appear

* Adjust scroll when statuses above the current scroll position are deleted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment