diff --git a/scripts/travis/unit-tests.sh b/scripts/travis/unit-tests.sh index a8e0a63b31..a7e8425fa0 100755 --- a/scripts/travis/unit-tests.sh +++ b/scripts/travis/unit-tests.sh @@ -7,4 +7,4 @@ set -ev scripts/travis/build.sh -npm run test +CHROME_BIN='/usr/bin/google-chrome-stable' npm run test diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 185af4cd6d..9034123e52 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -635,9 +635,9 @@ module.exports = React.createClass({ _onTypingVisible: function() { const scrollPanel = this.refs.scrollPanel; if (scrollPanel && scrollPanel.getScrollState().stuckAtBottom) { - scrollPanel.blockShrinking(); // scroll down if at bottom scrollPanel.checkScroll(); + scrollPanel.blockShrinking(); } }, @@ -648,12 +648,23 @@ module.exports = React.createClass({ const isAtBottom = scrollPanel.isAtBottom(); const whoIsTyping = this.refs.whoIsTyping; const isTypingVisible = whoIsTyping && whoIsTyping.isVisible(); + // when messages get added to the timeline, + // but somebody else is still typing, + // update the min-height, so once the last + // person stops typing, no jumping occurs if (isAtBottom && isTypingVisible) { scrollPanel.blockShrinking(); } } }, + clearTimelineHeight: function() { + const scrollPanel = this.refs.scrollPanel; + if (scrollPanel) { + scrollPanel.clearBlockShrinking(); + } + }, + onResize: function() { dis.dispatch({ action: 'timeline_resize' }, true); }, diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index a1a7d08e0b..2dbc711067 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -78,6 +78,27 @@ if (DEBUG_SCROLL) { * scroll down further. If stickyBottom is disabled, we just save the scroll * offset as normal. */ + + +function createTimelineResizeDetector(scrollNode, itemlist, callback) { + if (typeof ResizeObserver !== "undefined") { + const ro = new ResizeObserver(callback); + ro.observe(itemlist); + return ro; + } else if (typeof IntersectionObserver !== "undefined") { + const threshold = []; + for (let i = 0; i <= 1000; ++i) { + threshold.push(i / 1000); + } + const io = new IntersectionObserver( + callback, + {root: scrollNode, threshold}, + ); + io.observe(itemlist); + return io; + } +} + module.exports = React.createClass({ displayName: 'ScrollPanel', @@ -160,6 +181,12 @@ module.exports = React.createClass({ componentDidMount: function() { this.checkScroll(); + + this._timelineSizeObserver = createTimelineResizeDetector( + this._getScrollNode(), + this.refs.itemlist, + () => { this._restoreSavedScrollState(); }, + ); }, componentDidUpdate: function() { @@ -169,10 +196,6 @@ module.exports = React.createClass({ // // This will also re-check the fill state, in case the paginate was inadequate this.checkScroll(); - - if (!this.isAtBottom()) { - this.clearBlockShrinking(); - } }, componentWillUnmount: function() { @@ -181,6 +204,10 @@ module.exports = React.createClass({ // // (We could use isMounted(), but facebook have deprecated that.) this.unmounted = true; + if (this._timelineSizeObserver) { + this._timelineSizeObserver.disconnect(); + this._timelineSizeObserver = null; + } }, onScroll: function(ev) { @@ -188,46 +215,21 @@ module.exports = React.createClass({ debuglog("Scroll event: offset now:", sn.scrollTop, "_lastSetScroll:", this._lastSetScroll); - // Sometimes we see attempts to write to scrollTop essentially being - // ignored. (Or rather, it is successfully written, but on the next - // scroll event, it's been reset again). - // - // This was observed on Chrome 47, when scrolling using the trackpad in OS - // X Yosemite. Can't reproduce on El Capitan. Our theory is that this is - // due to Chrome not being able to cope with the scroll offset being reset - // while a two-finger drag is in progress. - // - // By way of a workaround, we detect this situation and just keep - // resetting scrollTop until we see the scroll node have the right - // value. - if (this._lastSetScroll !== undefined && sn.scrollTop < this._lastSetScroll-200) { - console.log("Working around vector-im/vector-web#528"); - this._restoreSavedScrollState(); - return; - } - // If there weren't enough children to fill the viewport, the scroll we // got might be different to the scroll we wanted; we don't want to // forget what we wanted, so don't overwrite the saved state unless // this appears to be a user-initiated scroll. if (sn.scrollTop != this._lastSetScroll) { - // when scrolling, we don't care about disappearing typing notifs shrinking the timeline - // this might cause the scrollbar to resize in case the max-height was not correct - // but that's better than ending up with a lot of whitespace at the bottom of the timeline. - // we need to above check because when showing the typing notifs, an onScroll event is also triggered - if (!this.isAtBottom()) { - this.clearBlockShrinking(); - } - this._saveScrollState(); } else { debuglog("Ignoring scroll echo"); - // only ignore the echo once, otherwise we'll get confused when the // user scrolls away from, and back to, the autoscroll point. this._lastSetScroll = undefined; } + this._checkBlockShrinking(); + this.props.onScroll(ev); this.checkFillState(); @@ -235,8 +237,6 @@ module.exports = React.createClass({ onResize: function() { this.props.onResize(); - // clear min-height as the height might have changed - this.clearBlockShrinking(); this.checkScroll(); if (this._gemScroll) this._gemScroll.forceUpdate(); }, @@ -245,6 +245,7 @@ module.exports = React.createClass({ // where it ought to be, and set off pagination requests if necessary. checkScroll: function() { this._restoreSavedScrollState(); + this._checkBlockShrinking(); this.checkFillState(); }, @@ -386,8 +387,6 @@ module.exports = React.createClass({ } this._unfillDebouncer = setTimeout(() => { this._unfillDebouncer = null; - // if timeline shrinks, min-height should be cleared - this.clearBlockShrinking(); this.props.onUnfillRequest(backwards, markerScrollToken); }, UNFILL_REQUEST_DEBOUNCE_MS); } @@ -583,9 +582,10 @@ module.exports = React.createClass({ } const scrollNode = this._getScrollNode(); - const wrapperRect = ReactDOM.findDOMNode(this).getBoundingClientRect(); - const boundingRect = node.getBoundingClientRect(); - const scrollDelta = boundingRect.bottom + pixelOffset - wrapperRect.bottom; + const scrollBottom = scrollNode.scrollTop + scrollNode.clientHeight; + + const nodeBottom = node.offsetTop + node.clientHeight; + const scrollDelta = nodeBottom + pixelOffset - scrollBottom; debuglog("ScrollPanel: scrolling to token '" + scrollToken + "'+" + pixelOffset + " (delta: "+scrollDelta+")"); @@ -602,42 +602,43 @@ module.exports = React.createClass({ return; } + const scrollNode = this._getScrollNode(); + const scrollBottom = scrollNode.scrollTop + scrollNode.clientHeight; + const itemlist = this.refs.itemlist; - const wrapperRect = ReactDOM.findDOMNode(this).getBoundingClientRect(); const messages = itemlist.children; - let newScrollState = null; + let node = null; + // loop backwards, from bottom-most message (as that is the most common case) for (let i = messages.length-1; i >= 0; --i) { - const node = messages[i]; - if (!node.dataset.scrollTokens) continue; - - const boundingRect = node.getBoundingClientRect(); - newScrollState = { - stuckAtBottom: false, - 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 - // as the scrollToken. - // If this is false for the entire for-loop, we default to the last node - // (which is why newScrollState is set on every iteration). - if (boundingRect.top < wrapperRect.bottom) { + if (!messages[i].dataset.scrollTokens) { + continue; + } + node = messages[i]; + // break at the first message (coming from the bottom) + // that has it's offsetTop above the bottom of the viewport. + if (node.offsetTop < scrollBottom) { // Use this node as the scrollToken break; } } - // 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); - } else { + + if (!node) { debuglog("ScrollPanel: unable to save scroll state: found no children in the viewport"); + return; } + + const nodeBottom = node.offsetTop + node.clientHeight; + debuglog("ScrollPanel: saved scroll state", this.scrollState); + this.scrollState = { + stuckAtBottom: false, + trackedScrollToken: node.dataset.scrollTokens.split(',')[0], + pixelOffset: scrollBottom - nodeBottom, + }; }, _restoreSavedScrollState: function() { const scrollState = this.scrollState; - const scrollNode = this._getScrollNode(); if (scrollState.stuckAtBottom) { this._setScrollTop(Number.MAX_VALUE); @@ -717,6 +718,21 @@ module.exports = React.createClass({ } }, + _checkBlockShrinking: function() { + const sn = this._getScrollNode(); + const scrollState = this.scrollState; + if (!scrollState.stuckAtBottom) { + const spaceBelowViewport = sn.scrollHeight - (sn.scrollTop + sn.clientHeight); + // only if we've scrolled up 200px from the bottom + // should we clear the min-height used by the typing notifications, + // otherwise we might still see it jump as the whitespace disappears + // when scrolling up from the bottom + if (spaceBelowViewport >= 200) { + this.clearBlockShrinking(); + } + } + }, + render: function() { const GeminiScrollbarWrapper = sdk.getComponent("elements.GeminiScrollbarWrapper"); // TODO: the classnames on the div and ol could do with being updated to diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 8890c26d42..911499e314 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -935,6 +935,11 @@ var TimelinePanel = React.createClass({ {windowLimit: this.props.timelineCap}); const onLoaded = () => { + // clear the timeline min-height when + // (re)loading the timeline + if (this.refs.messagePanel) { + this.refs.messagePanel.clearTimelineHeight(); + } this._reloadEvents(); // If we switched away from the room while there were pending diff --git a/test/components/structures/ScrollPanel-test.js b/test/components/structures/ScrollPanel-test.js index 0e091cdddf..6bdbcdb247 100644 --- a/test/components/structures/ScrollPanel-test.js +++ b/test/components/structures/ScrollPanel-test.js @@ -224,57 +224,4 @@ describe('ScrollPanel', function() { expect(scrollingDiv.scrollTop).toEqual(1950); }); }); - - it('should not get stuck in #528 workaround', function(done) { - let events = []; - Promise.resolve().then(() => { - // initialise with a bunch of events - for (let i = 0; i < 40; i++) { - events.push(i); - } - tester.setTileKeys(events); - expect(tester.fillCounts.b).toEqual(1); - expect(tester.fillCounts.f).toEqual(2); - expect(scrollingDiv.scrollHeight).toEqual(6050); // 40*150 + 50 - expect(scrollingDiv.scrollTop).toEqual(6050 - 600); - - // try to scroll up, to a non-integer offset. - tester.scrollPanel().scrollToToken("30", -101/3); - - expect(scrollingDiv.scrollTop).toEqual(4616); // 31*150 - 34 - - // wait for the scroll event to land - return tester.awaitScroll(); // fails - }).then(() => { - expect(tester.lastScrollEvent).toEqual(4616); - - // Now one more event; this will make it reset the scroll, but - // because the delta will be less than 1, will not trigger a - // scroll event, this leaving recentEventScroll defined. - console.log("Adding event 50"); - events.push(50); - tester.setTileKeys(events); - - // wait for the scrollpanel to stop trying to paginate - }).then(() => { - // Now, simulate hitting "scroll to bottom". - events = []; - for (let i = 100; i < 120; i++) { - events.push(i); - } - tester.setTileKeys(events); - tester.scrollPanel().scrollToBottom(); - - // wait for the scroll event to land - return tester.awaitScroll(); // fails - }).then(() => { - expect(scrollingDiv.scrollTop).toEqual(20*150 + 50 - 600); - - // simulate a user-initiated scroll on the div - scrollingDiv.scrollTop = 1200; - return tester.awaitScroll(); - }).then(() => { - expect(scrollingDiv.scrollTop).toEqual(1200); - }).done(done); - }); });