From 1d76c5ef5d35d7a5d87b0bc07f4b271de6a7d4a7 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 10 May 2022 19:18:55 +0300 Subject: [PATCH] FIX: Store scroll position when using Back button (#16658) For some pages, when navigating to a topic and then pressing the Back browser button to go back to the topic list, the scroll position was not preserved and the user was taken to the beginning of the list. This happened because the application failed to detect when the user used the Back button and whether the topic list should be fetch from the cache or not. The scroll position is preserved only for cached topic lists. Other improvements: * Improve isPoppedState * Reset position for topic-lists from user-activity page * Remove usage of jQuery * Make sure the scrollTo function has effect Follow up to 618a1ba5711a67603e5783a528fd5d468ed42d7e. --- .../discourse/app/components/bookmark-list.js | 6 +++--- .../discourse/app/components/discovery-topics-list.js | 10 +++++++--- .../javascripts/discourse/app/components/topic-list.js | 6 +++--- .../discourse/app/components/user-stream.js | 6 +++--- .../app/controllers/user-activity-bookmarks.js | 5 +++-- .../javascripts/discourse/app/routes/discourse.js | 5 ++++- .../discourse/app/routes/user-activity-bookmarks.js | 6 +++++- .../discourse/app/routes/user-activity-read.js | 6 ++++++ .../discourse/app/routes/user-activity-topics.js | 6 ++++++ 9 files changed, 40 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/bookmark-list.js b/app/assets/javascripts/discourse/app/components/bookmark-list.js index 17e07f858e4..f96c4aa94a5 100644 --- a/app/assets/javascripts/discourse/app/components/bookmark-list.js +++ b/app/assets/javascripts/discourse/app/components/bookmark-list.js @@ -1,6 +1,6 @@ import Component from "@ember/component"; import { action } from "@ember/object"; -import { schedule } from "@ember/runloop"; +import { next, schedule } from "@ember/runloop"; import bootbox from "bootbox"; import { openBookmarkModal } from "discourse/controllers/bookmark"; import { ajax } from "discourse/lib/ajax"; @@ -28,10 +28,10 @@ export default Component.extend(Scrolling, { scrollToLastPosition() { const scrollTo = this.session.bookmarkListScrollPosition; - if (scrollTo > 0) { + if (scrollTo >= 0) { schedule("afterRender", () => { if (this.element && !this.isDestroying && !this.isDestroyed) { - window.scrollTo(0, scrollTo + 1); + next(() => window.scrollTo(0, scrollTo + 1)); } }); } diff --git a/app/assets/javascripts/discourse/app/components/discovery-topics-list.js b/app/assets/javascripts/discourse/app/components/discovery-topics-list.js index 4683d8e8c18..2b304caa1d9 100644 --- a/app/assets/javascripts/discourse/app/components/discovery-topics-list.js +++ b/app/assets/javascripts/discourse/app/components/discovery-topics-list.js @@ -1,5 +1,5 @@ import { observes, on } from "discourse-common/utils/decorators"; -import { schedule, scheduleOnce } from "@ember/runloop"; +import { next, schedule, scheduleOnce } from "@ember/runloop"; import Component from "@ember/component"; import LoadMore from "discourse/mixins/load-more"; import UrlRefresh from "discourse/mixins/url-refresh"; @@ -14,8 +14,12 @@ export default Component.extend(UrlRefresh, LoadMore, { @observes("model") _readjustScrollPosition() { const scrollTo = this.session.topicListScrollPosition; - if (scrollTo > 0) { - schedule("afterRender", () => $(window).scrollTop(scrollTo + 1)); + if (scrollTo >= 0) { + schedule("afterRender", () => { + if (this.element && !this.isDestroying && !this.isDestroyed) { + next(() => window.scrollTo(0, scrollTo + 1)); + } + }); } else { scheduleOnce("afterRender", this, this.loadMoreUnlessFull); } diff --git a/app/assets/javascripts/discourse/app/components/topic-list.js b/app/assets/javascripts/discourse/app/components/topic-list.js index d542aa6d535..360425fae55 100644 --- a/app/assets/javascripts/discourse/app/components/topic-list.js +++ b/app/assets/javascripts/discourse/app/components/topic-list.js @@ -3,7 +3,7 @@ import discourseComputed, { observes } from "discourse-common/utils/decorators"; import Component from "@ember/component"; import LoadMore from "discourse/mixins/load-more"; import { on } from "@ember/object/evented"; -import { schedule } from "@ember/runloop"; +import { next, schedule } from "@ember/runloop"; import showModal from "discourse/lib/show-modal"; export default Component.extend(LoadMore, { @@ -75,10 +75,10 @@ export default Component.extend(LoadMore, { } const scrollTo = this.session.topicListScrollPosition; - if (scrollTo > 0) { + if (scrollTo >= 0) { schedule("afterRender", () => { if (this.element && !this.isDestroying && !this.isDestroyed) { - $(window).scrollTop(scrollTo + 1); + next(() => window.scrollTo(0, scrollTo + 1)); } }); } diff --git a/app/assets/javascripts/discourse/app/components/user-stream.js b/app/assets/javascripts/discourse/app/components/user-stream.js index 63273f7224c..6262ba75fe6 100644 --- a/app/assets/javascripts/discourse/app/components/user-stream.js +++ b/app/assets/javascripts/discourse/app/components/user-stream.js @@ -11,7 +11,7 @@ import { getOwner } from "discourse-common/lib/get-owner"; import { observes } from "discourse-common/utils/decorators"; import { on } from "@ember/object/evented"; import { popupAjaxError } from "discourse/lib/ajax-error"; -import { schedule } from "@ember/runloop"; +import { next, schedule } from "@ember/runloop"; export default Component.extend(LoadMore, { tagName: "ul", @@ -74,10 +74,10 @@ export default Component.extend(LoadMore, { _scrollToLastPosition() { const scrollTo = this.session.userStreamScrollPosition; - if (scrollTo > 0) { + if (scrollTo >= 0) { schedule("afterRender", () => { if (this.element && !this.isDestroying && !this.isDestroyed) { - window.scrollTo(0, scrollTo + 1); + next(() => window.scrollTo(0, scrollTo + 1)); } }); } diff --git a/app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js b/app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js index 4afe2aec1a5..aa33a867abe 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js +++ b/app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js @@ -67,7 +67,7 @@ export default Controller.extend({ this.set("loadingMore", true); return this._loadMoreBookmarks(this.q) - .then((response) => this._processLoadResponse(response)) + .then((response) => this._processLoadResponse(this.q, response)) .catch(() => this._bookmarksListDenied()) .finally(() => this.set("loadingMore", false)); }, @@ -91,12 +91,13 @@ export default Controller.extend({ this.set("permissionDenied", true); }, - _processLoadResponse(response) { + _processLoadResponse(searchTerm, response) { if (!response || !response.user_bookmark_list) { return; } response = response.user_bookmark_list; + this.model.searchTerm = searchTerm; this.model.loadMoreUrl = response.more_bookmarks_url; if (response.bookmarks) { diff --git a/app/assets/javascripts/discourse/app/routes/discourse.js b/app/assets/javascripts/discourse/app/routes/discourse.js index 9890e5c2ca9..fc9792da024 100644 --- a/app/assets/javascripts/discourse/app/routes/discourse.js +++ b/app/assets/javascripts/discourse/app/routes/discourse.js @@ -92,7 +92,10 @@ const DiscourseRoute = Route.extend({ }, isPoppedState(transition) { - return !transition._discourse_intercepted && !!transition.intent.url; + return ( + !transition._discourse_intercepted && + (!!transition.intent.url || !!transition.queryParamsOnly) + ); }, }); diff --git a/app/assets/javascripts/discourse/app/routes/user-activity-bookmarks.js b/app/assets/javascripts/discourse/app/routes/user-activity-bookmarks.js index 2fb053a9ca1..386b1054663 100644 --- a/app/assets/javascripts/discourse/app/routes/user-activity-bookmarks.js +++ b/app/assets/javascripts/discourse/app/routes/user-activity-bookmarks.js @@ -12,7 +12,11 @@ export default DiscourseRoute.extend({ model(params, transition) { const controller = this.controllerFor("user-activity-bookmarks"); - if (this.isPoppedState(transition) && this.session.bookmarksModel) { + if ( + this.isPoppedState(transition) && + this.session.bookmarksModel && + this.session.bookmarksModel.searchTerm === params.q + ) { return Promise.resolve(this.session.bookmarksModel); } diff --git a/app/assets/javascripts/discourse/app/routes/user-activity-read.js b/app/assets/javascripts/discourse/app/routes/user-activity-read.js index a663745c704..090b7aaf519 100644 --- a/app/assets/javascripts/discourse/app/routes/user-activity-read.js +++ b/app/assets/javascripts/discourse/app/routes/user-activity-read.js @@ -23,6 +23,12 @@ export default UserTopicListRoute.extend({ }); }, + afterModel(model, transition) { + if (!this.isPoppedState(transition)) { + this.session.set("topicListScrollPosition", null); + } + }, + emptyState() { const title = I18n.t("user_activity.no_read_topics_title"); const body = I18n.t("user_activity.no_read_topics_body", { diff --git a/app/assets/javascripts/discourse/app/routes/user-activity-topics.js b/app/assets/javascripts/discourse/app/routes/user-activity-topics.js index bb8ee51b784..c6b0bb90bfe 100644 --- a/app/assets/javascripts/discourse/app/routes/user-activity-topics.js +++ b/app/assets/javascripts/discourse/app/routes/user-activity-topics.js @@ -22,6 +22,12 @@ export default UserTopicListRoute.extend({ }); }, + afterModel(model, transition) { + if (!this.isPoppedState(transition)) { + this.session.set("topicListScrollPosition", null); + } + }, + emptyState() { const user = this.modelFor("user"); const title = this.isCurrentUser(user)