From 6f6a09d7c452b0ac6a2f86f910605c1bd30c7c78 Mon Sep 17 00:00:00 2001
From: Franz Liedke <franz@develophp.org>
Date: Fri, 31 Jul 2020 22:43:02 +0200
Subject: [PATCH] Start decoupling scrolling from state

---
 js/src/forum/components/DiscussionPage.js |  7 ++++-
 js/src/forum/components/PostStream.js     | 34 +++++++++++++++++------
 js/src/forum/states/PostStreamState.js    | 29 ++-----------------
 3 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/js/src/forum/components/DiscussionPage.js b/js/src/forum/components/DiscussionPage.js
index 4492879e3..a3cfb0074 100644
--- a/js/src/forum/components/DiscussionPage.js
+++ b/js/src/forum/components/DiscussionPage.js
@@ -109,7 +109,12 @@ export default class DiscussionPage extends Page {
                     <ul>{listItems(this.sidebarItems().toArray())}</ul>
                   </nav>
                   <div className="DiscussionPage-stream">
-                    {PostStream.component({ discussion, stream: this.stream, onPositionChange: this.positionChanged.bind(this) })}
+                    {PostStream.component({
+                      discussion,
+                      stream: this.stream,
+                      targetPost: this.stream.targetPost,
+                      onPositionChange: this.positionChanged.bind(this),
+                    })}
                   </div>
                 </div>,
               ]
diff --git a/js/src/forum/components/PostStream.js b/js/src/forum/components/PostStream.js
index 7fca157ea..ec8c61fd4 100644
--- a/js/src/forum/components/PostStream.js
+++ b/js/src/forum/components/PostStream.js
@@ -12,6 +12,7 @@ import Button from '../../common/components/Button';
  *
  * - `discussion`
  * - `stream`
+ * - `targetPost`
  * - `onPositionChange`
  */
 export default class PostStream extends Component {
@@ -102,14 +103,7 @@ export default class PostStream extends Component {
   }
 
   config(isInitialized, context) {
-    this.stream.scrollEffect((type, position, animate) => {
-      if (type === 'number') {
-        this.scrollToNumber(position, animate);
-      } else if (type === 'index') {
-        const backwards = position === this.stream.count() - 1;
-        this.scrollToIndex(position, animate, backwards);
-      }
-    });
+    this.triggerScroll();
 
     if (isInitialized) return;
 
@@ -123,6 +117,30 @@ export default class PostStream extends Component {
     };
   }
 
+  /**
+   * Start scrolling, if appropriate, to a newly-targeted post.
+   */
+  triggerScroll() {
+    if (!this.props.targetPost) return;
+
+    const oldTarget = this.prevTarget;
+    const newTarget = this.props.targetPost;
+
+    if (oldTarget) {
+      if ('number' in oldTarget && oldTarget.number === newTarget.number) return;
+      if ('index' in oldTarget && oldTarget.index === newTarget.index) return;
+    }
+
+    if ('number' in newTarget) {
+      this.scrollToNumber(newTarget.number, this.stream.noAnimationScroll);
+    } else if ('index' in newTarget) {
+      const backwards = newTarget.index === this.stream.count() - 1;
+      this.scrollToIndex(newTarget.index, this.stream.noAnimationScroll, backwards);
+    }
+
+    this.prevTarget = newTarget;
+  }
+
   /**
    * When the window is scrolled, check if either extreme of the post stream is
    * in the viewport, and if so, trigger loading the next/previous page.
diff --git a/js/src/forum/states/PostStreamState.js b/js/src/forum/states/PostStreamState.js
index a226394f4..b01bfd12f 100644
--- a/js/src/forum/states/PostStreamState.js
+++ b/js/src/forum/states/PostStreamState.js
@@ -20,7 +20,6 @@ class PostStreamState {
     this.loadPageTimeouts = {};
     this.pagesLoading = 0;
 
-    this.locationType = null;
     this.index = 0;
     this.number = 1;
 
@@ -92,9 +91,8 @@ class PostStreamState {
 
     this.loadPromise = this.loadNearNumber(number);
 
-    this.needsScroll = true;
+    this.targetPost = { number };
     this.noAnimationScroll = noAnimation;
-    this.locationType = 'number';
     this.number = number;
 
     // In this case, the redraw is only called after the response has been loaded
@@ -117,9 +115,8 @@ class PostStreamState {
 
     this.loadPromise = this.loadNearIndex(index);
 
-    this.needsScroll = true;
+    this.targetPost = { index };
     this.noAnimationScroll = noAnimation;
-    this.locationType = 'index';
     this.index = index;
 
     m.redraw();
@@ -127,28 +124,6 @@ class PostStreamState {
     return this.loadPromise;
   }
 
-  /**
-   * Execute a callback when necessary because the scroll position changed.
-   *
-   * The callback will be called with three arguments:
-   *  - the "type" of position to jump to ("number" or "index")
-   *  - the corresponding position
-   *  - whether scrolling should be animated
-   *
-   * @param callback
-   */
-  scrollEffect(callback) {
-    if (!this.needsScroll) return;
-
-    if (this.locationType === 'number') {
-      callback('number', this.number, !this.noAnimationScroll);
-    } else {
-      callback('index', this.sanitizeIndex(this.index), !this.noAnimationScroll);
-    }
-
-    this.needsScroll = false;
-  }
-
   /**
    * Clear the stream and load posts near a certain number. Returns a promise.
    * If the post with the given number is already loaded, the promise will be