From 573799495765c50a64bc69dd74e1911e67d5e059 Mon Sep 17 00:00:00 2001
From: Luke Barnard <lukeb@openmarket.com>
Date: Tue, 4 Apr 2017 13:28:26 +0100
Subject: [PATCH] Clarify and simplfiy unpagination logic

---
 src/components/structures/ScrollPanel.js   | 40 +++++++++-------------
 src/components/structures/TimelinePanel.js |  4 ++-
 2 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js
index 7460d6dac8..cff6a86b2c 100644
--- a/src/components/structures/ScrollPanel.js
+++ b/src/components/structures/ScrollPanel.js
@@ -333,34 +333,28 @@ module.exports = React.createClass({
         if (excessHeight <= 0) {
             return;
         }
-        var itemlist = this.refs.itemlist;
-        var tiles = itemlist.children;
+        const tiles = this.refs.itemlist.children;
 
         // The scroll token of the first/last tile to be unpaginated
         let markerScrollToken = null;
 
-        // Subtract clientHeights to simulate the events being unpaginated whilst counting
-        // the events to be unpaginated.
-        if (backwards) {
-            // Iterate forwards from start of tiles, subtracting event tile height
-            let i = 0;
-            while (i < tiles.length && excessHeight > tiles[i].clientHeight) {
-                excessHeight -= tiles[i].clientHeight;
-                if (tiles[i].dataset.scrollToken) {
-                    markerScrollToken = tiles[i].dataset.scrollToken;
-                }
-                i++;
-            }
-        } else {
-            // Iterate backwards from end of tiles, subtracting event tile height
-            let i = tiles.length - 1;
-            while (i > 0 && excessHeight > tiles[i].clientHeight) {
-                excessHeight -= tiles[i].clientHeight;
-                if (tiles[i].dataset.scrollToken) {
-                    markerScrollToken = tiles[i].dataset.scrollToken;
-                }
-                i--;
+        // Subtract heights of tiles to simulate the tiles being unpaginated until the
+        // excess height is less than the height of the next tile to subtract. This
+        // prevents excessHeight becoming negative, which could lead to future
+        // pagination.
+        //
+        // If backwards is true, we unpaginate (remove) tiles from the back (top).
+        let i = backwards ? 0 : tiles.length - 1;
+        while (
+            (backwards ? i < tiles.length : i > 0) && excessHeight > tiles[i].clientHeight
+        ) {
+            // Subtract height of tile as if it were unpaginated
+            excessHeight -= tiles[i].clientHeight;
+            // The tile may not have a scroll token, so guard it
+            if (tiles[i].dataset.scrollToken) {
+                markerScrollToken = tiles[i].dataset.scrollToken;
             }
+            i += backwards ? 1 : -1;
         }
 
         if (markerScrollToken) {
diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js
index ffeb5b9a1f..fe4a2f46fa 100644
--- a/src/components/structures/TimelinePanel.js
+++ b/src/components/structures/TimelinePanel.js
@@ -251,10 +251,12 @@ var TimelinePanel = React.createClass({
     },
 
     onMessageListUnfillRequest: function(backwards, scrollToken) {
+        // If backwards, unpaginate from the back
         let dir = backwards ? EventTimeline.BACKWARDS : EventTimeline.FORWARDS;
         debuglog("TimelinePanel: unpaginating events in direction", dir);
 
-        // All tiles are inserted by MessagePanel to have a scrollToken === eventId
+        // All tiles are inserted by MessagePanel to have a scrollToken === eventId, and
+        // this particular event should be the first or last to be unpaginated.
         let eventId = scrollToken;
 
         let marker = this.state.events.findIndex(