From bfa3123f9ba7f3aeab19c7205123c4639f7cd1ea Mon Sep 17 00:00:00 2001
From: Luke Barnard <lukeb@openmarket.com>
Date: Thu, 4 May 2017 10:00:13 +0100
Subject: [PATCH] Combine data-scroll-token and -contained-scroll-tokens

 - Instead of using one attribute, use one that might just contain one token
 - Use the first token when tracking a child
 - Mandate that no commas can be in individual tokens
---
 src/components/structures/MessagePanel.js     |  2 +-
 src/components/structures/ScrollPanel.js      | 41 ++++++++-----------
 .../views/elements/MemberEventListSummary.js  |  6 +--
 .../views/rooms/SearchResultTile.js           |  2 +-
 .../components/structures/ScrollPanel-test.js |  2 +-
 5 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js
index 74aec61511..d4bf147ad5 100644
--- a/src/components/structures/MessagePanel.js
+++ b/src/components/structures/MessagePanel.js
@@ -472,7 +472,7 @@ module.exports = React.createClass({
         ret.push(
                 <li key={eventId}
                         ref={this._collectEventNode.bind(this, eventId)}
-                        data-scroll-token={scrollToken}>
+                        data-scroll-tokens={scrollToken}>
                     <EventTile mxEvent={mxEv} continuation={continuation}
                         isRedacted={mxEv.isRedacted()}
                         onWidgetLoad={this._onWidgetLoad}
diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js
index 3f36fac89b..50951312e7 100644
--- a/src/components/structures/ScrollPanel.js
+++ b/src/components/structures/ScrollPanel.js
@@ -46,13 +46,13 @@ if (DEBUG_SCROLL) {
  * It also provides a hook which allows parents to provide more list elements
  * when we get close to the start or end of the list.
  *
- * Each child element should have a 'data-scroll-token'. This token is used to
- * serialise the scroll state, and returned as the 'trackedScrollToken'
- * attribute by getScrollState().
+ * Each child element should have a 'data-scroll-tokens'. This string of
+ * comma-separated tokens may contain a single token or many, where many indicates
+ * that the element contains elements that have scroll tokens themselves. The first
+ * token in 'data-scroll-tokens' is used to serialise the scroll state, and returned
+ * as the 'trackedScrollToken' attribute by getScrollState().
  *
- * Child elements that contain elements that have scroll tokens must declare the
- * contained scroll tokens using 'data-contained-scroll-tokens`. When scrolling
- * to a contained scroll token, the ScrollPanel will scroll to the container.
+ * IMPORTANT: INDIVIDUAL TOKENS WITHIN 'data-scroll-tokens' MUST NOT CONTAIN COMMAS.
  *
  * Some notes about the implementation:
  *
@@ -353,8 +353,8 @@ module.exports = React.createClass({
             // Subtract height of tile as if it were unpaginated
             excessHeight -= tile.clientHeight;
             // The tile may not have a scroll token, so guard it
-            if (tile.dataset.scrollToken) {
-                markerScrollToken = tile.dataset.scrollToken;
+            if (tile.dataset.scrollTokens) {
+                markerScrollToken = tile.dataset.scrollTokens.split(',')[0];
             }
             if (tile.clientHeight > excessHeight) {
                 break;
@@ -423,7 +423,8 @@ module.exports = React.createClass({
      *   scroll. false if we are tracking a particular child.
      *
      * string trackedScrollToken: undefined if stuckAtBottom is true; if it is
-     *   false, the data-scroll-token of the child which we are tracking.
+     *   false, the fist token in data-scroll-tokens of the child which we are
+     *   tracking.
      *
      * number pixelOffset: undefined if stuckAtBottom is true; if it is false,
      *   the number of pixels the bottom of the tracked child is above the
@@ -555,16 +556,10 @@ module.exports = React.createClass({
         var messages = this.refs.itemlist.children;
         for (var i = messages.length-1; i >= 0; --i) {
             var m = messages[i];
-            // 'data-contained-scroll-tokens' has been set, indicating that a child
-            // element contains elements that each have a token. Check this array of
-            // tokens for `scrollToken`.
-            if (m.dataset.containedScrollTokens &&
-                m.dataset.containedScrollTokens.indexOf(scrollToken) !== -1) {
-                node = m;
-                break;
-            }
-            if (!m.dataset.scrollToken) continue;
-            if (m.dataset.scrollToken == scrollToken) {
+            // 'data-scroll-tokens' is a DOMString of comma-separated scroll tokens
+            // There might only be one scroll token
+            if (m.dataset.scrollTokens &&
+                m.dataset.scrollTokens.split(',').indexOf(scrollToken) !== -1) {
                 node = m;
                 break;
             }
@@ -580,7 +575,7 @@ module.exports = React.createClass({
         var boundingRect = node.getBoundingClientRect();
         var scrollDelta = boundingRect.bottom + pixelOffset - wrapperRect.bottom;
 
-        debuglog("ScrollPanel: scrolling to token '" + node.dataset.scrollToken + "'+" +
+        debuglog("ScrollPanel: scrolling to token '" + scrollToken + "'+" +
                  pixelOffset + " (delta: "+scrollDelta+")");
 
         if(scrollDelta != 0) {
@@ -603,12 +598,12 @@ module.exports = React.createClass({
 
         for (var i = messages.length-1; i >= 0; --i) {
             var node = messages[i];
-            if (!node.dataset.scrollToken) continue;
+            if (!node.dataset.scrollTokens) continue;
 
             var boundingRect = node.getBoundingClientRect();
             newScrollState = {
                 stuckAtBottom: false,
-                trackedScrollToken: node.dataset.scrollToken,
+                trackedScrollToken: node.dataset.scrollTokens.split(',')[0],
                 pixelOffset: wrapperRect.bottom - boundingRect.bottom,
             };
             // If the bottom of the panel intersects the ClientRect of node, use this node
@@ -620,7 +615,7 @@ module.exports = React.createClass({
                 break;
             }
         }
-        // This is only false if there were no nodes with `node.dataset.scrollToken` set.
+        // This is only false if there were no nodes with `node.dataset.scrollTokens` set.
         if (newScrollState) {
             this.scrollState = newScrollState;
             debuglog("ScrollPanel: saved scroll state", this.scrollState);
diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js
index a24e19577d..ae8678894d 100644
--- a/src/components/views/elements/MemberEventListSummary.js
+++ b/src/components/views/elements/MemberEventListSummary.js
@@ -369,7 +369,7 @@ module.exports = React.createClass({
 
     render: function() {
         const eventsToRender = this.props.events;
-        const eventIds = eventsToRender.map(e => e.getId());
+        const eventIds = eventsToRender.map(e => e.getId()).join(',');
         const fewEvents = eventsToRender.length < this.props.threshold;
         const expanded = this.state.expanded || fewEvents;
 
@@ -380,7 +380,7 @@ module.exports = React.createClass({
 
         if (fewEvents) {
             return (
-                <div className="mx_MemberEventListSummary" data-contained-scroll-tokens={eventIds}>
+                <div className="mx_MemberEventListSummary" data-scroll-tokens={eventIds}>
                     {expandedEvents}
                 </div>
             );
@@ -438,7 +438,7 @@ module.exports = React.createClass({
         );
 
         return (
-            <div className="mx_MemberEventListSummary" data-contained-scroll-tokens={eventIds}>
+            <div className="mx_MemberEventListSummary" data-scroll-tokens={eventIds}>
                 {toggleButton}
                 {summaryContainer}
                 {expanded ? <div className="mx_MemberEventListSummary_line">&nbsp;</div> : null}
diff --git a/src/components/views/rooms/SearchResultTile.js b/src/components/views/rooms/SearchResultTile.js
index 7fac244481..1aba7c9196 100644
--- a/src/components/views/rooms/SearchResultTile.js
+++ b/src/components/views/rooms/SearchResultTile.js
@@ -60,7 +60,7 @@ module.exports = React.createClass({
             }
         }
         return (
-            <li data-scroll-token={eventId+"+"+j}>
+            <li data-scroll-tokens={eventId+"+"+j}>
                 {ret}
             </li>);
     },
diff --git a/test/components/structures/ScrollPanel-test.js b/test/components/structures/ScrollPanel-test.js
index eacaeb5fb4..7ecb74be6f 100644
--- a/test/components/structures/ScrollPanel-test.js
+++ b/test/components/structures/ScrollPanel-test.js
@@ -115,7 +115,7 @@ var Tester = React.createClass({
         //
         // there is an extra 50 pixels of margin at the bottom.
         return (
-            <li key={key} data-scroll-token={key}>
+            <li key={key} data-scroll-tokens={key}>
                 <div style={{height: '98px', margin: '50px', border: '1px solid black',
                              backgroundColor: '#fff8dc' }}>
                    {key}