Merge pull request #855 from matrix-org/luke/fix-jump-to-rm-in-mels

Fix jumping to an unread event when in MELS

src/components/structures/MessagePanel.js

@@ -354,7 +354,6 @@ module.exports = React.createClass({
                     <MemberEventListSummary
                         key={key}
                         events={summarisedEvents}
-                        data-scroll-token={eventId}
                         onToggle={this._onWidgetLoad} // Update scroll state
                     >
                             {eventTiles}
@@ -473,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}

src/components/structures/ScrollPanel.js

@@ -46,9 +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
+ * Each child element should have a 'data-scroll-tokens'. This string of
- * serialise the scroll state, and returned as the 'trackedScrollToken'
+ * comma-separated tokens may contain a single token or many, where many indicates
- * attribute by getScrollState().
+ * 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().
+ *
+ * IMPORTANT: INDIVIDUAL TOKENS WITHIN 'data-scroll-tokens' MUST NOT CONTAIN COMMAS.
  *
  * Some notes about the implementation:
  *
@@ -349,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) {
+            if (tile.dataset.scrollTokens) {
-                markerScrollToken = tile.dataset.scrollToken;
+                markerScrollToken = tile.dataset.scrollTokens.split(',')[0];
             }
             if (tile.clientHeight > excessHeight) {
                 break;
@@ -419,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 first 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
@@ -551,8 +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];
-            if (!m.dataset.scrollToken) continue;
+            // 'data-scroll-tokens' is a DOMString of comma-separated scroll tokens
-            if (m.dataset.scrollToken == scrollToken) {
+            // There might only be one scroll token
+            if (m.dataset.scrollTokens &&
+                m.dataset.scrollTokens.split(',').indexOf(scrollToken) !== -1) {
                 node = m;
                 break;
             }
@@ -568,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) {
@@ -591,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
@@ -608,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);

src/components/views/elements/MemberEventListSummary.js

@@ -369,6 +369,7 @@ module.exports = React.createClass({
 
     render: function() {
         const eventsToRender = this.props.events;
+        const eventIds = eventsToRender.map(e => e.getId()).join(',');
         const fewEvents = eventsToRender.length < this.props.threshold;
         const expanded = this.state.expanded || fewEvents;
 
@@ -379,7 +380,7 @@ module.exports = React.createClass({
 
         if (fewEvents) {
             return (
-                <div className="mx_MemberEventListSummary">
+                <div className="mx_MemberEventListSummary" data-scroll-tokens={eventIds}>
                     {expandedEvents}
                 </div>
             );
@@ -437,7 +438,7 @@ module.exports = React.createClass({
         );
 
         return (
-            <div className="mx_MemberEventListSummary">
+            <div className="mx_MemberEventListSummary" data-scroll-tokens={eventIds}>
                 {toggleButton}
                 {summaryContainer}
                 {expanded ? <div className="mx_MemberEventListSummary_line">&nbsp;</div> : null}

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>);
     },

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}