From d1a5d94916806d4dee2a940ca782f3b5d4f88043 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 22 Nov 2016 16:47:56 +0000 Subject: [PATCH 1/3] Make the unpagination process less aggressive This increases `UNPAGINATION_PADDING` (see the ASCII on ScrollPanel.js, `_getExcessHeight`), and also debounces unfilling requests made for 200ms. This forces unfilling requests not to be sent unless the next 200ms has no scrolling, effectively. --- src/components/structures/ScrollPanel.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 36dbf041e8..f7f954bc0f 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -25,7 +25,7 @@ var DEBUG_SCROLL = false; // The amount of extra scroll distance to allow prior to unfilling. // See _getExcessHeight. -const UNPAGINATION_PADDING = 500; +const UNPAGINATION_PADDING = 1500; if (DEBUG_SCROLL) { // using bind means that we get to keep useful line numbers in the console @@ -361,7 +361,14 @@ module.exports = React.createClass({ } if (markerScrollToken) { - this.props.onUnfillRequest(backwards, markerScrollToken); + // Use a debouncer to prevent multiple unfill calls in quick succession + // This is to make the unfilling process less aggressive + if (this._unfillDebouncer) { + clearTimeout(this._unfillDebouncer); + } + this._unfillDebouncer = setTimeout(() => { + this.props.onUnfillRequest(backwards, markerScrollToken); + }, 200); } }, From 42fc7b1b66f4dbedf0af82213016fb7713f56fa4 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 22 Nov 2016 17:23:06 +0000 Subject: [PATCH 2/3] Use UNFILL_REQUEST_DEBOUNCE_MS constant, reset unfillDebouncer timeout reference. --- src/components/structures/ScrollPanel.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index f7f954bc0f..31ac59c730 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -26,6 +26,9 @@ var DEBUG_SCROLL = false; // The amount of extra scroll distance to allow prior to unfilling. // See _getExcessHeight. const UNPAGINATION_PADDING = 1500; +// The number of milliseconds to debounce calls to onUnfillRequest, to prevent +// many scroll events causing many unfilling requests. +const UNFILL_REQUEST_DEBOUNCE_MS = 200; if (DEBUG_SCROLL) { // using bind means that we get to keep useful line numbers in the console @@ -367,8 +370,9 @@ module.exports = React.createClass({ clearTimeout(this._unfillDebouncer); } this._unfillDebouncer = setTimeout(() => { + this._unfillDebouncer = null; this.props.onUnfillRequest(backwards, markerScrollToken); - }, 200); + }, UNFILL_REQUEST_DEBOUNCE_MS); } }, From 8a6ed1d7e9ff158bf3e8ed720cfebb4176e03220 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 22 Nov 2016 17:43:45 +0000 Subject: [PATCH 3/3] Do not assume unpagination will occur during scroll test --- test/components/structures/TimelinePanel-test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/components/structures/TimelinePanel-test.js b/test/components/structures/TimelinePanel-test.js index 52b9185f18..b2cdfbd590 100644 --- a/test/components/structures/TimelinePanel-test.js +++ b/test/components/structures/TimelinePanel-test.js @@ -341,8 +341,9 @@ describe('TimelinePanel', function() { var events = scryEventTiles(panel); expect(events[0].props.mxEvent).toBe(timeline.getEvents()[0]); - // Expect to be able to paginate forwards, having unpaginated a few events - expect(panel.state.canForwardPaginate).toBe(true); + // At this point, we make no assumption that unpagination has happened. This doesn't + // mean that we shouldn't be able to scroll all the way down to the bottom to see the + // most recent event in the timeline. // scroll all the way to the bottom return scrollDown();