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
DEV: Move all scroll position reset/remember logic to a shared service #22552
Conversation
Previously we were implementing scroll reset/memorization on a per-page basis. Many of these approaches relied on the `didInsertElement` hook, which is no longer appropriate since Discourse changed to use the 'loading slider' strategy for page transitions. This commit rips out all of our custom scroll resetting/memorizing, and implements those things in a generic service. There are two features: 1. After every route transition, scroll to the top of the page 2. When using browser back/forward buttons, restore the last known scroll position for those routes
95d3485
to
2f115f7
Compare
@@ -6,6 +6,17 @@ let popstateFired = false; | |||
const supportsHistoryState = window.history && "state" in window.history; | |||
const popstateCallbacks = []; | |||
|
|||
function _uuid() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 'uuid' feature is part of upstream's "history-location" routing adapter, which we eventually intend to migrate to.
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, lots of old/hacky stuff gone 💯
app/assets/javascripts/discourse/app/services/route-scroll-manager.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
This pull request has been mentioned on Discourse Meta. There might be relevant details there: |
Using the lastViewedTopicId indiscriminately can cause strange scrolling behavior when navigating to a **different** topic list after viewing a topic. We only want to refocus the topic when going 'back' to the same topic list which originally triggered the navigation.
This reverts commit a3b24bf.
Previously we were implementing scroll reset/memorization on a per-page basis. Many of these approaches relied on the
didInsertElement
hook, which is no longer appropriate since Discourse changed to use the 'loading slider' strategy for page transitions.This commit rips out all of our custom scroll resetting/memorizing, and implements those things in a generic service. There are two features: