From 2f435f483628b7887fe9970b092087b0c3c5c475 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 21 Apr 2016 13:37:31 +0100 Subject: [PATCH] Fix bug which stopped us scrolling down after we scrolled up Make sure that, if we scroll up enough to move the timelinewindow away from the end of the timeline, we reset the canForwardPaginate flag. --- src/components/structures/TimelinePanel.js | 18 ++++- .../structures/TimelinePanel-test.js | 68 +++++++++++++++++++ test/test-utils.js | 8 ++- 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 5367b4208a..1f7c98b0b1 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -241,11 +241,25 @@ var TimelinePanel = React.createClass({ if (this.unmounted) { return; } debuglog("TimelinePanel: paginate complete backwards:"+backwards+"; success:"+r); - this.setState({ + + var newState = { [paginatingKey]: false, [canPaginateKey]: r, events: this._getEvents(), - }); + }; + + // moving the window in this direction may mean that we can now + // paginate in the other where we previously could not. + var otherDirection = backwards ? EventTimeline.FORWARDS : EventTimeline.BACKWARDS; + var canPaginateOtherWayKey = backwards ? 'canForwardPaginate' : 'canBackPaginate'; + if (!this.state[canPaginateOtherWayKey] && + this._timelineWindow.canPaginate(otherDirection)) { + debuglog('TimelinePanel: can now', otherDirection, 'paginate again'); + newState[canPaginateOtherWayKey] = true; + } + + this.setState(newState); + return r; }); }, diff --git a/test/components/structures/TimelinePanel-test.js b/test/components/structures/TimelinePanel-test.js index 671d5c7774..fe916f0f5d 100644 --- a/test/components/structures/TimelinePanel-test.js +++ b/test/components/structures/TimelinePanel-test.js @@ -204,4 +204,72 @@ describe('TimelinePanel', function() { }, 0); }, 0); }); + + it("should let you scroll down again after you've scrolled up", function(done) { + var N_EVENTS = 600; + + // sadly, loading all those events takes a while + this.timeout(N_EVENTS * 20); + + // client.getRoom is called a /lot/ in this test, so replace + // sinon's spy with a fast noop. + client.getRoom = function(id) { return null; }; + + // fill the timeline with lots of events + for (var i = 0; i < N_EVENTS; i++) { + timeline.addEvent(mkMessage()); + } + + var scrollDefer; + var panel = ReactDOM.render( + {scrollDefer.resolve()}} />, + parentDiv + ); + + var messagePanel = ReactTestUtils.findRenderedComponentWithType( + panel, sdk.getComponent('structures.MessagePanel')); + var scrollingDiv = ReactTestUtils.findRenderedDOMComponentWithClass( + panel, "gm-scroll-view"); + + // helper function which will return a promise which resolves when + // the TimelinePanel fires a scroll event + var awaitScroll = function() { + scrollDefer = q.defer(); + return scrollDefer.promise; + }; + + function backPaginate() { + scrollingDiv.scrollTop = 0; + return awaitScroll().then(() => { + if(scrollingDiv.scrollTop > 0) { + // need to go further + return backPaginate(); + } + + // hopefully, we got to the start of the timeline + expect(messagePanel.props.backPaginating).toBe(false); + }); + } + + // let the first round of pagination finish off + awaitScroll().then(() => { + // we should now have loaded the first few events + expect(messagePanel.props.backPaginating).toBe(false); + expect(messagePanel.props.suppressFirstDateSeparator).toBe(true); + + // back-paginate until we hit the start + return backPaginate(); + }).then(() => { + expect(messagePanel.props.suppressFirstDateSeparator).toBe(false); + var events = scryEventTiles(panel); + expect(events[0].props.mxEvent).toBe(timeline.getEvents()[0]) + + // we should now be able to scroll down, and paginate in the other + // direction. + scrollingDiv.scrollTop = scrollingDiv.scrollHeight; + return awaitScroll(); + }).then(() => { + console.log("done"); + }).done(done, done); + }); }); diff --git a/test/test-utils.js b/test/test-utils.js index a077722678..cbb8fabe18 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -42,8 +42,8 @@ module.exports.stubClient = function() { sendReadReceipt: sinon.stub().returns(q()), }; - // create the peg - + // stub out the methods in MatrixClientPeg + // // 'sandbox.restore()' doesn't work correctly on inherited methods, // so we do this for each method var methods = ['get', 'unset', 'replaceUsingUrls', @@ -51,7 +51,9 @@ module.exports.stubClient = function() { for (var i = 0; i < methods.length; i++) { sandbox.stub(peg, methods[i]); } - peg.get.returns(client); + // MatrixClientPeg.get() is called a /lot/, so implement it with our own + // fast stub function rather than a sinon stub + peg.get = function() { return client; }; return sandbox; }