Page Scroll Cleanup (#2396)

- Reintroduce cancellable scroll top on page change
- IndexPage: rely on browser to retain scroll position on page reload
- Remove obsolete browser hack
- Fix broken selector
- When on mobile, only retain scroll for IndexPage if we're coming from a discussion
- Move app.cache.scrollTop save into `onbeforeremove` so we make sure to do it before DOM is detached

js/src/common/components/Page.js

@@ -22,6 +22,13 @@ export default class Page extends Component {
      * @type {String}
      */
     this.bodyClass = '';
+
+    /**
+     * Whether we should scroll to the top of the page when its rendered.
+     *
+     * @type {Boolean}
+     */
+    this.scrollTopOnCreate = true;
   }
 
   oncreate(vnode) {
@@ -30,6 +37,10 @@ export default class Page extends Component {
     if (this.bodyClass) {
       $('#app').addClass(this.bodyClass);
     }
+
+    if (this.scrollTopOnCreate) {
+      $(window).scrollTop(0);
+    }
   }
 
   onremove() {

js/src/forum/components/IndexPage.js

@@ -42,6 +42,7 @@ export default class IndexPage extends Page {
     app.history.push('index', app.translator.trans('core.forum.header.back_to_index_tooltip'));
 
     this.bodyClass = 'App--index';
+    this.scrollTopOnCreate = false;
   }
 
   view() {
@@ -85,18 +86,22 @@ export default class IndexPage extends Page {
 
     $('#app').css('min-height', $(window).height() + heroHeight);
 
-    // Scroll to the remembered position. We do this after a short delay so that
+    // Let browser handle scrolling on page reload.
-    // it happens after the browser has done its own "back button" scrolling,
+    if (app.previous.type == null) return;
-    // which isn't right. https://github.com/flarum/core/issues/835
+
-    const scroll = () => $(window).scrollTop(scrollTop - oldHeroHeight + heroHeight);
+    // When on mobile, only retain scroll if we're coming from a discussion page.
-    scroll();
+    // Otherwise, we've just changed the filter, so we should go to the top of the page.
-    setTimeout(scroll, 1);
+    if (app.screen() == 'desktop' || app.screen() == 'desktop-hd' || this.lastDiscussion) {
+      $(window).scrollTop(scrollTop - oldHeroHeight + heroHeight);
+    } else {
+      $(window).scrollTop(0);
+    }
 
     // If we've just returned from a discussion page, then the constructor will
     // have set the `lastDiscussion` property. If this is the case, we want to
     // scroll down to that discussion so that it's in view.
     if (this.lastDiscussion) {
-      const $discussion = this.$(`.DiscussionListItem[data-id="${this.lastDiscussion.id()}"]`);
+      const $discussion = this.$(`li[data-id="${this.lastDiscussion.id()}"] .DiscussionListItem`);
 
       if ($discussion.length) {
         const indexTop = $('#header').outerHeight();
@@ -111,14 +116,16 @@ export default class IndexPage extends Page {
     }
   }
 
+  onbeforeremove() {
+    // Save the scroll position so we can restore it when we return to the
+    // discussion list.
+    app.cache.scrollTop = $(window).scrollTop();
+  }
+
   onremove() {
     super.onremove();
 
     $('#app').css('min-height', '');
-
-    // Save the scroll position so we can restore it when we return to the
-    // discussion list.
-    app.cache.scrollTop = $(window).scrollTop();
   }
 
   /**