From 8cf2607415ad7bdb837fa0de15dc9c28a353bae8 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:30:06 +0100 Subject: [PATCH 01/52] use AutoHideScrollbar in ScrollPanel --- res/css/structures/_RoomView.scss | 2 + .../structures/AutoHideScrollbar.js | 1 + src/components/structures/ScrollPanel.js | 40 +++++++++---------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/res/css/structures/_RoomView.scss b/res/css/structures/_RoomView.scss index f15552e484a..1b639928e02 100644 --- a/res/css/structures/_RoomView.scss +++ b/res/css/structures/_RoomView.scss @@ -84,6 +84,7 @@ limitations under the License. display: flex; flex-direction: column; flex: 1; + min-width: 0; } .mx_RoomView_body .mx_RoomView_timeline { @@ -111,6 +112,7 @@ limitations under the License. .mx_RoomView_messagePanel { width: 100%; overflow-y: auto; + flex: 1 1 0; } .mx_RoomView_messagePanelSearchSpinner { diff --git a/src/components/structures/AutoHideScrollbar.js b/src/components/structures/AutoHideScrollbar.js index 0f93f204078..72d48a20841 100644 --- a/src/components/structures/AutoHideScrollbar.js +++ b/src/components/structures/AutoHideScrollbar.js @@ -121,6 +121,7 @@ export default class AutoHideScrollbar extends React.Component { render() { return (
diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index ee4045c91e4..cdb79686adc 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -15,14 +15,13 @@ limitations under the License. */ const React = require("react"); -const ReactDOM = require("react-dom"); import PropTypes from 'prop-types'; import Promise from 'bluebird'; import { KeyCode } from '../../Keyboard'; -import sdk from '../../index.js'; +import AutoHideScrollbar from "./AutoHideScrollbar"; const DEBUG_SCROLL = false; -// var DEBUG_SCROLL = true; +// const DEBUG_SCROLL = true; // The amount of extra scroll distance to allow prior to unfilling. // See _getExcessHeight. @@ -129,11 +128,6 @@ module.exports = React.createClass({ */ onScroll: PropTypes.func, - /* onResize: a callback which is called whenever the Gemini scroll - * panel is resized - */ - onResize: PropTypes.func, - /* className: classnames to add to the top-level div */ className: PropTypes.string, @@ -150,7 +144,6 @@ module.exports = React.createClass({ onFillRequest: function(backwards) { return Promise.resolve(false); }, onUnfillRequest: function(backwards, scrollToken) {}, onScroll: function() {}, - onResize: function() {}, }; }, @@ -185,6 +178,16 @@ module.exports = React.createClass({ debuglog("Scroll event: offset now:", sn.scrollTop, "_lastSetScroll:", this._lastSetScroll); + // ignore scroll events where scrollTop hasn't changed, + // appears to happen when the layout changes outside + // of the scroll container, like resizing the right panel. + if (sn.scrollTop === this._lastEventScroll) { + debuglog("ignore scroll event with same scrollTop as before"); + return; + } + + this._lastEventScroll = sn.scrollTop; + // 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). @@ -225,9 +228,7 @@ module.exports = React.createClass({ onResize: function() { this.clearBlockShrinking(); - this.props.onResize(); this.checkScroll(); - if (this._gemScroll) this._gemScroll.forceUpdate(); }, // after an update to the contents of the panel, check that the scroll is @@ -672,17 +673,17 @@ module.exports = React.createClass({ throw new Error("ScrollPanel._getScrollNode called when unmounted"); } - if (!this._gemScroll) { + if (!this._divScroll) { // Likewise, we should have the ref by this point, but if not // turn the NPE into something meaningful. throw new Error("ScrollPanel._getScrollNode called before gemini ref collected"); } - return this._gemScroll.scrollbar.getViewElement(); + return this._divScroll; }, - _collectGeminiScroll: function(gemScroll) { - this._gemScroll = gemScroll; + _collectScroll: function(divScroll) { + this._divScroll = divScroll; }, /** @@ -724,19 +725,18 @@ module.exports = React.createClass({ }, render: function() { - const GeminiScrollbarWrapper = sdk.getComponent("elements.GeminiScrollbarWrapper"); // TODO: the classnames on the div and ol could do with being updated to // reflect the fact that we don't necessarily contain a list of messages. // it's not obvious why we have a separate div and ol anyway. - return (
    { this.props.children }
- - ); + + ); }, }); From 27070b314960ed3cfc0e4415e96534accff821a3 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:33:05 +0100 Subject: [PATCH 02/52] remove onChildResize in RoomView as it's unused --- src/components/structures/RoomView.js | 15 ++------------- src/components/views/rooms/MessageComposer.js | 5 ----- .../views/rooms/MessageComposerInput.js | 4 ---- 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 2a1c7fe79e1..d7488a2558f 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -1355,8 +1355,7 @@ module.exports = React.createClass({ const showBar = this.refs.messagePanel.canJumpToReadMarker(); if (this.state.showTopUnreadMessagesBar != showBar) { - this.setState({showTopUnreadMessagesBar: showBar}, - this.onChildResize); + this.setState({showTopUnreadMessagesBar: showBar}); } }, @@ -1399,7 +1398,7 @@ module.exports = React.createClass({ }; }, - onResize: function(e) { + onResize: function() { // It seems flexbox doesn't give us a way to constrain the auxPanel height to have // a minimum of the height of the video element, whilst also capping it from pushing out the page // so we have to do it via JS instead. In this implementation we cap the height by putting @@ -1417,9 +1416,6 @@ module.exports = React.createClass({ if (auxPanelMaxHeight < 50) auxPanelMaxHeight = 50; this.setState({auxPanelMaxHeight: auxPanelMaxHeight}); - - // changing the maxHeight on the auxpanel will trigger a callback go - // onChildResize, so no need to worry about that here. }, onFullscreenClick: function() { @@ -1449,10 +1445,6 @@ module.exports = React.createClass({ this.forceUpdate(); // TODO: just update the voip buttons }, - onChildResize: function() { - // no longer anything to do here - }, - onStatusBarVisible: function() { if (this.unmounted) return; this.setState({ @@ -1645,7 +1637,6 @@ module.exports = React.createClass({ isPeeking={myMembership !== "join"} onInviteClick={this.onInviteButtonClick} onStopWarningClick={this.onStopAloneWarningClick} - onResize={this.onChildResize} onVisible={this.onStatusBarVisible} onHidden={this.onStatusBarHidden} />; @@ -1714,7 +1705,6 @@ module.exports = React.createClass({ draggingFile={this.state.draggingFile} displayConfCallNotification={this.state.displayConfCallNotification} maxHeight={this.state.auxPanelMaxHeight} - onResize={this.onChildResize} showApps={this.state.showApps} hideAppsDrawer={false} > { aux } @@ -1730,7 +1720,6 @@ module.exports = React.createClass({ messageComposer = this.messageComposerInput = c} key="controls_input" - onResize={this.props.onResize} room={this.props.room} placeholder={placeholderText} onFilesPasted={this.uploadFiles} @@ -505,10 +504,6 @@ export default class MessageComposer extends React.Component { } MessageComposer.propTypes = { - // a callback which is called when the height of the composer is - // changed due to a change in content. - onResize: PropTypes.func, - // js-sdk Room object room: PropTypes.object.isRequired, diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 6b80902c8f8..cbea2bccb9a 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -135,10 +135,6 @@ function rangeEquals(a: Range, b: Range): boolean { */ export default class MessageComposerInput extends React.Component { static propTypes = { - // a callback which is called when the height of the composer is - // changed due to a change in content. - onResize: PropTypes.func, - // js-sdk Room object room: PropTypes.object.isRequired, From 735b4f6fcfbff30e0064568282f17e4dc8b765bc Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:36:12 +0100 Subject: [PATCH 03/52] create ResizeNotifier to derive which areas of the app resize and emit --- src/components/structures/FilePanel.js | 1 + src/components/structures/LoggedInView.js | 2 + src/components/structures/MainSplit.js | 1 + src/components/structures/MatrixChat.js | 3 ++ src/components/structures/MessagePanel.js | 3 +- src/components/structures/RightPanel.js | 2 +- src/components/structures/RoomView.js | 13 ++++-- src/components/structures/ScrollPanel.js | 3 ++ src/components/structures/TimelinePanel.js | 1 + src/utils/ResizeNotifier.js | 52 ++++++++++++++++++++++ 10 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 src/utils/ResizeNotifier.js diff --git a/src/components/structures/FilePanel.js b/src/components/structures/FilePanel.js index 927449750c9..e35a39a1073 100644 --- a/src/components/structures/FilePanel.js +++ b/src/components/structures/FilePanel.js @@ -123,6 +123,7 @@ const FilePanel = React.createClass({ timelineSet={this.state.timelineSet} showUrlPreview = {false} tileShape="file_grid" + resizeNotifier={this.props.resizeNotifier} empty={_t('There are no visible files in this room')} /> ); diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index c6c1be67ec5..5267dba715d 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -173,6 +173,7 @@ const LoggedInView = React.createClass({ }, onResized: (size) => { window.localStorage.setItem("mx_lhs_size", '' + size); + this.props.resizeNotifier.notifyLeftHandleResized(); }, }; const resizer = new Resizer( @@ -448,6 +449,7 @@ const LoggedInView = React.createClass({ disabled={this.props.middleDisabled} collapsedRhs={this.props.collapsedRhs} ConferenceHandler={this.props.ConferenceHandler} + resizeNotifier={this.props.resizeNotifier} />; break; diff --git a/src/components/structures/MainSplit.js b/src/components/structures/MainSplit.js index 0427130eeac..c0bf74d02c7 100644 --- a/src/components/structures/MainSplit.js +++ b/src/components/structures/MainSplit.js @@ -27,6 +27,7 @@ export default class MainSplit extends React.Component { _onResized(size) { window.localStorage.setItem("mx_rhs_size", size); + this.props.resizeNotifier.notifyRightHandleResized(); } _createResizer() { diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 2622a6bf93b..a9b34c9058a 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -48,6 +48,7 @@ import { _t, getCurrentLanguage } from '../../languageHandler'; import SettingsStore, {SettingLevel} from "../../settings/SettingsStore"; import { startAnyRegistrationFlow } from "../../Registration.js"; import { messageForSyncError } from '../../utils/ErrorUtils'; +import ResizeNotifier from "../../utils/ResizeNotifier"; const AutoDiscovery = Matrix.AutoDiscovery; @@ -194,6 +195,7 @@ export default React.createClass({ hideToSRUsers: false, syncError: null, // If the current syncing status is ERROR, the error object, otherwise null. + resizeNotifier: new ResizeNotifier(), }; return s; }, @@ -1661,6 +1663,7 @@ export default React.createClass({ dis.dispatch({ action: 'show_right_panel' }); } + this.state.resizeNotifier.notifyWindowResized(); this._windowWidth = window.innerWidth; }, diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index b1f88a62212..aec2f8cbe17 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -703,7 +703,8 @@ module.exports = React.createClass({ onFillRequest={this.props.onFillRequest} onUnfillRequest={this.props.onUnfillRequest} style={style} - stickyBottom={this.props.stickyBottom}> + stickyBottom={this.props.stickyBottom} + resizeNotifier={this.props.resizeNotifier}> { topSpinner } { this._getEventTiles() } { whoIsTyping } diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index 5c745b04cc2..93cbff32330 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -193,7 +193,7 @@ export default class RightPanel extends React.Component { } else if (this.state.phase === RightPanel.Phase.NotificationPanel) { panel = ; } else if (this.state.phase === RightPanel.Phase.FilePanel) { - panel = ; + panel = ; } const classes = classNames("mx_RightPanel", "mx_fadable", { diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index d7488a2558f..5a914cbd1cb 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -392,7 +392,7 @@ module.exports = React.createClass({ this._updateConfCallNotification(); window.addEventListener('beforeunload', this.onPageUnload); - window.addEventListener('resize', this.onResize); + this.props.resizeNotifier.on("middlePanelResized", this.onResize); this.onResize(); document.addEventListener("keydown", this.onKeyDown); @@ -472,7 +472,7 @@ module.exports = React.createClass({ } window.removeEventListener('beforeunload', this.onPageUnload); - window.removeEventListener('resize', this.onResize); + this.props.resizeNotifier.removeListener("middlePanelResized", this.onResize); document.removeEventListener("keydown", this.onKeyDown); @@ -1829,6 +1829,7 @@ module.exports = React.createClass({ className="mx_RoomView_messagePanel" membersLoaded={this.state.membersLoaded} permalinkCreator={this.state.permalinkCreator} + resizeNotifier={this.props.resizeNotifier} />); let topUnreadMessagesBar = null; @@ -1861,7 +1862,7 @@ module.exports = React.createClass({ }, ); - const rightPanel = this.state.room ? : undefined; + const rightPanel = this.state.room ? : undefined; return (
@@ -1877,7 +1878,11 @@ module.exports = React.createClass({ onLeaveClick={(myMembership === "join") ? this.onLeaveClick : null} e2eStatus={this.state.e2eStatus} /> - +
{ auxPanel }
diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index cdb79686adc..799c88140e2 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -149,6 +149,8 @@ module.exports = React.createClass({ componentWillMount: function() { this._pendingFillRequests = {b: null, f: null}; + this.props.resizeNotifier.on("middlePanelResized", this.onResize); + this.resetScrollState(); }, @@ -171,6 +173,7 @@ module.exports = React.createClass({ // // (We could use isMounted(), but facebook have deprecated that.) this.unmounted = true; + this.props.resizeNotifier.removeListener("middlePanelResized", this.onResize); }, onScroll: function(ev) { diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 911499e3149..537862ab1e2 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -1227,6 +1227,7 @@ var TimelinePanel = React.createClass({ alwaysShowTimestamps={this.state.alwaysShowTimestamps} className={this.props.className} tileShape={this.props.tileShape} + resizeNotifier={this.props.resizeNotifier} /> ); }, diff --git a/src/utils/ResizeNotifier.js b/src/utils/ResizeNotifier.js new file mode 100644 index 00000000000..43578ebcaa8 --- /dev/null +++ b/src/utils/ResizeNotifier.js @@ -0,0 +1,52 @@ +/* +Copyright 2019 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/** + * Fires when the middle panel has been resized. + * @event module:utils~ResizeNotifier#"middlePanelResized" + */ +import { EventEmitter } from "events"; +import { throttle } from "lodash"; + +export default class ResizeNotifier extends EventEmitter { + + constructor() { + super(); + // with default options, will call fn once at first call, and then every x ms + // if there was another call in that timespan + this._throttledMiddlePanel = throttle(() => this.emit("middlePanelResized"), 200); + } + + notifyBannersChanged() { + this.emit("middlePanelResized"); + } + + // can be called in quick succession + notifyLeftHandleResized() { + this._throttledMiddlePanel(); + } + + // can be called in quick succession + notifyRightHandleResized() { + this._throttledMiddlePanel(); + } + + // can be called in quick succession + notifyWindowResized() { + this._throttledMiddlePanel(); + } +} + From 56aeb5194a5e584c30b28e25abff3ab7b4fce33c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:37:20 +0100 Subject: [PATCH 04/52] emit timeline_resize in MatrixChat based on ResizeNotifier as it's used in PersistentElement which could be used at various places --- src/components/structures/MatrixChat.js | 8 ++++++++ src/components/structures/MessagePanel.js | 5 ----- src/components/structures/RoomView.js | 5 ----- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index a9b34c9058a..6e05138e2d3 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -318,6 +318,9 @@ export default React.createClass({ // N.B. we don't call the whole of setTheme() here as we may be // racing with the theme CSS download finishing from index.js Tinter.tint(); + + // For PersistentElement + this.state.resizeNotifier.on("middlePanelResized", this._dispatchTimelineResize); }, componentDidMount: function() { @@ -400,6 +403,7 @@ export default React.createClass({ dis.unregister(this.dispatcherRef); window.removeEventListener("focus", this.onFocus); window.removeEventListener('resize', this.handleResize); + this.state.resizeNotifier.removeListener("middlePanelResized", this._dispatchTimelineResize); }, componentWillUpdate: function(props, state) { @@ -1667,6 +1671,10 @@ export default React.createClass({ this._windowWidth = window.innerWidth; }, + _dispatchTimelineResize() { + dis.dispatch({ action: 'timeline_resize' }, true); + }, + onRoomCreated: function(roomId) { dis.dispatch({ action: "view_room", diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index aec2f8cbe17..fecf5f1ad7d 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -21,7 +21,6 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import shouldHideEvent from '../../shouldHideEvent'; import {wantsDateSeparator} from '../../DateUtils'; -import dis from "../../dispatcher"; import sdk from '../../index'; import MatrixClientPeg from '../../MatrixClientPeg'; @@ -665,10 +664,6 @@ module.exports = React.createClass({ } }, - onResize: function() { - dis.dispatch({ action: 'timeline_resize' }, true); - }, - render: function() { const ScrollPanel = sdk.getComponent("structures.ScrollPanel"); const WhoIsTypingTile = sdk.getComponent("rooms.WhoIsTypingTile"); diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 5a914cbd1cb..507fbf6979d 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -856,10 +856,6 @@ module.exports = React.createClass({ } }, - onSearchResultsResize: function() { - dis.dispatch({ action: 'timeline_resize' }, true); - }, - onSearchResultsFillRequest: function(backwards) { if (!backwards) { return Promise.resolve(false); @@ -1794,7 +1790,6 @@ module.exports = React.createClass({
  • { this.getSearchResultTiles() } From c8123ec66536c0e10e07c607b7a116d4a444ab6d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:38:30 +0100 Subject: [PATCH 05/52] use AutoHideScrollbar in memberlist --- res/css/views/rooms/_MemberList.scss | 5 +++++ src/components/views/rooms/MemberList.js | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/res/css/views/rooms/_MemberList.scss b/res/css/views/rooms/_MemberList.scss index 9f2b5da930f..cac97cb60d1 100644 --- a/res/css/views/rooms/_MemberList.scss +++ b/res/css/views/rooms/_MemberList.scss @@ -20,6 +20,7 @@ limitations under the License. flex: 1; display: flex; flex-direction: column; + min-height: 0; .mx_Spinner { flex: 1 0 auto; @@ -35,6 +36,10 @@ limitations under the License. margin-top: 8px; margin-bottom: 4px; } + + .mx_AutoHideScrollbar { + flex: 1 1 0; + } } .mx_MemberList_chevron { diff --git a/src/components/views/rooms/MemberList.js b/src/components/views/rooms/MemberList.js index f9ce672c167..29f49f1691c 100644 --- a/src/components/views/rooms/MemberList.js +++ b/src/components/views/rooms/MemberList.js @@ -20,6 +20,8 @@ import React from 'react'; import { _t } from '../../../languageHandler'; import SdkConfig from '../../../SdkConfig'; import dis from '../../../dispatcher'; +import AutoHideScrollbar from "../../structures/AutoHideScrollbar"; + const MatrixClientPeg = require("../../../MatrixClientPeg"); const sdk = require('../../../index'); const rate_limited_func = require('../../../ratelimitedfunc'); @@ -439,7 +441,6 @@ module.exports = React.createClass({ const SearchBox = sdk.getComponent('structures.SearchBox'); const TruncatedList = sdk.getComponent("elements.TruncatedList"); - const GeminiScrollbarWrapper = sdk.getComponent("elements.GeminiScrollbarWrapper"); const cli = MatrixClientPeg.get(); const room = cli.getRoom(this.props.roomId); @@ -466,7 +467,7 @@ module.exports = React.createClass({ return (
    { inviteButton } - +
    - + Date: Tue, 12 Mar 2019 16:38:42 +0100 Subject: [PATCH 06/52] use AutoHideScrollbar in member info panel --- src/components/views/rooms/MemberInfo.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/MemberInfo.js b/src/components/views/rooms/MemberInfo.js index 3ada730ec84..35161dedf7d 100644 --- a/src/components/views/rooms/MemberInfo.js +++ b/src/components/views/rooms/MemberInfo.js @@ -44,6 +44,7 @@ import SdkConfig from '../../../SdkConfig'; import MultiInviter from "../../../utils/MultiInviter"; import SettingsStore from "../../../settings/SettingsStore"; import E2EIcon from "./E2EIcon"; +import AutoHideScrollbar from "../../structures/AutoHideScrollbar"; module.exports = withMatrixClient(React.createClass({ displayName: 'MemberInfo', @@ -1003,7 +1004,7 @@ module.exports = withMatrixClient(React.createClass({ { roomMemberDetails }
    - +
    { this._renderUserOptions() } @@ -1015,7 +1016,7 @@ module.exports = withMatrixClient(React.createClass({ { spinner }
    -     +
    ); }, From 58f26ee9b070b1eca2943bdca5e14a96ca390a84 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 17:29:16 +0100 Subject: [PATCH 07/52] emit resize event when banners are shown/hidden to restore scroll pos --- src/Notifier.js | 12 +++++++++++- src/components/structures/LoggedInView.js | 19 +++++++++++++------ src/components/structures/MatrixChat.js | 13 ++++++++++--- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/Notifier.js b/src/Notifier.js index 80e8be10846..ab8009c4579 100644 --- a/src/Notifier.js +++ b/src/Notifier.js @@ -220,7 +220,17 @@ const Notifier = { } }, - isToolbarHidden: function() { + shouldShowToolbar: function() { + const client = MatrixClientPeg.get(); + if (!client) { + return false; + } + const isGuest = client.isGuest(); + return !isGuest && Notifier.supportsDesktopNotifications() && + !Notifier.isEnabled() && !Notifier._isToolbarHidden(); + }, + + _isToolbarHidden: function() { // Check localStorage for any such meta data if (global.localStorage) { return global.localStorage.getItem("notifications_hidden") === "true"; diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index 5267dba715d..c22c217e5fb 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -22,7 +22,6 @@ import PropTypes from 'prop-types'; import { DragDropContext } from 'react-beautiful-dnd'; import { KeyCode, isOnlyCtrlOrCmdKeyEvent } from '../../Keyboard'; -import Notifier from '../../Notifier'; import PageTypes from '../../PageTypes'; import CallMediaHandler from '../../CallMediaHandler'; import sdk from '../../index'; @@ -121,6 +120,18 @@ const LoggedInView = React.createClass({ this._matrixClient.on("RoomState.events", this.onRoomStateEvents); }, + componentDidUpdate(prevProps) { + // attempt to guess when a banner was opened or closed + if ( + (prevProps.showCookieBar !== this.props.showCookieBar) || + (prevProps.hasNewVersion !== this.props.hasNewVersion) || + (prevProps.userHasGeneratedPassword !== this.props.userHasGeneratedPassword) || + (prevProps.showNotifierToolbar !== this.props.showNotifierToolbar) + ) { + this.props.resizeNotifier.notifyBannersChanged(); + } + }, + componentWillUnmount: function() { document.removeEventListener('keydown', this._onKeyDown); this._matrixClient.removeListener("accountData", this.onAccountData); @@ -491,7 +502,6 @@ const LoggedInView = React.createClass({ }); let topBar; - const isGuest = this.props.matrixClient.isGuest(); if (this.state.syncErrorData && this.state.syncErrorData.error.errcode === 'M_RESOURCE_LIMIT_EXCEEDED') { topBar = ; } else if (this.state.userHasGeneratedPassword) { topBar = ; - } else if ( - !isGuest && Notifier.supportsDesktopNotifications() && - !Notifier.isEnabled() && !Notifier.isToolbarHidden() - ) { + } else if (this.props.showNotifierToolbar) { topBar = ; } diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 6e05138e2d3..a7192b96cbf 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -29,6 +29,7 @@ import PlatformPeg from "../../PlatformPeg"; import SdkConfig from "../../SdkConfig"; import * as RoomListSorter from "../../RoomListSorter"; import dis from "../../dispatcher"; +import Notifier from '../../Notifier'; import Modal from "../../Modal"; import Tinter from "../../Tinter"; @@ -196,6 +197,7 @@ export default React.createClass({ syncError: null, // If the current syncing status is ERROR, the error object, otherwise null. resizeNotifier: new ResizeNotifier(), + showNotifierToolbar: Notifier.shouldShowToolbar(), }; return s; }, @@ -644,8 +646,9 @@ export default React.createClass({ case 'view_invite': showRoomInviteDialog(payload.roomId); break; - case 'notifier_enabled': - this.forceUpdate(); + case 'notifier_enabled': { + this.setState({showNotifierToolbar: Notifier.shouldShowToolbar()}); + } break; case 'hide_left_panel': this.setState({ @@ -1180,6 +1183,7 @@ export default React.createClass({ */ _onLoggedIn: async function() { this.setStateForNewView({view: VIEWS.LOGGED_IN}); + this.setState({showNotifierToolbar: Notifier.shouldShowToolbar()}); if (this._is_registered) { this._is_registered = false; @@ -1672,7 +1676,10 @@ export default React.createClass({ }, _dispatchTimelineResize() { - dis.dispatch({ action: 'timeline_resize' }, true); + // prevent dispatch from within dispatch error + setTimeout(() => { + dis.dispatch({ action: 'timeline_resize' }, true); + }, 0); }, onRoomCreated: function(roomId) { From 9541cc175f357fc0cdeab5299239eac199d58b9a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 18:00:05 +0100 Subject: [PATCH 08/52] use ResizeNotifier as well to relayout room list --- src/components/structures/LeftPanel.js | 2 +- src/components/structures/LoggedInView.js | 2 +- src/components/views/rooms/RoomList.js | 10 ++++++---- src/utils/ResizeNotifier.js | 8 ++++++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/components/structures/LeftPanel.js b/src/components/structures/LeftPanel.js index 21438c597cd..95b57a0ca57 100644 --- a/src/components/structures/LeftPanel.js +++ b/src/components/structures/LeftPanel.js @@ -234,7 +234,7 @@ const LeftPanel = React.createClass({ diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index c22c217e5fb..4771c6f4872 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -543,7 +543,7 @@ const LoggedInView = React.createClass({
    diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index 227dd318edf..2de9918e6e3 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -212,7 +212,7 @@ module.exports = React.createClass({ this._checkSubListsOverflow(); this.resizer.attach(); - window.addEventListener("resize", this.onWindowResize); + this.props.resizeNotifier.on("leftPanelResized", this.onResize); this.mounted = true; }, @@ -260,7 +260,6 @@ module.exports = React.createClass({ componentWillUnmount: function() { this.mounted = false; - window.removeEventListener("resize", this.onWindowResize); dis.unregister(this.dispatcherRef); if (MatrixClientPeg.get()) { MatrixClientPeg.get().removeListener("Room", this.onRoom); @@ -272,6 +271,8 @@ module.exports = React.createClass({ MatrixClientPeg.get().removeListener("Group.myMembership", this._onGroupMyMembership); MatrixClientPeg.get().removeListener("RoomState.events", this.onRoomStateEvents); } + this.props.resizeNotifier.removeListener("leftPanelResized", this.onResize); + if (this._tagStoreToken) { this._tagStoreToken.remove(); @@ -293,13 +294,14 @@ module.exports = React.createClass({ this._delayedRefreshRoomList.cancelPendingCall(); }, - onWindowResize: function() { + + onResize: function() { if (this.mounted && this._layout && this.resizeContainer && Array.isArray(this._layoutSections) ) { this._layout.update( this._layoutSections, - this.resizeContainer.offsetHeight + this.resizeContainer.offsetHeight, ); } }, diff --git a/src/utils/ResizeNotifier.js b/src/utils/ResizeNotifier.js index 43578ebcaa8..ff4b79091be 100644 --- a/src/utils/ResizeNotifier.js +++ b/src/utils/ResizeNotifier.js @@ -31,11 +31,13 @@ export default class ResizeNotifier extends EventEmitter { } notifyBannersChanged() { + this.emit("leftPanelResized"); this.emit("middlePanelResized"); } // can be called in quick succession notifyLeftHandleResized() { + // don't emit event for own region this._throttledMiddlePanel(); } @@ -46,6 +48,12 @@ export default class ResizeNotifier extends EventEmitter { // can be called in quick succession notifyWindowResized() { + // no need to throttle this one, + // also it could make scrollbars appear for + // a split second when the room list manual layout is now + // taller than the available space + this.emit("leftPanelResized"); + this._throttledMiddlePanel(); } } From 955ec14db98816dba3fe8a53de769cf11848df1e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 14 Mar 2019 15:04:09 +0100 Subject: [PATCH 09/52] chrome apparently anchors the scroll position, which fights against our restore position logic. Disable it like this. --- res/css/structures/_RoomView.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/res/css/structures/_RoomView.scss b/res/css/structures/_RoomView.scss index 1b639928e02..ced0b9eab36 100644 --- a/res/css/structures/_RoomView.scss +++ b/res/css/structures/_RoomView.scss @@ -113,6 +113,7 @@ limitations under the License. width: 100%; overflow-y: auto; flex: 1 1 0; + overflow-anchor: none; } .mx_RoomView_messagePanelSearchSpinner { From 0b8196343f008d9fd2e22cd54c2c3be85f7c5c8c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 15 Mar 2019 09:57:26 +0100 Subject: [PATCH 10/52] fix some tests --- test/components/structures/MessagePanel-test.js | 9 ++++++++- test/components/structures/ScrollPanel-test.js | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 0a51cb89186..c7287f15236 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -21,6 +21,7 @@ const ReactDOM = require("react-dom"); const TestUtils = require('react-addons-test-utils'); const expect = require('expect'); import sinon from 'sinon'; +import { EventEmitter } from "events"; const sdk = require('matrix-react-sdk'); @@ -46,8 +47,14 @@ const WrappedMessagePanel = React.createClass({ }; }, + getInitialState: function() { + return { + resizeNotifier: new EventEmitter(), + }; + }, + render: function() { - return ; + return ; }, }); diff --git a/test/components/structures/ScrollPanel-test.js b/test/components/structures/ScrollPanel-test.js index 0e091cdddf6..41d0f4469b1 100644 --- a/test/components/structures/ScrollPanel-test.js +++ b/test/components/structures/ScrollPanel-test.js @@ -19,6 +19,7 @@ const ReactDOM = require("react-dom"); const ReactTestUtils = require('react-addons-test-utils'); const expect = require('expect'); import Promise from 'bluebird'; +import { EventEmitter } from "events"; const sdk = require('matrix-react-sdk'); @@ -29,6 +30,7 @@ const Tester = React.createClass({ getInitialState: function() { return { tileKeys: [], + resizeNotifier: new EventEmitter(), }; }, @@ -130,7 +132,8 @@ const Tester = React.createClass({ return ( + onFillRequest={this._onFillRequest} + resizeNotifier={this.state.resizeNotifier}> { tiles } ); From 30d848b86e514066888f865fff049a266aa60e0d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:30:06 +0100 Subject: [PATCH 11/52] use AutoHideScrollbar in ScrollPanel --- res/css/structures/_RoomView.scss | 2 + .../structures/AutoHideScrollbar.js | 1 + src/components/structures/ScrollPanel.js | 40 +++++++++---------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/res/css/structures/_RoomView.scss b/res/css/structures/_RoomView.scss index f15552e484a..1b639928e02 100644 --- a/res/css/structures/_RoomView.scss +++ b/res/css/structures/_RoomView.scss @@ -84,6 +84,7 @@ limitations under the License. display: flex; flex-direction: column; flex: 1; + min-width: 0; } .mx_RoomView_body .mx_RoomView_timeline { @@ -111,6 +112,7 @@ limitations under the License. .mx_RoomView_messagePanel { width: 100%; overflow-y: auto; + flex: 1 1 0; } .mx_RoomView_messagePanelSearchSpinner { diff --git a/src/components/structures/AutoHideScrollbar.js b/src/components/structures/AutoHideScrollbar.js index 0f93f204078..72d48a20841 100644 --- a/src/components/structures/AutoHideScrollbar.js +++ b/src/components/structures/AutoHideScrollbar.js @@ -121,6 +121,7 @@ export default class AutoHideScrollbar extends React.Component { render() { return (
    diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index ee4045c91e4..cdb79686adc 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -15,14 +15,13 @@ limitations under the License. */ const React = require("react"); -const ReactDOM = require("react-dom"); import PropTypes from 'prop-types'; import Promise from 'bluebird'; import { KeyCode } from '../../Keyboard'; -import sdk from '../../index.js'; +import AutoHideScrollbar from "./AutoHideScrollbar"; const DEBUG_SCROLL = false; -// var DEBUG_SCROLL = true; +// const DEBUG_SCROLL = true; // The amount of extra scroll distance to allow prior to unfilling. // See _getExcessHeight. @@ -129,11 +128,6 @@ module.exports = React.createClass({ */ onScroll: PropTypes.func, - /* onResize: a callback which is called whenever the Gemini scroll - * panel is resized - */ - onResize: PropTypes.func, - /* className: classnames to add to the top-level div */ className: PropTypes.string, @@ -150,7 +144,6 @@ module.exports = React.createClass({ onFillRequest: function(backwards) { return Promise.resolve(false); }, onUnfillRequest: function(backwards, scrollToken) {}, onScroll: function() {}, - onResize: function() {}, }; }, @@ -185,6 +178,16 @@ module.exports = React.createClass({ debuglog("Scroll event: offset now:", sn.scrollTop, "_lastSetScroll:", this._lastSetScroll); + // ignore scroll events where scrollTop hasn't changed, + // appears to happen when the layout changes outside + // of the scroll container, like resizing the right panel. + if (sn.scrollTop === this._lastEventScroll) { + debuglog("ignore scroll event with same scrollTop as before"); + return; + } + + this._lastEventScroll = sn.scrollTop; + // 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). @@ -225,9 +228,7 @@ module.exports = React.createClass({ onResize: function() { this.clearBlockShrinking(); - this.props.onResize(); this.checkScroll(); - if (this._gemScroll) this._gemScroll.forceUpdate(); }, // after an update to the contents of the panel, check that the scroll is @@ -672,17 +673,17 @@ module.exports = React.createClass({ throw new Error("ScrollPanel._getScrollNode called when unmounted"); } - if (!this._gemScroll) { + if (!this._divScroll) { // Likewise, we should have the ref by this point, but if not // turn the NPE into something meaningful. throw new Error("ScrollPanel._getScrollNode called before gemini ref collected"); } - return this._gemScroll.scrollbar.getViewElement(); + return this._divScroll; }, - _collectGeminiScroll: function(gemScroll) { - this._gemScroll = gemScroll; + _collectScroll: function(divScroll) { + this._divScroll = divScroll; }, /** @@ -724,19 +725,18 @@ module.exports = React.createClass({ }, render: function() { - const GeminiScrollbarWrapper = sdk.getComponent("elements.GeminiScrollbarWrapper"); // TODO: the classnames on the div and ol could do with being updated to // reflect the fact that we don't necessarily contain a list of messages. // it's not obvious why we have a separate div and ol anyway. - return (
      { this.props.children }
    -     - ); + + ); }, }); From f71a9f10dd7f7420c1f5d3fd3100057139772ba0 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:33:05 +0100 Subject: [PATCH 12/52] remove onChildResize in RoomView as it's unused --- src/components/structures/RoomView.js | 15 ++------------- src/components/views/rooms/MessageComposer.js | 5 ----- .../views/rooms/MessageComposerInput.js | 4 ---- 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 34c711ee6f1..ae3a4b2bacd 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -1364,8 +1364,7 @@ module.exports = React.createClass({ const showBar = this.refs.messagePanel.canJumpToReadMarker(); if (this.state.showTopUnreadMessagesBar != showBar) { - this.setState({showTopUnreadMessagesBar: showBar}, - this.onChildResize); + this.setState({showTopUnreadMessagesBar: showBar}); } }, @@ -1408,7 +1407,7 @@ module.exports = React.createClass({ }; }, - onResize: function(e) { + onResize: function() { // It seems flexbox doesn't give us a way to constrain the auxPanel height to have // a minimum of the height of the video element, whilst also capping it from pushing out the page // so we have to do it via JS instead. In this implementation we cap the height by putting @@ -1426,9 +1425,6 @@ module.exports = React.createClass({ if (auxPanelMaxHeight < 50) auxPanelMaxHeight = 50; this.setState({auxPanelMaxHeight: auxPanelMaxHeight}); - - // changing the maxHeight on the auxpanel will trigger a callback go - // onChildResize, so no need to worry about that here. }, onFullscreenClick: function() { @@ -1458,10 +1454,6 @@ module.exports = React.createClass({ this.forceUpdate(); // TODO: just update the voip buttons }, - onChildResize: function() { - // no longer anything to do here - }, - onStatusBarVisible: function() { if (this.unmounted) return; this.setState({ @@ -1654,7 +1646,6 @@ module.exports = React.createClass({ isPeeking={myMembership !== "join"} onInviteClick={this.onInviteButtonClick} onStopWarningClick={this.onStopAloneWarningClick} - onResize={this.onChildResize} onVisible={this.onStatusBarVisible} onHidden={this.onStatusBarHidden} />; @@ -1723,7 +1714,6 @@ module.exports = React.createClass({ draggingFile={this.state.draggingFile} displayConfCallNotification={this.state.displayConfCallNotification} maxHeight={this.state.auxPanelMaxHeight} - onResize={this.onChildResize} showApps={this.state.showApps} hideAppsDrawer={false} > { aux } @@ -1739,7 +1729,6 @@ module.exports = React.createClass({ messageComposer = this.messageComposerInput = c} key="controls_input" - onResize={this.props.onResize} room={this.props.room} placeholder={placeholderText} onFilesPasted={this.uploadFiles} @@ -505,10 +504,6 @@ export default class MessageComposer extends React.Component { } MessageComposer.propTypes = { - // a callback which is called when the height of the composer is - // changed due to a change in content. - onResize: PropTypes.func, - // js-sdk Room object room: PropTypes.object.isRequired, diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 6b80902c8f8..cbea2bccb9a 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -135,10 +135,6 @@ function rangeEquals(a: Range, b: Range): boolean { */ export default class MessageComposerInput extends React.Component { static propTypes = { - // a callback which is called when the height of the composer is - // changed due to a change in content. - onResize: PropTypes.func, - // js-sdk Room object room: PropTypes.object.isRequired, From 891e343df624be7e1b22f858a6894110cb7e7525 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:36:12 +0100 Subject: [PATCH 13/52] create ResizeNotifier to derive which areas of the app resize and emit --- src/components/structures/FilePanel.js | 1 + src/components/structures/LoggedInView.js | 2 + src/components/structures/MainSplit.js | 1 + src/components/structures/MatrixChat.js | 3 ++ src/components/structures/MessagePanel.js | 3 +- src/components/structures/RightPanel.js | 2 +- src/components/structures/RoomView.js | 13 ++++-- src/components/structures/ScrollPanel.js | 3 ++ src/components/structures/TimelinePanel.js | 1 + src/utils/ResizeNotifier.js | 52 ++++++++++++++++++++++ 10 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 src/utils/ResizeNotifier.js diff --git a/src/components/structures/FilePanel.js b/src/components/structures/FilePanel.js index 927449750c9..e35a39a1073 100644 --- a/src/components/structures/FilePanel.js +++ b/src/components/structures/FilePanel.js @@ -123,6 +123,7 @@ const FilePanel = React.createClass({ timelineSet={this.state.timelineSet} showUrlPreview = {false} tileShape="file_grid" + resizeNotifier={this.props.resizeNotifier} empty={_t('There are no visible files in this room')} /> ); diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index c6c1be67ec5..5267dba715d 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -173,6 +173,7 @@ const LoggedInView = React.createClass({ }, onResized: (size) => { window.localStorage.setItem("mx_lhs_size", '' + size); + this.props.resizeNotifier.notifyLeftHandleResized(); }, }; const resizer = new Resizer( @@ -448,6 +449,7 @@ const LoggedInView = React.createClass({ disabled={this.props.middleDisabled} collapsedRhs={this.props.collapsedRhs} ConferenceHandler={this.props.ConferenceHandler} + resizeNotifier={this.props.resizeNotifier} />; break; diff --git a/src/components/structures/MainSplit.js b/src/components/structures/MainSplit.js index 0427130eeac..c0bf74d02c7 100644 --- a/src/components/structures/MainSplit.js +++ b/src/components/structures/MainSplit.js @@ -27,6 +27,7 @@ export default class MainSplit extends React.Component { _onResized(size) { window.localStorage.setItem("mx_rhs_size", size); + this.props.resizeNotifier.notifyRightHandleResized(); } _createResizer() { diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 2622a6bf93b..a9b34c9058a 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -48,6 +48,7 @@ import { _t, getCurrentLanguage } from '../../languageHandler'; import SettingsStore, {SettingLevel} from "../../settings/SettingsStore"; import { startAnyRegistrationFlow } from "../../Registration.js"; import { messageForSyncError } from '../../utils/ErrorUtils'; +import ResizeNotifier from "../../utils/ResizeNotifier"; const AutoDiscovery = Matrix.AutoDiscovery; @@ -194,6 +195,7 @@ export default React.createClass({ hideToSRUsers: false, syncError: null, // If the current syncing status is ERROR, the error object, otherwise null. + resizeNotifier: new ResizeNotifier(), }; return s; }, @@ -1661,6 +1663,7 @@ export default React.createClass({ dis.dispatch({ action: 'show_right_panel' }); } + this.state.resizeNotifier.notifyWindowResized(); this._windowWidth = window.innerWidth; }, diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index b1f88a62212..aec2f8cbe17 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -703,7 +703,8 @@ module.exports = React.createClass({ onFillRequest={this.props.onFillRequest} onUnfillRequest={this.props.onUnfillRequest} style={style} - stickyBottom={this.props.stickyBottom}> + stickyBottom={this.props.stickyBottom} + resizeNotifier={this.props.resizeNotifier}> { topSpinner } { this._getEventTiles() } { whoIsTyping } diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index 5c745b04cc2..93cbff32330 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -193,7 +193,7 @@ export default class RightPanel extends React.Component { } else if (this.state.phase === RightPanel.Phase.NotificationPanel) { panel = ; } else if (this.state.phase === RightPanel.Phase.FilePanel) { - panel = ; + panel = ; } const classes = classNames("mx_RightPanel", "mx_fadable", { diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index ae3a4b2bacd..bb2c322b0a9 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -392,7 +392,7 @@ module.exports = React.createClass({ this._updateConfCallNotification(); window.addEventListener('beforeunload', this.onPageUnload); - window.addEventListener('resize', this.onResize); + this.props.resizeNotifier.on("middlePanelResized", this.onResize); this.onResize(); document.addEventListener("keydown", this.onKeyDown); @@ -472,7 +472,7 @@ module.exports = React.createClass({ } window.removeEventListener('beforeunload', this.onPageUnload); - window.removeEventListener('resize', this.onResize); + this.props.resizeNotifier.removeListener("middlePanelResized", this.onResize); document.removeEventListener("keydown", this.onKeyDown); @@ -1838,6 +1838,7 @@ module.exports = React.createClass({ className="mx_RoomView_messagePanel" membersLoaded={this.state.membersLoaded} permalinkCreator={this.state.permalinkCreator} + resizeNotifier={this.props.resizeNotifier} />); let topUnreadMessagesBar = null; @@ -1870,7 +1871,7 @@ module.exports = React.createClass({ }, ); - const rightPanel = this.state.room ? : undefined; + const rightPanel = this.state.room ? : undefined; return (
    @@ -1886,7 +1887,11 @@ module.exports = React.createClass({ onLeaveClick={(myMembership === "join") ? this.onLeaveClick : null} e2eStatus={this.state.e2eStatus} /> - +
    { auxPanel }
    diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index cdb79686adc..799c88140e2 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -149,6 +149,8 @@ module.exports = React.createClass({ componentWillMount: function() { this._pendingFillRequests = {b: null, f: null}; + this.props.resizeNotifier.on("middlePanelResized", this.onResize); + this.resetScrollState(); }, @@ -171,6 +173,7 @@ module.exports = React.createClass({ // // (We could use isMounted(), but facebook have deprecated that.) this.unmounted = true; + this.props.resizeNotifier.removeListener("middlePanelResized", this.onResize); }, onScroll: function(ev) { diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index f0feaf94c5b..c983f904a01 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -1228,6 +1228,7 @@ var TimelinePanel = React.createClass({ alwaysShowTimestamps={this.state.alwaysShowTimestamps} className={this.props.className} tileShape={this.props.tileShape} + resizeNotifier={this.props.resizeNotifier} /> ); }, diff --git a/src/utils/ResizeNotifier.js b/src/utils/ResizeNotifier.js new file mode 100644 index 00000000000..43578ebcaa8 --- /dev/null +++ b/src/utils/ResizeNotifier.js @@ -0,0 +1,52 @@ +/* +Copyright 2019 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/** + * Fires when the middle panel has been resized. + * @event module:utils~ResizeNotifier#"middlePanelResized" + */ +import { EventEmitter } from "events"; +import { throttle } from "lodash"; + +export default class ResizeNotifier extends EventEmitter { + + constructor() { + super(); + // with default options, will call fn once at first call, and then every x ms + // if there was another call in that timespan + this._throttledMiddlePanel = throttle(() => this.emit("middlePanelResized"), 200); + } + + notifyBannersChanged() { + this.emit("middlePanelResized"); + } + + // can be called in quick succession + notifyLeftHandleResized() { + this._throttledMiddlePanel(); + } + + // can be called in quick succession + notifyRightHandleResized() { + this._throttledMiddlePanel(); + } + + // can be called in quick succession + notifyWindowResized() { + this._throttledMiddlePanel(); + } +} + From 1bdbf3086f04579d7ec860e8f95605168c3e7bf7 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:37:20 +0100 Subject: [PATCH 14/52] emit timeline_resize in MatrixChat based on ResizeNotifier as it's used in PersistentElement which could be used at various places --- src/components/structures/MatrixChat.js | 8 ++++++++ src/components/structures/MessagePanel.js | 5 ----- src/components/structures/RoomView.js | 5 ----- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index a9b34c9058a..6e05138e2d3 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -318,6 +318,9 @@ export default React.createClass({ // N.B. we don't call the whole of setTheme() here as we may be // racing with the theme CSS download finishing from index.js Tinter.tint(); + + // For PersistentElement + this.state.resizeNotifier.on("middlePanelResized", this._dispatchTimelineResize); }, componentDidMount: function() { @@ -400,6 +403,7 @@ export default React.createClass({ dis.unregister(this.dispatcherRef); window.removeEventListener("focus", this.onFocus); window.removeEventListener('resize', this.handleResize); + this.state.resizeNotifier.removeListener("middlePanelResized", this._dispatchTimelineResize); }, componentWillUpdate: function(props, state) { @@ -1667,6 +1671,10 @@ export default React.createClass({ this._windowWidth = window.innerWidth; }, + _dispatchTimelineResize() { + dis.dispatch({ action: 'timeline_resize' }, true); + }, + onRoomCreated: function(roomId) { dis.dispatch({ action: "view_room", diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index aec2f8cbe17..fecf5f1ad7d 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -21,7 +21,6 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import shouldHideEvent from '../../shouldHideEvent'; import {wantsDateSeparator} from '../../DateUtils'; -import dis from "../../dispatcher"; import sdk from '../../index'; import MatrixClientPeg from '../../MatrixClientPeg'; @@ -665,10 +664,6 @@ module.exports = React.createClass({ } }, - onResize: function() { - dis.dispatch({ action: 'timeline_resize' }, true); - }, - render: function() { const ScrollPanel = sdk.getComponent("structures.ScrollPanel"); const WhoIsTypingTile = sdk.getComponent("rooms.WhoIsTypingTile"); diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index bb2c322b0a9..d8b4fbb0f1e 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -865,10 +865,6 @@ module.exports = React.createClass({ } }, - onSearchResultsResize: function() { - dis.dispatch({ action: 'timeline_resize' }, true); - }, - onSearchResultsFillRequest: function(backwards) { if (!backwards) { return Promise.resolve(false); @@ -1803,7 +1799,6 @@ module.exports = React.createClass({
  • { this.getSearchResultTiles() } From dad382a4b750bedd1ba6349f23aa701909348f85 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 16:38:30 +0100 Subject: [PATCH 15/52] use AutoHideScrollbar in memberlist --- res/css/views/rooms/_MemberList.scss | 5 +++++ src/components/views/rooms/MemberList.js | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/res/css/views/rooms/_MemberList.scss b/res/css/views/rooms/_MemberList.scss index 9f2b5da930f..cac97cb60d1 100644 --- a/res/css/views/rooms/_MemberList.scss +++ b/res/css/views/rooms/_MemberList.scss @@ -20,6 +20,7 @@ limitations under the License. flex: 1; display: flex; flex-direction: column; + min-height: 0; .mx_Spinner { flex: 1 0 auto; @@ -35,6 +36,10 @@ limitations under the License. margin-top: 8px; margin-bottom: 4px; } + + .mx_AutoHideScrollbar { + flex: 1 1 0; + } } .mx_MemberList_chevron { diff --git a/src/components/views/rooms/MemberList.js b/src/components/views/rooms/MemberList.js index f9ce672c167..29f49f1691c 100644 --- a/src/components/views/rooms/MemberList.js +++ b/src/components/views/rooms/MemberList.js @@ -20,6 +20,8 @@ import React from 'react'; import { _t } from '../../../languageHandler'; import SdkConfig from '../../../SdkConfig'; import dis from '../../../dispatcher'; +import AutoHideScrollbar from "../../structures/AutoHideScrollbar"; + const MatrixClientPeg = require("../../../MatrixClientPeg"); const sdk = require('../../../index'); const rate_limited_func = require('../../../ratelimitedfunc'); @@ -439,7 +441,6 @@ module.exports = React.createClass({ const SearchBox = sdk.getComponent('structures.SearchBox'); const TruncatedList = sdk.getComponent("elements.TruncatedList"); - const GeminiScrollbarWrapper = sdk.getComponent("elements.GeminiScrollbarWrapper"); const cli = MatrixClientPeg.get(); const room = cli.getRoom(this.props.roomId); @@ -466,7 +467,7 @@ module.exports = React.createClass({ return (
    { inviteButton } - +
    - + Date: Tue, 12 Mar 2019 16:38:42 +0100 Subject: [PATCH 16/52] use AutoHideScrollbar in member info panel --- src/components/views/rooms/MemberInfo.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/MemberInfo.js b/src/components/views/rooms/MemberInfo.js index 3ada730ec84..35161dedf7d 100644 --- a/src/components/views/rooms/MemberInfo.js +++ b/src/components/views/rooms/MemberInfo.js @@ -44,6 +44,7 @@ import SdkConfig from '../../../SdkConfig'; import MultiInviter from "../../../utils/MultiInviter"; import SettingsStore from "../../../settings/SettingsStore"; import E2EIcon from "./E2EIcon"; +import AutoHideScrollbar from "../../structures/AutoHideScrollbar"; module.exports = withMatrixClient(React.createClass({ displayName: 'MemberInfo', @@ -1003,7 +1004,7 @@ module.exports = withMatrixClient(React.createClass({ { roomMemberDetails }
    - +
    { this._renderUserOptions() } @@ -1015,7 +1016,7 @@ module.exports = withMatrixClient(React.createClass({ { spinner }
    -     +
    ); }, From 4795625cee23c73864efc7bb32b93548a551529f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 17:29:16 +0100 Subject: [PATCH 17/52] emit resize event when banners are shown/hidden to restore scroll pos --- src/Notifier.js | 12 +++++++++++- src/components/structures/LoggedInView.js | 19 +++++++++++++------ src/components/structures/MatrixChat.js | 13 ++++++++++--- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/Notifier.js b/src/Notifier.js index 80e8be10846..ab8009c4579 100644 --- a/src/Notifier.js +++ b/src/Notifier.js @@ -220,7 +220,17 @@ const Notifier = { } }, - isToolbarHidden: function() { + shouldShowToolbar: function() { + const client = MatrixClientPeg.get(); + if (!client) { + return false; + } + const isGuest = client.isGuest(); + return !isGuest && Notifier.supportsDesktopNotifications() && + !Notifier.isEnabled() && !Notifier._isToolbarHidden(); + }, + + _isToolbarHidden: function() { // Check localStorage for any such meta data if (global.localStorage) { return global.localStorage.getItem("notifications_hidden") === "true"; diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index 5267dba715d..c22c217e5fb 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -22,7 +22,6 @@ import PropTypes from 'prop-types'; import { DragDropContext } from 'react-beautiful-dnd'; import { KeyCode, isOnlyCtrlOrCmdKeyEvent } from '../../Keyboard'; -import Notifier from '../../Notifier'; import PageTypes from '../../PageTypes'; import CallMediaHandler from '../../CallMediaHandler'; import sdk from '../../index'; @@ -121,6 +120,18 @@ const LoggedInView = React.createClass({ this._matrixClient.on("RoomState.events", this.onRoomStateEvents); }, + componentDidUpdate(prevProps) { + // attempt to guess when a banner was opened or closed + if ( + (prevProps.showCookieBar !== this.props.showCookieBar) || + (prevProps.hasNewVersion !== this.props.hasNewVersion) || + (prevProps.userHasGeneratedPassword !== this.props.userHasGeneratedPassword) || + (prevProps.showNotifierToolbar !== this.props.showNotifierToolbar) + ) { + this.props.resizeNotifier.notifyBannersChanged(); + } + }, + componentWillUnmount: function() { document.removeEventListener('keydown', this._onKeyDown); this._matrixClient.removeListener("accountData", this.onAccountData); @@ -491,7 +502,6 @@ const LoggedInView = React.createClass({ }); let topBar; - const isGuest = this.props.matrixClient.isGuest(); if (this.state.syncErrorData && this.state.syncErrorData.error.errcode === 'M_RESOURCE_LIMIT_EXCEEDED') { topBar = ; } else if (this.state.userHasGeneratedPassword) { topBar = ; - } else if ( - !isGuest && Notifier.supportsDesktopNotifications() && - !Notifier.isEnabled() && !Notifier.isToolbarHidden() - ) { + } else if (this.props.showNotifierToolbar) { topBar = ; } diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 6e05138e2d3..a7192b96cbf 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -29,6 +29,7 @@ import PlatformPeg from "../../PlatformPeg"; import SdkConfig from "../../SdkConfig"; import * as RoomListSorter from "../../RoomListSorter"; import dis from "../../dispatcher"; +import Notifier from '../../Notifier'; import Modal from "../../Modal"; import Tinter from "../../Tinter"; @@ -196,6 +197,7 @@ export default React.createClass({ syncError: null, // If the current syncing status is ERROR, the error object, otherwise null. resizeNotifier: new ResizeNotifier(), + showNotifierToolbar: Notifier.shouldShowToolbar(), }; return s; }, @@ -644,8 +646,9 @@ export default React.createClass({ case 'view_invite': showRoomInviteDialog(payload.roomId); break; - case 'notifier_enabled': - this.forceUpdate(); + case 'notifier_enabled': { + this.setState({showNotifierToolbar: Notifier.shouldShowToolbar()}); + } break; case 'hide_left_panel': this.setState({ @@ -1180,6 +1183,7 @@ export default React.createClass({ */ _onLoggedIn: async function() { this.setStateForNewView({view: VIEWS.LOGGED_IN}); + this.setState({showNotifierToolbar: Notifier.shouldShowToolbar()}); if (this._is_registered) { this._is_registered = false; @@ -1672,7 +1676,10 @@ export default React.createClass({ }, _dispatchTimelineResize() { - dis.dispatch({ action: 'timeline_resize' }, true); + // prevent dispatch from within dispatch error + setTimeout(() => { + dis.dispatch({ action: 'timeline_resize' }, true); + }, 0); }, onRoomCreated: function(roomId) { From bab2730d405b3fdde49891c2843c7810514a89de Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 12 Mar 2019 18:00:05 +0100 Subject: [PATCH 18/52] use ResizeNotifier as well to relayout room list --- src/components/structures/LeftPanel.js | 2 +- src/components/structures/LoggedInView.js | 2 +- src/components/views/rooms/RoomList.js | 10 ++++++---- src/utils/ResizeNotifier.js | 8 ++++++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/components/structures/LeftPanel.js b/src/components/structures/LeftPanel.js index 21438c597cd..95b57a0ca57 100644 --- a/src/components/structures/LeftPanel.js +++ b/src/components/structures/LeftPanel.js @@ -234,7 +234,7 @@ const LeftPanel = React.createClass({ diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index c22c217e5fb..4771c6f4872 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -543,7 +543,7 @@ const LoggedInView = React.createClass({
    diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index 227dd318edf..2de9918e6e3 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -212,7 +212,7 @@ module.exports = React.createClass({ this._checkSubListsOverflow(); this.resizer.attach(); - window.addEventListener("resize", this.onWindowResize); + this.props.resizeNotifier.on("leftPanelResized", this.onResize); this.mounted = true; }, @@ -260,7 +260,6 @@ module.exports = React.createClass({ componentWillUnmount: function() { this.mounted = false; - window.removeEventListener("resize", this.onWindowResize); dis.unregister(this.dispatcherRef); if (MatrixClientPeg.get()) { MatrixClientPeg.get().removeListener("Room", this.onRoom); @@ -272,6 +271,8 @@ module.exports = React.createClass({ MatrixClientPeg.get().removeListener("Group.myMembership", this._onGroupMyMembership); MatrixClientPeg.get().removeListener("RoomState.events", this.onRoomStateEvents); } + this.props.resizeNotifier.removeListener("leftPanelResized", this.onResize); + if (this._tagStoreToken) { this._tagStoreToken.remove(); @@ -293,13 +294,14 @@ module.exports = React.createClass({ this._delayedRefreshRoomList.cancelPendingCall(); }, - onWindowResize: function() { + + onResize: function() { if (this.mounted && this._layout && this.resizeContainer && Array.isArray(this._layoutSections) ) { this._layout.update( this._layoutSections, - this.resizeContainer.offsetHeight + this.resizeContainer.offsetHeight, ); } }, diff --git a/src/utils/ResizeNotifier.js b/src/utils/ResizeNotifier.js index 43578ebcaa8..ff4b79091be 100644 --- a/src/utils/ResizeNotifier.js +++ b/src/utils/ResizeNotifier.js @@ -31,11 +31,13 @@ export default class ResizeNotifier extends EventEmitter { } notifyBannersChanged() { + this.emit("leftPanelResized"); this.emit("middlePanelResized"); } // can be called in quick succession notifyLeftHandleResized() { + // don't emit event for own region this._throttledMiddlePanel(); } @@ -46,6 +48,12 @@ export default class ResizeNotifier extends EventEmitter { // can be called in quick succession notifyWindowResized() { + // no need to throttle this one, + // also it could make scrollbars appear for + // a split second when the room list manual layout is now + // taller than the available space + this.emit("leftPanelResized"); + this._throttledMiddlePanel(); } } From 987a1a00b38d3895e62c7efbcde2d9b505571bdb Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 14 Mar 2019 15:04:09 +0100 Subject: [PATCH 19/52] chrome apparently anchors the scroll position, which fights against our restore position logic. Disable it like this. --- res/css/structures/_RoomView.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/res/css/structures/_RoomView.scss b/res/css/structures/_RoomView.scss index 1b639928e02..ced0b9eab36 100644 --- a/res/css/structures/_RoomView.scss +++ b/res/css/structures/_RoomView.scss @@ -113,6 +113,7 @@ limitations under the License. width: 100%; overflow-y: auto; flex: 1 1 0; + overflow-anchor: none; } .mx_RoomView_messagePanelSearchSpinner { From 95e61a57bc6a8a49e7e9e27b86bf45ada629b05c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 15 Mar 2019 09:57:26 +0100 Subject: [PATCH 20/52] fix some tests --- test/components/structures/MessagePanel-test.js | 9 ++++++++- test/components/structures/ScrollPanel-test.js | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 0a51cb89186..c7287f15236 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -21,6 +21,7 @@ const ReactDOM = require("react-dom"); const TestUtils = require('react-addons-test-utils'); const expect = require('expect'); import sinon from 'sinon'; +import { EventEmitter } from "events"; const sdk = require('matrix-react-sdk'); @@ -46,8 +47,14 @@ const WrappedMessagePanel = React.createClass({ }; }, + getInitialState: function() { + return { + resizeNotifier: new EventEmitter(), + }; + }, + render: function() { - return ; + return ; }, }); diff --git a/test/components/structures/ScrollPanel-test.js b/test/components/structures/ScrollPanel-test.js index 0e091cdddf6..41d0f4469b1 100644 --- a/test/components/structures/ScrollPanel-test.js +++ b/test/components/structures/ScrollPanel-test.js @@ -19,6 +19,7 @@ const ReactDOM = require("react-dom"); const ReactTestUtils = require('react-addons-test-utils'); const expect = require('expect'); import Promise from 'bluebird'; +import { EventEmitter } from "events"; const sdk = require('matrix-react-sdk'); @@ -29,6 +30,7 @@ const Tester = React.createClass({ getInitialState: function() { return { tileKeys: [], + resizeNotifier: new EventEmitter(), }; }, @@ -130,7 +132,8 @@ const Tester = React.createClass({ return ( + onFillRequest={this._onFillRequest} + resizeNotifier={this.state.resizeNotifier}> { tiles } ); From d84003ac44e0ed1e4a1bdb9d647d500e1cf6a4db Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 18 Mar 2019 14:05:56 +0100 Subject: [PATCH 21/52] dont break room directory and search --- src/components/structures/RoomDirectory.js | 1 - src/components/structures/RoomView.js | 1 + src/components/structures/ScrollPanel.js | 13 +++++++++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/components/structures/RoomDirectory.js b/src/components/structures/RoomDirectory.js index e13eab8eb3a..59b4cbe2e62 100644 --- a/src/components/structures/RoomDirectory.js +++ b/src/components/structures/RoomDirectory.js @@ -551,7 +551,6 @@ module.exports = React.createClass({ onFillRequest={ this.onFillRequest } stickyBottom={false} startAtBottom={false} - onResize={function() {}} > { scrollpanel_content } ; diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index d8b4fbb0f1e..ba6b54bdbca 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -1799,6 +1799,7 @@ module.exports = React.createClass({
  • { this.getSearchResultTiles() } diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 799c88140e2..b4325a173ab 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -135,6 +135,9 @@ module.exports = React.createClass({ /* style: styles to add to the top-level div */ style: PropTypes.object, + /* resizeNotifier: ResizeNotifier to know when middle column has changed size + */ + resizeNotifier: PropTypes.object, }, getDefaultProps: function() { @@ -149,7 +152,10 @@ module.exports = React.createClass({ componentWillMount: function() { this._pendingFillRequests = {b: null, f: null}; - this.props.resizeNotifier.on("middlePanelResized", this.onResize); + + if (this.props.resizeNotifier) { + this.props.resizeNotifier.on("middlePanelResized", this.onResize); + } this.resetScrollState(); }, @@ -173,7 +179,10 @@ module.exports = React.createClass({ // // (We could use isMounted(), but facebook have deprecated that.) this.unmounted = true; - this.props.resizeNotifier.removeListener("middlePanelResized", this.onResize); + + if (this.props.resizeNotifier) { + this.props.resizeNotifier.removeListener("middlePanelResized", this.onResize); + } }, onScroll: function(ev) { From 71f6b08b265697a716b0cef3a5b1d9d974e84325 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 19 Mar 2019 13:42:22 +0100 Subject: [PATCH 22/52] first impl of new scrolling, still a bit broken --- res/css/_components.scss | 1 + res/css/structures/_ScrollPanel.scss | 26 +++ src/components/structures/ScrollPanel.js | 281 ++++++++++++----------- 3 files changed, 173 insertions(+), 135 deletions(-) create mode 100644 res/css/structures/_ScrollPanel.scss diff --git a/res/css/_components.scss b/res/css/_components.scss index 4fb0eed4afc..69f4730d857 100644 --- a/res/css/_components.scss +++ b/res/css/_components.scss @@ -19,6 +19,7 @@ @import "./structures/_RoomStatusBar.scss"; @import "./structures/_RoomSubList.scss"; @import "./structures/_RoomView.scss"; +@import "./structures/_ScrollPanel.scss"; @import "./structures/_SearchBox.scss"; @import "./structures/_TabbedView.scss"; @import "./structures/_TagPanel.scss"; diff --git a/res/css/structures/_ScrollPanel.scss b/res/css/structures/_ScrollPanel.scss new file mode 100644 index 00000000000..699224949b4 --- /dev/null +++ b/res/css/structures/_ScrollPanel.scss @@ -0,0 +1,26 @@ +/* +Copyright 2019 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +.mx_ScrollPanel { + + .mx_RoomView_MessageList { + position: relative; + display: flex; + flex-direction: column; + justify-content: flex-end; + overflow-y: hidden; + } +} diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index b4325a173ab..9d0bfce6cb1 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -18,6 +18,7 @@ const React = require("react"); import PropTypes from 'prop-types'; import Promise from 'bluebird'; import { KeyCode } from '../../Keyboard'; +import Timer from '../../utils/Timer'; import AutoHideScrollbar from "./AutoHideScrollbar"; const DEBUG_SCROLL = false; @@ -30,11 +31,14 @@ const UNPAGINATION_PADDING = 6000; // many scroll events causing many unfilling requests. const UNFILL_REQUEST_DEBOUNCE_MS = 200; +const PAGE_SIZE = 200; + +let debuglog; if (DEBUG_SCROLL) { // using bind means that we get to keep useful line numbers in the console - var debuglog = console.log.bind(console); + debuglog = console.log.bind(console, "ScrollPanel debuglog:"); } else { - var debuglog = function() {}; + debuglog = function() {}; } /* This component implements an intelligent scrolling list. @@ -186,56 +190,12 @@ module.exports = React.createClass({ }, onScroll: function(ev) { - const sn = this._getScrollNode(); - debuglog("Scroll event: offset now:", sn.scrollTop, - "_lastSetScroll:", this._lastSetScroll); - - // ignore scroll events where scrollTop hasn't changed, - // appears to happen when the layout changes outside - // of the scroll container, like resizing the right panel. - if (sn.scrollTop === this._lastEventScroll) { - debuglog("ignore scroll event with same scrollTop as before"); - return; - } - - this._lastEventScroll = sn.scrollTop; - - // 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) { - 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._scrollTimeout.restart(); + this._saveScrollState(); this._checkBlockShrinking(); + this.checkFillState(); this.props.onScroll(ev); - - this.checkFillState(); }, onResize: function() { @@ -258,14 +218,7 @@ module.exports = React.createClass({ // whether it will stay that way when the children update. isAtBottom: function() { const sn = this._getScrollNode(); - - // there seems to be some bug with flexbox/gemini/chrome/richvdh's - // understanding of the box model, wherein the scrollNode ends up 2 - // pixels higher than the available space, even when there are less - // than a screenful of messages. + 3 is a fudge factor to pretend - // that we're at the bottom when we're still a few pixels off. - - return sn.scrollHeight - Math.ceil(sn.scrollTop) <= sn.clientHeight + 3; + return sn.scrollTop === sn.scrollHeight - sn.clientHeight; }, // returns the vertical height in the given direction that can be removed from @@ -301,10 +254,15 @@ module.exports = React.createClass({ // `---------' - _getExcessHeight: function(backwards) { const sn = this._getScrollNode(); + const contentHeight = this._getMessagesHeight(); + const listHeight = this._getListHeight(); + const clippedHeight = contentHeight - listHeight; + const unclippedScrollTop = sn.scrollTop + clippedHeight; + if (backwards) { - return sn.scrollTop - sn.clientHeight - UNPAGINATION_PADDING; + return unclippedScrollTop - sn.clientHeight - UNPAGINATION_PADDING; } else { - return sn.scrollHeight - (sn.scrollTop + 2*sn.clientHeight) - UNPAGINATION_PADDING; + return contentHeight - (unclippedScrollTop + 2*sn.clientHeight) - UNPAGINATION_PADDING; } }, @@ -356,6 +314,9 @@ module.exports = React.createClass({ if (excessHeight <= 0) { return; } + + const origExcessHeight = excessHeight; + const tiles = this.refs.itemlist.children; // The scroll token of the first/last tile to be unpaginated @@ -367,8 +328,9 @@ module.exports = React.createClass({ // pagination. // // If backwards is true, we unpaginate (remove) tiles from the back (top). + let tile; for (let i = 0; i < tiles.length; i++) { - const tile = tiles[backwards ? i : tiles.length - 1 - i]; + tile = tiles[backwards ? i : tiles.length - 1 - i]; // Subtract height of tile as if it were unpaginated excessHeight -= tile.clientHeight; //If removing the tile would lead to future pagination, break before setting scroll token @@ -380,6 +342,7 @@ module.exports = React.createClass({ markerScrollToken = tile.dataset.scrollTokens.split(',')[0]; } } + debuglog("unfilling now", backwards, origExcessHeight, Array.prototype.indexOf.call(tiles, tile)); if (markerScrollToken) { // Use a debouncer to prevent multiple unfill calls in quick succession @@ -439,7 +402,7 @@ module.exports = React.createClass({ * 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, + * number bottomOffset: undefined if stuckAtBottom is true; if it is false, * the number of pixels the bottom of the tracked child is above the * bottom of the scroll panel. */ @@ -460,14 +423,20 @@ module.exports = React.createClass({ * child list.) */ resetScrollState: function() { - this.scrollState = {stuckAtBottom: this.props.startAtBottom}; + this.scrollState = { + stuckAtBottom: this.props.startAtBottom, + }; + this._bottomGrowth = 0; + this._pages = 0; + this._scrollTimeout = new Timer(100); + this._heightUpdateInProgress = false; }, /** * jump to the top of the content. */ scrollToTop: function() { - this._setScrollTop(0); + this._getScrollNode().scrollTop = 0; this._saveScrollState(); }, @@ -479,24 +448,26 @@ module.exports = React.createClass({ // saved is to do the scroll, then save the updated state. (Calculating // it ourselves is hard, and we can't rely on an onScroll callback // happening, since there may be no user-visible change here). - this._setScrollTop(Number.MAX_VALUE); + const sn = this._getScrollNode(); + sn.scrollTop = sn.scrollHeight; this._saveScrollState(); }, /** * Page up/down. * - * mult: -1 to page up, +1 to page down + * @param {number} mult: -1 to page up, +1 to page down */ scrollRelative: function(mult) { const scrollNode = this._getScrollNode(); const delta = mult * scrollNode.clientHeight * 0.5; - this._setScrollTop(scrollNode.scrollTop + delta); + scrollNode.scrollTop = scrollNode.scrollTop + delta; this._saveScrollState(); }, /** * Scroll up/down in response to a scroll key + * @param {object} ev the keyboard event */ handleScrollKey: function(ev) { switch (ev.keyCode) { @@ -529,21 +500,21 @@ module.exports = React.createClass({ /* Scroll the panel to bring the DOM node with the scroll token * `scrollToken` into view. * - * offsetBase gives the reference point for the pixelOffset. 0 means the + * offsetBase gives the reference point for the bottomOffset. 0 means the * top of the container, 1 means the bottom, and fractional values mean * somewhere in the middle. If omitted, it defaults to 0. * - * pixelOffset gives the number of pixels *above* the offsetBase that the + * bottomOffset gives the number of pixels *above* the offsetBase that the * node (specifically, the bottom of it) will be positioned. If omitted, it * defaults to 0. */ - scrollToToken: function(scrollToken, pixelOffset, offsetBase) { - pixelOffset = pixelOffset || 0; + scrollToToken: function(scrollToken, bottomOffset, offsetBase) { + bottomOffset = bottomOffset || 0; offsetBase = offsetBase || 0; - // convert pixelOffset so that it is based on the bottom of the + // convert bottomOffset so that it is based on the bottom of the // container. - pixelOffset += this._getScrollNode().clientHeight * (1-offsetBase); + bottomOffset += this._getScrollNode().clientHeight * (1-offsetBase); // save the desired scroll state. It's important we do this here rather // than as a result of the scroll event, because (a) we might not *get* @@ -554,50 +525,13 @@ module.exports = React.createClass({ this.scrollState = { stuckAtBottom: false, trackedScrollToken: scrollToken, - pixelOffset: pixelOffset, + bottomOffset: bottomOffset, }; // ... then make it so. this._restoreSavedScrollState(); }, - // set the scrollTop attribute appropriately to position the given child at the - // given offset in the window. A helper for _restoreSavedScrollState. - _scrollToToken: function(scrollToken, pixelOffset) { - /* find the dom node with the right scrolltoken */ - let node; - const messages = this.refs.itemlist.children; - for (let i = messages.length-1; i >= 0; --i) { - const m = messages[i]; - // '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; - } - } - - if (!node) { - debuglog("ScrollPanel: No node with scrollToken '"+scrollToken+"'"); - return; - } - - const scrollNode = this._getScrollNode(); - const scrollTop = scrollNode.scrollTop; - const viewportBottom = scrollTop + scrollNode.clientHeight; - const nodeBottom = node.offsetTop + node.clientHeight; - const intendedViewportBottom = nodeBottom + pixelOffset; - const scrollDelta = intendedViewportBottom - viewportBottom; - - debuglog("ScrollPanel: scrolling to token '" + scrollToken + "'+" + - pixelOffset + " (delta: "+scrollDelta+")"); - - if (scrollDelta !== 0) { - this._setScrollTop(scrollTop + scrollDelta); - } - }, - _saveScrollState: function() { if (this.props.stickyBottom && this.isAtBottom()) { this.scrollState = { stuckAtBottom: true }; @@ -606,12 +540,13 @@ module.exports = React.createClass({ } const scrollNode = this._getScrollNode(); - const viewportBottom = scrollNode.scrollTop + scrollNode.clientHeight; + const viewportBottom = scrollNode.scrollHeight - (scrollNode.scrollTop + scrollNode.clientHeight); const itemlist = this.refs.itemlist; const messages = itemlist.children; let node = null; + // TODO: do a binary search here, as items are sorted by offsetTop // loop backwards, from bottom-most message (as that is the most common case) for (let i = messages.length-1; i >= 0; --i) { if (!messages[i].dataset.scrollTokens) { @@ -631,12 +566,12 @@ module.exports = React.createClass({ return; } - const nodeBottom = node.offsetTop + node.clientHeight; - debuglog("ScrollPanel: saved scroll state", this.scrollState); + debuglog("ScrollPanel: replacing scroll state"); this.scrollState = { stuckAtBottom: false, + trackedNode: node, trackedScrollToken: node.dataset.scrollTokens.split(',')[0], - pixelOffset: viewportBottom - nodeBottom, + bottomOffset: this._topFromBottom(node), }; }, @@ -644,35 +579,111 @@ module.exports = React.createClass({ const scrollState = this.scrollState; if (scrollState.stuckAtBottom) { - this._setScrollTop(Number.MAX_VALUE); + const sn = this._getScrollNode(); + sn.scrollTop = sn.scrollHeight; } else if (scrollState.trackedScrollToken) { - this._scrollToToken(scrollState.trackedScrollToken, - scrollState.pixelOffset); + const itemlist = this.refs.itemlist; + const trackedNode = this._getTrackedNode(); + if (trackedNode) { + const newBottomOffset = this._topFromBottom(trackedNode); + const bottomDiff = newBottomOffset - scrollState.bottomOffset; + this._bottomGrowth += bottomDiff; + scrollState.bottomOffset = newBottomOffset; + itemlist.style.height = `${this._getListHeight()}px`; + debuglog("ScrollPanel: balancing height because messages below viewport grew by "+bottomDiff+"px"); + } + } + // TODO: also call _updateHeight if not already in progress + if (!this._heightUpdateInProgress) { + const heightDiff = this._getMessagesHeight() - this._getListHeight(); + if (heightDiff > 0) { + this._updateHeight(); + } } }, + // need a better name that also indicates this will change scrollTop? Rebalance height? Reveal content? + async _updateHeight() { + if (this._heightUpdateInProgress) { + return; + } + this._heightUpdateInProgress = true; + try { + // wait until user has stopped scrolling + if (this._scrollTimeout.isRunning()) { + await this._scrollTimeout.finished(); + } - _setScrollTop: function(scrollTop) { - const scrollNode = this._getScrollNode(); + const sn = this._getScrollNode(); + const itemlist = this.refs.itemlist; + const contentHeight = this._getMessagesHeight(); + const minHeight = sn.clientHeight; + const height = Math.max(minHeight, contentHeight); + this._pages = Math.ceil(height / PAGE_SIZE); + this._bottomGrowth = 0; + const newHeight = this._getListHeight(); + + if (this.scrollState.stuckAtBottom) { + itemlist.style.height = `${newHeight}px`; + sn.scrollTop = sn.scrollHeight; + debuglog("updateHeight to", newHeight); + } else { + const trackedNode = this._getTrackedNode(); + const oldTop = trackedNode.offsetTop; + itemlist.style.height = `${newHeight}px`; + const newTop = trackedNode.offsetTop; + const topDiff = newTop - oldTop; + sn.scrollTop = sn.scrollTop + topDiff; + debuglog("updateHeight to", newHeight, topDiff); + } + } finally { + this._heightUpdateInProgress = false; + } + }, + + _getTrackedNode() { + const scrollState = this.scrollState; + const trackedNode = scrollState.trackedNode; + + if (!trackedNode || !trackedNode.parentElement) { + let node; + const messages = this.refs.itemlist.children; + const scrollToken = scrollState.trackedScrollToken; + + for (let i = messages.length-1; i >= 0; --i) { + const m = messages[i]; + // '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; + } + } + debuglog("had to find tracked node again for " + scrollState.trackedScrollToken); + scrollState.trackedNode = node; + } - const prevScroll = scrollNode.scrollTop; + if (!scrollState.trackedNode) { + debuglog("ScrollPanel: No node with ; '"+scrollState.trackedScrollToken+"'"); + return; + } - // FF ignores attempts to set scrollTop to very large numbers - scrollNode.scrollTop = Math.min(scrollTop, scrollNode.scrollHeight); + return scrollState.trackedNode; + }, - // If this change generates a scroll event, we should not update the - // saved scroll state on it. See the comments in onScroll. - // - // If we *don't* expect a scroll event, we need to leave _lastSetScroll - // alone, otherwise we'll end up ignoring a future scroll event which is - // nothing to do with this change. + _getListHeight() { + return this._bottomGrowth + (this._pages * PAGE_SIZE); + }, - if (scrollNode.scrollTop != prevScroll) { - this._lastSetScroll = scrollNode.scrollTop; - } + _getMessagesHeight() { + const itemlist = this.refs.itemlist; + const lastNode = itemlist.lastElementChild; + // 18 is itemlist padding + return (lastNode.offsetTop + lastNode.clientHeight) - itemlist.firstElementChild.offsetTop + (18 * 2); + }, - debuglog("ScrollPanel: set scrollTop:", scrollNode.scrollTop, - "requested:", scrollTop, - "_lastSetScroll:", this._lastSetScroll); + _topFromBottom(node) { + return this.refs.itemlist.clientHeight - node.offsetTop; }, /* get the DOM node which has the scrollTop property we care about for our @@ -742,7 +753,7 @@ module.exports = React.createClass({ // it's not obvious why we have a separate div and ol anyway. return ( + className={`mx_ScrollPanel ${this.props.className}`} style={this.props.style}>
      { this.props.children } From 469511aa4433d41aa8895f91db9a90babb98bd89 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 19 Mar 2019 16:50:55 +0100 Subject: [PATCH 23/52] correctly calculate last node in viewport these variables are now relative to bottom of timeline, before it was the top --- src/components/structures/ScrollPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 9d0bfce6cb1..5ddc0e00284 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -555,7 +555,7 @@ module.exports = React.createClass({ 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 < viewportBottom) { + if (this._topFromBottom(node) > viewportBottom) { // Use this node as the scrollToken break; } From 2bcced72ad0bc91b429d25bde5639f7ab501b0a9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 19 Mar 2019 16:51:39 +0100 Subject: [PATCH 24/52] take (potentially clipped) content height into account for filling --- src/components/structures/ScrollPanel.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 5ddc0e00284..601caf78d69 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -298,11 +298,14 @@ module.exports = React.createClass({ // `---------' - // - if (sn.scrollTop < sn.clientHeight) { + const contentHeight = this._getMessagesHeight(); + const contentTop = contentHeight - this._getListHeight(); + const contentScrollTop = sn.scrollTop + contentTop; + if (contentScrollTop < sn.clientHeight) { // need to back-fill this._maybeFill(true); } - if (sn.scrollTop > sn.scrollHeight - sn.clientHeight * 2) { + if (contentScrollTop > contentHeight - sn.clientHeight * 2) { // need to forward-fill this._maybeFill(false); } From 88f039fe44c175c4970690e21b313524bd64ddc4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 19 Mar 2019 16:53:23 +0100 Subject: [PATCH 25/52] keep track of current updateHeight request outside of method it's only called from one place --- src/components/structures/ScrollPanel.js | 68 +++++++++++------------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 601caf78d69..1cc78655cb5 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -578,7 +578,7 @@ module.exports = React.createClass({ }; }, - _restoreSavedScrollState: function() { + _restoreSavedScrollState: async function() { const scrollState = this.scrollState; if (scrollState.stuckAtBottom) { @@ -598,48 +598,44 @@ module.exports = React.createClass({ } // TODO: also call _updateHeight if not already in progress if (!this._heightUpdateInProgress) { - const heightDiff = this._getMessagesHeight() - this._getListHeight(); - if (heightDiff > 0) { - this._updateHeight(); + this._heightUpdateInProgress = true; + try { + await this._updateHeight(); + } finally { + this._heightUpdateInProgress = false; } } }, // need a better name that also indicates this will change scrollTop? Rebalance height? Reveal content? async _updateHeight() { - if (this._heightUpdateInProgress) { - return; + const startTs = Date.now(); + // wait until user has stopped scrolling + if (this._scrollTimeout.isRunning()) { + debuglog("xxx updateHeight waiting for scrolling to end ... "); + await this._scrollTimeout.finished(); } - this._heightUpdateInProgress = true; - try { - // wait until user has stopped scrolling - if (this._scrollTimeout.isRunning()) { - await this._scrollTimeout.finished(); - } - const sn = this._getScrollNode(); - const itemlist = this.refs.itemlist; - const contentHeight = this._getMessagesHeight(); - const minHeight = sn.clientHeight; - const height = Math.max(minHeight, contentHeight); - this._pages = Math.ceil(height / PAGE_SIZE); - this._bottomGrowth = 0; - const newHeight = this._getListHeight(); - - if (this.scrollState.stuckAtBottom) { - itemlist.style.height = `${newHeight}px`; - sn.scrollTop = sn.scrollHeight; - debuglog("updateHeight to", newHeight); - } else { - const trackedNode = this._getTrackedNode(); - const oldTop = trackedNode.offsetTop; - itemlist.style.height = `${newHeight}px`; - const newTop = trackedNode.offsetTop; - const topDiff = newTop - oldTop; - sn.scrollTop = sn.scrollTop + topDiff; - debuglog("updateHeight to", newHeight, topDiff); - } - } finally { - this._heightUpdateInProgress = false; + const sn = this._getScrollNode(); + const itemlist = this.refs.itemlist; + const contentHeight = this._getMessagesHeight(); + const minHeight = sn.clientHeight; + const height = Math.max(minHeight, contentHeight); + this._pages = Math.ceil(height / PAGE_SIZE); + this._bottomGrowth = 0; + const newHeight = this._getListHeight(); + + if (this.scrollState.stuckAtBottom) { + itemlist.style.height = `${newHeight}px`; + sn.scrollTop = sn.scrollHeight; + debuglog("xxx updateHeight to", newHeight); + } else { + const trackedNode = this._getTrackedNode(); + const oldTop = trackedNode.offsetTop; + itemlist.style.height = `${newHeight}px`; + const newTop = trackedNode.offsetTop; + const topDiff = newTop - oldTop; + sn.scrollTop = sn.scrollTop + topDiff; + debuglog("xxx updateHeight to", newHeight, topDiff, Date.now() - startTs); } }, From c306181fcd7f008ffcfd95523a2a0e0cf7dce355 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 11:00:27 +0100 Subject: [PATCH 26/52] take into account that node might not be in DOM while updating height --- src/components/structures/ScrollPanel.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 1cc78655cb5..1867a5abb0b 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -624,18 +624,25 @@ module.exports = React.createClass({ this._bottomGrowth = 0; const newHeight = this._getListHeight(); - if (this.scrollState.stuckAtBottom) { + const scrollState = this.scrollState; + if (scrollState.stuckAtBottom) { itemlist.style.height = `${newHeight}px`; sn.scrollTop = sn.scrollHeight; debuglog("xxx updateHeight to", newHeight); - } else { + } else if (scrollState.trackedScrollToken) { const trackedNode = this._getTrackedNode(); - const oldTop = trackedNode.offsetTop; - itemlist.style.height = `${newHeight}px`; - const newTop = trackedNode.offsetTop; - const topDiff = newTop - oldTop; - sn.scrollTop = sn.scrollTop + topDiff; - debuglog("xxx updateHeight to", newHeight, topDiff, Date.now() - startTs); + // if the timeline has been reloaded + // this can be called before scrollToBottom or whatever has been called + // so don't do anything of the node has disappeared from + // the currently filled piece of the timeline + if (trackedNode) { + const oldTop = trackedNode.offsetTop; + itemlist.style.height = `${newHeight}px`; + const newTop = trackedNode.offsetTop; + const topDiff = newTop - oldTop; + sn.scrollTop = sn.scrollTop + topDiff; + debuglog("xxx updateHeight to", newHeight, topDiff, Date.now() - startTs); + } } }, From 1e372aad47711332554ac9f7572b1c60f1cc8cbb Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 11:10:04 +0100 Subject: [PATCH 27/52] only log when node was found --- src/components/structures/ScrollPanel.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 1867a5abb0b..36cf4dc0181 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -665,7 +665,9 @@ module.exports = React.createClass({ break; } } - debuglog("had to find tracked node again for " + scrollState.trackedScrollToken); + if (node) { + debuglog("had to find tracked node again for " + scrollState.trackedScrollToken); + } scrollState.trackedNode = node; } From f164a78eaaebb4987302afd39c13a38f63719518 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 17:02:53 +0100 Subject: [PATCH 28/52] reimplement typing notif timeline shrinking prevention instead of setting a min-height on the whole timeline, track how much height we need to add to prevent shrinking and set paddingBottom on the container element of the timeline. --- src/components/structures/MessagePanel.js | 31 +++-- src/components/structures/ScrollPanel.js | 107 +++++++++++++----- src/components/structures/TimelinePanel.js | 2 +- src/components/views/rooms/WhoIsTypingTile.js | 21 ++-- 4 files changed, 115 insertions(+), 46 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index fecf5f1ad7d..805749b1cd5 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -631,12 +631,22 @@ module.exports = React.createClass({ } }, - _onTypingVisible: function() { + _onTypingShown: function() { const scrollPanel = this.refs.scrollPanel; + // this will make the timeline grow, so checkScroll + scrollPanel.checkScroll(); if (scrollPanel && scrollPanel.getScrollState().stuckAtBottom) { - // scroll down if at bottom - scrollPanel.checkScroll(); - scrollPanel.blockShrinking(); + scrollPanel.preventShrinking(); + } + }, + + _onTypingHidden: function() { + const scrollPanel = this.refs.scrollPanel; + if (scrollPanel) { + // as hiding the typing notifications doesn't + // update the scrollPanel, we tell it to apply + // the shrinking prevention once the typing notifs are hidden + scrollPanel.updatePreventShrinking(); } }, @@ -652,15 +662,15 @@ module.exports = React.createClass({ // update the min-height, so once the last // person stops typing, no jumping occurs if (isAtBottom && isTypingVisible) { - scrollPanel.blockShrinking(); + scrollPanel.preventShrinking(); } } }, - clearTimelineHeight: function() { + onTimelineReset: function() { const scrollPanel = this.refs.scrollPanel; if (scrollPanel) { - scrollPanel.clearBlockShrinking(); + scrollPanel.clearPreventShrinking(); } }, @@ -688,7 +698,12 @@ module.exports = React.createClass({ let whoIsTyping; if (this.props.room) { - whoIsTyping = (); + whoIsTyping = ( + ); } return ( diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 36cf4dc0181..b1add8235f0 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -175,6 +175,7 @@ module.exports = React.createClass({ // // This will also re-check the fill state, in case the paginate was inadequate this.checkScroll(); + this.updatePreventShrinking(); }, componentWillUnmount: function() { @@ -192,22 +193,23 @@ module.exports = React.createClass({ onScroll: function(ev) { this._scrollTimeout.restart(); this._saveScrollState(); - this._checkBlockShrinking(); this.checkFillState(); - + this.updatePreventShrinking(); this.props.onScroll(ev); }, onResize: function() { - this.clearBlockShrinking(); this.checkScroll(); + // update preventShrinkingState if present + if (this.preventShrinkingState) { + this.preventShrinking(); + } }, // after an update to the contents of the panel, check that the scroll is // where it ought to be, and set off pagination requests if necessary. checkScroll: function() { this._restoreSavedScrollState(); - this._checkBlockShrinking(); this.checkFillState(); }, @@ -718,39 +720,84 @@ module.exports = React.createClass({ }, /** - * Set the current height as the min height for the message list - * so the timeline cannot shrink. This is used to avoid - * jumping when the typing indicator gets replaced by a smaller message. - */ - blockShrinking: function() { + Mark the bottom offset of the last tile so we can balance it out when + anything below it changes, by calling updatePreventShrinking, to keep + the same minimum bottom offset, effectively preventing the timeline to shrink. + */ + preventShrinking: function() { const messageList = this.refs.itemlist; - if (messageList) { - const currentHeight = messageList.clientHeight; - messageList.style.minHeight = `${currentHeight}px`; + const tiles = messageList && messageList.children; + if (!messageList) { + return; + } + let lastTileNode; + for (let i = tiles.length - 1; i >= 0; i--) { + const node = tiles[i]; + if (node.dataset.scrollTokens) { + lastTileNode = node; + break; + } } + if (!lastTileNode) { + return; + } + this.clearPreventShrinking(); + const offsetFromBottom = messageList.clientHeight - (lastTileNode.offsetTop + lastTileNode.clientHeight); + this.preventShrinkingState = { + offsetFromBottom: offsetFromBottom, + offsetNode: lastTileNode, + }; + debuglog("prevent shrinking, last tile ", offsetFromBottom, "px from bottom"); }, - /** - * Clear the previously set min height - */ - clearBlockShrinking: function() { + /** Clear shrinking prevention. Used internally, and when the timeline is reloaded. */ + clearPreventShrinking: function() { const messageList = this.refs.itemlist; - if (messageList) { - messageList.style.minHeight = null; - } + const balanceElement = messageList && messageList.parentElement; + if (balanceElement) balanceElement.style.paddingBottom = null; + this.preventShrinkingState = null; + debuglog("prevent shrinking cleared"); }, - _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(); + /** + update the container padding to balance + the bottom offset of the last tile since + preventShrinking was called. + Clears the prevent-shrinking state ones the offset + from the bottom of the marked tile grows larger than + what it was when marking. + */ + updatePreventShrinking: function() { + if (this.preventShrinkingState) { + const sn = this._getScrollNode(); + const scrollState = this.scrollState; + const messageList = this.refs.itemlist; + const {offsetNode, offsetFromBottom} = this.preventShrinkingState; + // element used to set paddingBottom to balance the typing notifs disappearing + const balanceElement = messageList.parentElement; + // if the offsetNode got unmounted, clear + let shouldClear = !offsetNode.parentElement; + // also if 200px from bottom + if (!shouldClear && !scrollState.stuckAtBottom) { + const spaceBelowViewport = sn.scrollHeight - (sn.scrollTop + sn.clientHeight); + shouldClear = spaceBelowViewport >= 200; + } + // try updating if not clearing + if (!shouldClear) { + const currentOffset = messageList.clientHeight - (offsetNode.offsetTop + offsetNode.clientHeight); + const offsetDiff = offsetFromBottom - currentOffset; + if (offsetDiff > 0) { + balanceElement.style.paddingBottom = `${offsetDiff}px`; + if (this.scrollState.stuckAtBottom) { + sn.scrollTop = sn.scrollHeight; + } + debuglog("update prevent shrinking ", offsetDiff, "px from bottom"); + } else if (offsetDiff < 0) { + shouldClear = true; + } + } + if (shouldClear) { + this.clearPreventShrinking(); } } }, diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index c983f904a01..aba7964a157 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -939,7 +939,7 @@ var TimelinePanel = React.createClass({ // clear the timeline min-height when // (re)loading the timeline if (this.refs.messagePanel) { - this.refs.messagePanel.clearTimelineHeight(); + this.refs.messagePanel.onTimelineReset(); } this._reloadEvents(); diff --git a/src/components/views/rooms/WhoIsTypingTile.js b/src/components/views/rooms/WhoIsTypingTile.js index 9dd690f6e55..95cf0717c7d 100644 --- a/src/components/views/rooms/WhoIsTypingTile.js +++ b/src/components/views/rooms/WhoIsTypingTile.js @@ -29,7 +29,8 @@ module.exports = React.createClass({ propTypes: { // the room this statusbar is representing. room: PropTypes.object.isRequired, - onVisible: PropTypes.func, + onShown: PropTypes.func, + onHidden: PropTypes.func, // Number of names to display in typing indication. E.g. set to 3, will // result in "X, Y, Z and 100 others are typing." whoIsTypingLimit: PropTypes.number, @@ -59,11 +60,13 @@ module.exports = React.createClass({ }, componentDidUpdate: function(_, prevState) { - if (this.props.onVisible && - !prevState.usersTyping.length && - this.state.usersTyping.length - ) { - this.props.onVisible(); + const wasVisible = this._isVisible(prevState); + const isVisible = this._isVisible(this.state); + if (this.props.onShown && !wasVisible && isVisible) { + this.props.onShown(); + } + else if (this.props.onHidden && wasVisible && !isVisible) { + this.props.onHidden(); } }, @@ -77,8 +80,12 @@ module.exports = React.createClass({ Object.values(this.state.delayedStopTypingTimers).forEach((t) => t.abort()); }, + _isVisible: function(state) { + return state.usersTyping.length !== 0 || Object.keys(state.delayedStopTypingTimers).length !== 0; + }, + isVisible: function() { - return this.state.usersTyping.length !== 0 || Object.keys(this.state.delayedStopTypingTimers).length !== 0; + return this._isVisible(this.state); }, onRoomTimeline: function(event, room) { From 460f9a5959312a621b1828f920dabba160c09436 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 17:10:38 +0100 Subject: [PATCH 29/52] comment typo --- src/components/structures/ScrollPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index b1add8235f0..4e17cff320f 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -216,7 +216,7 @@ module.exports = React.createClass({ // return true if the content is fully scrolled down right now; else false. // // note that this is independent of the 'stuckAtBottom' state - it is simply - // about whether the the content is scrolled down right now, irrespective of + // about whether the content is scrolled down right now, irrespective of // whether it will stay that way when the children update. isAtBottom: function() { const sn = this._getScrollNode(); From a8b149cfbbcf0df35ab0cf7310fcc77af38fdfb9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 17:12:49 +0100 Subject: [PATCH 30/52] cleanup scrollpanel logging --- src/components/structures/ScrollPanel.js | 30 +++++++++++++----------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 4e17cff320f..7d9ac0fb6a2 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -22,7 +22,6 @@ import Timer from '../../utils/Timer'; import AutoHideScrollbar from "./AutoHideScrollbar"; const DEBUG_SCROLL = false; -// const DEBUG_SCROLL = true; // The amount of extra scroll distance to allow prior to unfilling. // See _getExcessHeight. @@ -347,7 +346,6 @@ module.exports = React.createClass({ markerScrollToken = tile.dataset.scrollTokens.split(',')[0]; } } - debuglog("unfilling now", backwards, origExcessHeight, Array.prototype.indexOf.call(tiles, tile)); if (markerScrollToken) { // Use a debouncer to prevent multiple unfill calls in quick succession @@ -357,6 +355,7 @@ module.exports = React.createClass({ } this._unfillDebouncer = setTimeout(() => { this._unfillDebouncer = null; + debuglog("unfilling now", backwards, origExcessHeight); this.props.onUnfillRequest(backwards, markerScrollToken); }, UNFILL_REQUEST_DEBOUNCE_MS); } @@ -366,11 +365,11 @@ module.exports = React.createClass({ _maybeFill: function(backwards) { const dir = backwards ? 'b' : 'f'; if (this._pendingFillRequests[dir]) { - debuglog("ScrollPanel: Already a "+dir+" fill in progress - not starting another"); + debuglog("Already a "+dir+" fill in progress - not starting another"); return; } - debuglog("ScrollPanel: starting "+dir+" fill"); + debuglog("starting "+dir+" fill"); // onFillRequest can end up calling us recursively (via onScroll // events) so make sure we set this before firing off the call. @@ -387,7 +386,7 @@ module.exports = React.createClass({ // Unpaginate once filling is complete this._checkUnfillState(!backwards); - debuglog("ScrollPanel: "+dir+" fill complete; hasMoreResults:"+hasMoreResults); + debuglog(""+dir+" fill complete; hasMoreResults:"+hasMoreResults); if (hasMoreResults) { // further pagination requests have been disabled until now, so // it's time to check the fill state again in case the pagination @@ -540,7 +539,7 @@ module.exports = React.createClass({ _saveScrollState: function() { if (this.props.stickyBottom && this.isAtBottom()) { this.scrollState = { stuckAtBottom: true }; - debuglog("ScrollPanel: Saved scroll state", this.scrollState); + debuglog("saved stuckAtBottom state"); return; } @@ -567,11 +566,11 @@ module.exports = React.createClass({ } if (!node) { - debuglog("ScrollPanel: unable to save scroll state: found no children in the viewport"); + debuglog("unable to save scroll state: found no children in the viewport"); return; } - debuglog("ScrollPanel: replacing scroll state"); + debuglog("saving anchored scroll state to message", node && node.innerText); this.scrollState = { stuckAtBottom: false, trackedNode: node, @@ -595,7 +594,7 @@ module.exports = React.createClass({ this._bottomGrowth += bottomDiff; scrollState.bottomOffset = newBottomOffset; itemlist.style.height = `${this._getListHeight()}px`; - debuglog("ScrollPanel: balancing height because messages below viewport grew by "+bottomDiff+"px"); + debuglog("balancing height because messages below viewport grew by "+bottomDiff+"px"); } } // TODO: also call _updateHeight if not already in progress @@ -606,15 +605,18 @@ module.exports = React.createClass({ } finally { this._heightUpdateInProgress = false; } + } else { + debuglog("not updating height because request already in progress"); } }, // need a better name that also indicates this will change scrollTop? Rebalance height? Reveal content? async _updateHeight() { - const startTs = Date.now(); // wait until user has stopped scrolling if (this._scrollTimeout.isRunning()) { - debuglog("xxx updateHeight waiting for scrolling to end ... "); + debuglog("updateHeight waiting for scrolling to end ... "); await this._scrollTimeout.finished(); + } else { + debuglog("updateHeight getting straight to business, no scrolling going on."); } const sn = this._getScrollNode(); @@ -630,7 +632,7 @@ module.exports = React.createClass({ if (scrollState.stuckAtBottom) { itemlist.style.height = `${newHeight}px`; sn.scrollTop = sn.scrollHeight; - debuglog("xxx updateHeight to", newHeight); + debuglog("updateHeight to", newHeight); } else if (scrollState.trackedScrollToken) { const trackedNode = this._getTrackedNode(); // if the timeline has been reloaded @@ -643,7 +645,7 @@ module.exports = React.createClass({ const newTop = trackedNode.offsetTop; const topDiff = newTop - oldTop; sn.scrollTop = sn.scrollTop + topDiff; - debuglog("xxx updateHeight to", newHeight, topDiff, Date.now() - startTs); + debuglog("updateHeight to", newHeight, topDiff); } } }, @@ -674,7 +676,7 @@ module.exports = React.createClass({ } if (!scrollState.trackedNode) { - debuglog("ScrollPanel: No node with ; '"+scrollState.trackedScrollToken+"'"); + debuglog("No node with ; '"+scrollState.trackedScrollToken+"'"); return; } From 9da13fe430eff77ac765c00f091215659c047efd Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 17:13:09 +0100 Subject: [PATCH 31/52] small cleanup --- src/components/structures/ScrollPanel.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 7d9ac0fb6a2..b6dd17e8b67 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -569,7 +569,6 @@ module.exports = React.createClass({ debuglog("unable to save scroll state: found no children in the viewport"); return; } - debuglog("saving anchored scroll state to message", node && node.innerText); this.scrollState = { stuckAtBottom: false, @@ -597,7 +596,6 @@ module.exports = React.createClass({ debuglog("balancing height because messages below viewport grew by "+bottomDiff+"px"); } } - // TODO: also call _updateHeight if not already in progress if (!this._heightUpdateInProgress) { this._heightUpdateInProgress = true; try { From 02a5aa3b1f698d178bb5ee6ebd41abf909d9276b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 17:15:43 +0100 Subject: [PATCH 32/52] more logging cleanup --- src/components/structures/ScrollPanel.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index b6dd17e8b67..d322a832258 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -569,11 +569,12 @@ module.exports = React.createClass({ debuglog("unable to save scroll state: found no children in the viewport"); return; } - debuglog("saving anchored scroll state to message", node && node.innerText); + const scrollToken = node.dataset.scrollTokens.split(',')[0]; + debuglog("saving anchored scroll state to message", node && node.innerText, scrollToken); this.scrollState = { stuckAtBottom: false, trackedNode: node, - trackedScrollToken: node.dataset.scrollTokens.split(',')[0], + trackedScrollToken: scrollToken, bottomOffset: this._topFromBottom(node), }; }, From 85d305430f92fe6f38298e42feaeefa2b0bebc63 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 17:37:34 +0100 Subject: [PATCH 33/52] no need for forceUpdate here --- src/components/structures/MessagePanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 805749b1cd5..51fee851f33 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -627,7 +627,7 @@ module.exports = React.createClass({ _onHeightChanged: function() { const scrollPanel = this.refs.scrollPanel; if (scrollPanel) { - scrollPanel.forceUpdate(); + scrollPanel.checkScroll(); } }, From 3e13a11372b2194f3ea97689193be8f8d04f9cc5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 20 Mar 2019 17:38:05 +0100 Subject: [PATCH 34/52] restore scroll position after hiding typing notifs with checkScroll --- src/components/structures/MessagePanel.js | 3 +++ src/components/structures/ScrollPanel.js | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 51fee851f33..b57b659136a 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -647,6 +647,9 @@ module.exports = React.createClass({ // update the scrollPanel, we tell it to apply // the shrinking prevention once the typing notifs are hidden scrollPanel.updatePreventShrinking(); + // order is important here as checkScroll will scroll down to + // reveal added padding to balance the notifs disappearing. + scrollPanel.checkScroll(); } }, diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index d322a832258..f33fec9d8ab 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -789,9 +789,6 @@ module.exports = React.createClass({ const offsetDiff = offsetFromBottom - currentOffset; if (offsetDiff > 0) { balanceElement.style.paddingBottom = `${offsetDiff}px`; - if (this.scrollState.stuckAtBottom) { - sn.scrollTop = sn.scrollHeight; - } debuglog("update prevent shrinking ", offsetDiff, "px from bottom"); } else if (offsetDiff < 0) { shouldClear = true; From 680afc5ce0966a121bcdade742560a6562679880 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 21 Mar 2019 15:37:06 +0100 Subject: [PATCH 35/52] fix scrollToToken alignment --- src/components/structures/ScrollPanel.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index f33fec9d8ab..c92d74e2834 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -518,7 +518,9 @@ module.exports = React.createClass({ // convert bottomOffset so that it is based on the bottom of the // container. - bottomOffset += this._getScrollNode().clientHeight * (1-offsetBase); + const scrollNode = this._getScrollNode(); + const viewportBottom = scrollNode.scrollHeight - (scrollNode.scrollTop + scrollNode.clientHeight); + bottomOffset += viewportBottom + scrollNode.clientHeight * (1-offsetBase); // save the desired scroll state. It's important we do this here rather // than as a result of the scroll event, because (a) we might not *get* From ab49bc4fcf94ce8bebc51bdc5820c6c98431501e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 21 Mar 2019 15:37:35 +0100 Subject: [PATCH 36/52] fix comment typo --- src/components/structures/ScrollPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index c92d74e2834..a04e4ef171b 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -638,7 +638,7 @@ module.exports = React.createClass({ const trackedNode = this._getTrackedNode(); // if the timeline has been reloaded // this can be called before scrollToBottom or whatever has been called - // so don't do anything of the node has disappeared from + // so don't do anything if the node has disappeared from // the currently filled piece of the timeline if (trackedNode) { const oldTop = trackedNode.offsetTop; From 4bfdbe3094832c5491edf15c1923a5d6c7616d25 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 22 Mar 2019 15:23:21 +0100 Subject: [PATCH 37/52] fix jumping when unfilling the top while scrolling down --- src/components/structures/ScrollPanel.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index a04e4ef171b..7f992097bfb 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -642,11 +642,17 @@ module.exports = React.createClass({ // the currently filled piece of the timeline if (trackedNode) { const oldTop = trackedNode.offsetTop; + // changing the height might change the scrollTop + // if the new height is smaller than the scrollTop. + // We calculate the diff that needs to be applied + // ourselves, so be sure to measure the + // scrollTop before changing the height. + const preexistingScrollTop = sn.scrollTop; itemlist.style.height = `${newHeight}px`; const newTop = trackedNode.offsetTop; const topDiff = newTop - oldTop; - sn.scrollTop = sn.scrollTop + topDiff; - debuglog("updateHeight to", newHeight, topDiff); + sn.scrollTop = preexistingScrollTop + topDiff; + debuglog("updateHeight to", {newHeight, topDiff, preexistingScrollTop}); } } }, From c9c251924069ef9a641c78856010f2b8ea79c48e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 22 Mar 2019 15:47:04 +0100 Subject: [PATCH 38/52] log scroll events --- src/components/structures/ScrollPanel.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 7f992097bfb..f00ff401554 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -190,6 +190,7 @@ module.exports = React.createClass({ }, onScroll: function(ev) { + debuglog("onScroll", this._getScrollNode().scrollTop); this._scrollTimeout.restart(); this._saveScrollState(); this.checkFillState(); From 8f7170a4a161012ee3f84d6214836a8fc31cf1e6 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 25 Mar 2019 12:45:51 +0100 Subject: [PATCH 39/52] add timeline profiling markers when updateheight and onscroll run --- src/components/structures/ScrollPanel.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index f00ff401554..ccf2e06f9f6 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -195,6 +195,7 @@ module.exports = React.createClass({ this._saveScrollState(); this.checkFillState(); this.updatePreventShrinking(); + console.timeStamp("onScroll:" + JSON.stringify({st: this._getScrollNode().scrollTop, mh: this._getMessagesHeight(), lh: this._getListHeight()})); this.props.onScroll(ev); }, @@ -596,6 +597,7 @@ module.exports = React.createClass({ const bottomDiff = newBottomOffset - scrollState.bottomOffset; this._bottomGrowth += bottomDiff; scrollState.bottomOffset = newBottomOffset; + console.timeStamp("restoreSavedScrollState:" + JSON.stringify({bd: bottomDiff, lh: this._getListHeight()})); itemlist.style.height = `${this._getListHeight()}px`; debuglog("balancing height because messages below viewport grew by "+bottomDiff+"px"); } @@ -635,6 +637,7 @@ module.exports = React.createClass({ itemlist.style.height = `${newHeight}px`; sn.scrollTop = sn.scrollHeight; debuglog("updateHeight to", newHeight); + console.timeStamp("updateHeight: to bottom"); } else if (scrollState.trackedScrollToken) { const trackedNode = this._getTrackedNode(); // if the timeline has been reloaded @@ -654,6 +657,7 @@ module.exports = React.createClass({ const topDiff = newTop - oldTop; sn.scrollTop = preexistingScrollTop + topDiff; debuglog("updateHeight to", {newHeight, topDiff, preexistingScrollTop}); + console.timeStamp("updateHeight:" + JSON.stringify({nh: newHeight, td: topDiff, st: preexistingScrollTop})); } } }, From 18b5041ed21cb99d54e44c3aca3f664277842b06 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 26 Mar 2019 12:19:30 +0100 Subject: [PATCH 40/52] try filling async instead of sync in scroll handler see if that avoids jumps --- src/components/structures/ScrollPanel.js | 49 +++++++++++++++++++----- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index ccf2e06f9f6..eaf3e26db7e 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -21,7 +21,7 @@ import { KeyCode } from '../../Keyboard'; import Timer from '../../utils/Timer'; import AutoHideScrollbar from "./AutoHideScrollbar"; -const DEBUG_SCROLL = false; +const DEBUG_SCROLL = true; // The amount of extra scroll distance to allow prior to unfilling. // See _getExcessHeight. @@ -193,10 +193,10 @@ module.exports = React.createClass({ debuglog("onScroll", this._getScrollNode().scrollTop); this._scrollTimeout.restart(); this._saveScrollState(); - this.checkFillState(); this.updatePreventShrinking(); console.timeStamp("onScroll:" + JSON.stringify({st: this._getScrollNode().scrollTop, mh: this._getMessagesHeight(), lh: this._getListHeight()})); this.props.onScroll(ev); + this.checkFillState(); }, onResize: function() { @@ -270,11 +270,12 @@ module.exports = React.createClass({ }, // check the scroll state and send out backfill requests if necessary. - checkFillState: function() { + checkFillState: async function(depth=0) { if (this.unmounted) { return; } + const isFirstCall = depth === 0; const sn = this._getScrollNode(); // if there is less than a screenful of messages above or below the @@ -301,16 +302,46 @@ module.exports = React.createClass({ // `---------' - // + if (isFirstCall) { + if (this._isFilling) { + debuglog("_isFilling: not entering while request is ongoing, marking for a subsequent request"); + this._fillRequestWhileRunning = true; + return; + } + debuglog("_isFilling: setting"); + this._isFilling = true; + } + + const contentHeight = this._getMessagesHeight(); const contentTop = contentHeight - this._getListHeight(); const contentScrollTop = sn.scrollTop + contentTop; + const fillPromises = []; + if (contentScrollTop < sn.clientHeight) { // need to back-fill - this._maybeFill(true); + fillPromises.push(this._maybeFill(depth, true)); } if (contentScrollTop > contentHeight - sn.clientHeight * 2) { // need to forward-fill - this._maybeFill(false); + fillPromises.push(this._maybeFill(depth, false)); + } + + if (fillPromises.length) { + try { + await Promise.all(fillPromises); + } catch (err) { + console.error(err); + } + } + if (isFirstCall) { + debuglog("_isFilling: clearing"); + this._isFilling = false; + } + + if (this._fillRequestWhileRunning) { + this._fillRequestWhileRunning = false; + this.checkFillState(); } }, @@ -364,7 +395,7 @@ module.exports = React.createClass({ }, // check if there is already a pending fill request. If not, set one off. - _maybeFill: function(backwards) { + _maybeFill: function(depth, backwards) { const dir = backwards ? 'b' : 'f'; if (this._pendingFillRequests[dir]) { debuglog("Already a "+dir+" fill in progress - not starting another"); @@ -377,7 +408,7 @@ module.exports = React.createClass({ // events) so make sure we set this before firing off the call. this._pendingFillRequests[dir] = true; - Promise.try(() => { + return new Promise(resolve => setTimeout(resolve, 1)).then(() => { return this.props.onFillRequest(backwards); }).finally(() => { this._pendingFillRequests[dir] = false; @@ -393,9 +424,9 @@ module.exports = React.createClass({ // further pagination requests have been disabled until now, so // it's time to check the fill state again in case the pagination // was insufficient. - this.checkFillState(); + return this.checkFillState(depth + 1); } - }).done(); + }); }, /* get the current scroll state. This returns an object with the following From 82a9b348c05d42f6148229aa4498b4aa0508458c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 26 Mar 2019 13:34:55 +0100 Subject: [PATCH 41/52] add some comments and initialization for async filling --- src/components/structures/ScrollPanel.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index eaf3e26db7e..7ae102eb4b3 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -154,6 +154,8 @@ module.exports = React.createClass({ }, componentWillMount: function() { + this._fillRequestWhileRunning = false; + this._isFilling = false; this._pendingFillRequests = {b: null, f: null}; if (this.props.resizeNotifier) { @@ -302,6 +304,10 @@ module.exports = React.createClass({ // `---------' - // + // as filling is async and recursive, + // don't allow more than 1 chain of calls concurrently + // do make a note when a new request comes in while already running one, + // so we can trigger a new chain of calls once done. if (isFirstCall) { if (this._isFilling) { debuglog("_isFilling: not entering while request is ongoing, marking for a subsequent request"); @@ -408,6 +414,10 @@ module.exports = React.createClass({ // events) so make sure we set this before firing off the call. this._pendingFillRequests[dir] = true; + // wait 1ms before paginating, because otherwise + // this will block the scroll event handler for +700ms + // if messages are already cached in memory, + // This would cause jumping to happen on Chrome/macOS. return new Promise(resolve => setTimeout(resolve, 1)).then(() => { return this.props.onFillRequest(backwards); }).finally(() => { From 805539fdc740bcdd98852348c56b68e41e3c3040 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 26 Mar 2019 13:44:39 +0100 Subject: [PATCH 42/52] remove timestamp logging in profiler --- src/components/structures/ScrollPanel.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 7ae102eb4b3..ba67468fbb0 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -196,7 +196,6 @@ module.exports = React.createClass({ this._scrollTimeout.restart(); this._saveScrollState(); this.updatePreventShrinking(); - console.timeStamp("onScroll:" + JSON.stringify({st: this._getScrollNode().scrollTop, mh: this._getMessagesHeight(), lh: this._getListHeight()})); this.props.onScroll(ev); this.checkFillState(); }, @@ -638,9 +637,8 @@ module.exports = React.createClass({ const bottomDiff = newBottomOffset - scrollState.bottomOffset; this._bottomGrowth += bottomDiff; scrollState.bottomOffset = newBottomOffset; - console.timeStamp("restoreSavedScrollState:" + JSON.stringify({bd: bottomDiff, lh: this._getListHeight()})); itemlist.style.height = `${this._getListHeight()}px`; - debuglog("balancing height because messages below viewport grew by "+bottomDiff+"px"); + debuglog("balancing height because messages below viewport grew by", bottomDiff); } } if (!this._heightUpdateInProgress) { @@ -678,7 +676,6 @@ module.exports = React.createClass({ itemlist.style.height = `${newHeight}px`; sn.scrollTop = sn.scrollHeight; debuglog("updateHeight to", newHeight); - console.timeStamp("updateHeight: to bottom"); } else if (scrollState.trackedScrollToken) { const trackedNode = this._getTrackedNode(); // if the timeline has been reloaded @@ -698,7 +695,6 @@ module.exports = React.createClass({ const topDiff = newTop - oldTop; sn.scrollTop = preexistingScrollTop + topDiff; debuglog("updateHeight to", {newHeight, topDiff, preexistingScrollTop}); - console.timeStamp("updateHeight:" + JSON.stringify({nh: newHeight, td: topDiff, st: preexistingScrollTop})); } } }, From 46f5f872c4b12fa0c44e2e14e4d1762803f21456 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 26 Mar 2019 15:51:02 +0100 Subject: [PATCH 43/52] implement scrolling to a token (best effort) --- src/components/structures/ScrollPanel.js | 41 ++++++++++++------------ 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index ba67468fbb0..57185b08b0f 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -546,38 +546,35 @@ module.exports = React.createClass({ /* Scroll the panel to bring the DOM node with the scroll token * `scrollToken` into view. * - * offsetBase gives the reference point for the bottomOffset. 0 means the + * offsetBase gives the reference point for the pixelOffset. 0 means the * top of the container, 1 means the bottom, and fractional values mean * somewhere in the middle. If omitted, it defaults to 0. * - * bottomOffset gives the number of pixels *above* the offsetBase that the + * pixelOffset gives the number of pixels *above* the offsetBase that the * node (specifically, the bottom of it) will be positioned. If omitted, it * defaults to 0. */ - scrollToToken: function(scrollToken, bottomOffset, offsetBase) { - bottomOffset = bottomOffset || 0; + scrollToToken: function(scrollToken, pixelOffset, offsetBase) { + pixelOffset = pixelOffset || 0; offsetBase = offsetBase || 0; - // convert bottomOffset so that it is based on the bottom of the - // container. - const scrollNode = this._getScrollNode(); - const viewportBottom = scrollNode.scrollHeight - (scrollNode.scrollTop + scrollNode.clientHeight); - bottomOffset += viewportBottom + scrollNode.clientHeight * (1-offsetBase); - - // save the desired scroll state. It's important we do this here rather - // than as a result of the scroll event, because (a) we might not *get* - // a scroll event, and (b) it might not currently be possible to set - // the requested scroll state (eg, because we hit the end of the - // timeline and need to do more pagination); we want to save the - // *desired* scroll state rather than what we end up achieving. + // set the trackedScrollToken so we can get the node through _getTrackedNode this.scrollState = { stuckAtBottom: false, trackedScrollToken: scrollToken, - bottomOffset: bottomOffset, }; - - // ... then make it so. - this._restoreSavedScrollState(); + const trackedNode = this._getTrackedNode(); + const scrollNode = this._getScrollNode(); + if (trackedNode) { + // set the scrollTop to the position we want. + // note though, that this might not succeed if the combination of offsetBase and pixelOffset + // would position the trackedNode towards the top of the viewport. + // This because when setting the scrollTop only 10 or so events might be loaded, + // not giving enough content below the trackedNode to scroll downwards + // enough so it ends up in the top of the viewport. + scrollNode.scrollTop = (trackedNode.offsetTop - (scrollNode.clientHeight * offsetBase)) + pixelOffset; + this._saveScrollState(); + } }, _saveScrollState: function() { @@ -615,11 +612,13 @@ module.exports = React.createClass({ } const scrollToken = node.dataset.scrollTokens.split(',')[0]; debuglog("saving anchored scroll state to message", node && node.innerText, scrollToken); + const bottomOffset = this._topFromBottom(node); this.scrollState = { stuckAtBottom: false, trackedNode: node, trackedScrollToken: scrollToken, - bottomOffset: this._topFromBottom(node), + bottomOffset: bottomOffset, + pixelOffset: bottomOffset - viewportBottom, //needed for restoring the scroll position when coming back to the room }; }, From 7e56a9a80eeb09bdd462498a2fcf707d6b650326 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 26 Mar 2019 17:13:02 +0100 Subject: [PATCH 44/52] prevent resize handle hit area overlapping with (native) scrollbar --- res/css/structures/_MainSplit.scss | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/res/css/structures/_MainSplit.scss b/res/css/structures/_MainSplit.scss index 28c89fe7ca5..4d73953cd74 100644 --- a/res/css/structures/_MainSplit.scss +++ b/res/css/structures/_MainSplit.scss @@ -19,3 +19,9 @@ limitations under the License. flex-direction: row; min-width: 0; } + +// move hit area 5px to the right so it doesn't overlap with the timeline scrollbar +.mx_MainSplit > .mx_ResizeHandle.mx_ResizeHandle_horizontal { + margin: 0 -10px 0 0; + padding: 0 10px 0 0; +} From f2f3661b7e56ece752e003fffa50f955de675d8e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 26 Mar 2019 17:40:24 +0100 Subject: [PATCH 45/52] more debug logging --- src/components/structures/ScrollPanel.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 57185b08b0f..99c2df1b142 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -572,6 +572,7 @@ module.exports = React.createClass({ // This because when setting the scrollTop only 10 or so events might be loaded, // not giving enough content below the trackedNode to scroll downwards // enough so it ends up in the top of the viewport. + debuglog("scrollToken: setting scrollTop", {offsetBase, pixelOffset, offsetTop: trackedNode.offsetTop}); scrollNode.scrollTop = (trackedNode.offsetTop - (scrollNode.clientHeight * offsetBase)) + pixelOffset; this._saveScrollState(); } From 5d53913e35c02768d5e2216faf4e5d10d864f826 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 27 Mar 2019 11:35:38 +0100 Subject: [PATCH 46/52] fix filling conditions --- src/components/structures/ScrollPanel.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 99c2df1b142..4043631d39f 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -317,17 +317,20 @@ module.exports = React.createClass({ this._isFilling = true; } - - const contentHeight = this._getMessagesHeight(); - const contentTop = contentHeight - this._getListHeight(); - const contentScrollTop = sn.scrollTop + contentTop; + const itemlist = this.refs.itemlist; + const firstTile = itemlist && itemlist.firstElementChild; + const contentTop = firstTile && firstTile.offsetTop; const fillPromises = []; - if (contentScrollTop < sn.clientHeight) { + // if scrollTop gets to 1 screen from the top of the first tile, + // try backward filling + if (!firstTile || (sn.scrollTop - contentTop) < sn.clientHeight) { // need to back-fill fillPromises.push(this._maybeFill(depth, true)); } - if (contentScrollTop > contentHeight - sn.clientHeight * 2) { + // if scrollTop gets to 2 screens from the end (so 1 screen below viewport), + // try forward filling + if ((sn.scrollHeight - sn.scrollTop) < sn.clientHeight * 2) { // need to forward-fill fillPromises.push(this._maybeFill(depth, false)); } From 82e44249ff57f77ee0b86834db6b19a713e07372 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 27 Mar 2019 16:38:17 +0100 Subject: [PATCH 47/52] make resizeNotifier optional in MainSplit for GroupView --- src/components/structures/MainSplit.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/structures/MainSplit.js b/src/components/structures/MainSplit.js index c0bf74d02c7..64f841da975 100644 --- a/src/components/structures/MainSplit.js +++ b/src/components/structures/MainSplit.js @@ -27,7 +27,9 @@ export default class MainSplit extends React.Component { _onResized(size) { window.localStorage.setItem("mx_rhs_size", size); - this.props.resizeNotifier.notifyRightHandleResized(); + if (this.props.resizeNotifier) { + this.props.resizeNotifier.notifyRightHandleResized(); + } } _createResizer() { From 41c5582a7b46835dbaae775569758b6d52e9d993 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 28 Mar 2019 17:56:12 +0100 Subject: [PATCH 48/52] make resizeNotifier optional so tests don't fail --- src/components/structures/RoomView.js | 8 ++++++-- src/components/views/rooms/RoomList.js | 9 +++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index ba6b54bdbca..cfc645abb42 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -392,7 +392,9 @@ module.exports = React.createClass({ this._updateConfCallNotification(); window.addEventListener('beforeunload', this.onPageUnload); - this.props.resizeNotifier.on("middlePanelResized", this.onResize); + if (this.props.resizeNotifier) { + this.props.resizeNotifier.on("middlePanelResized", this.onResize); + } this.onResize(); document.addEventListener("keydown", this.onKeyDown); @@ -472,7 +474,9 @@ module.exports = React.createClass({ } window.removeEventListener('beforeunload', this.onPageUnload); - this.props.resizeNotifier.removeListener("middlePanelResized", this.onResize); + if (this.props.resizeNotifier) { + this.props.resizeNotifier.removeListener("middlePanelResized", this.onResize); + } document.removeEventListener("keydown", this.onKeyDown); diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index 2de9918e6e3..33b97964f62 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -212,7 +212,9 @@ module.exports = React.createClass({ this._checkSubListsOverflow(); this.resizer.attach(); - this.props.resizeNotifier.on("leftPanelResized", this.onResize); + if (this.props.resizeNotifier) { + this.props.resizeNotifier.on("leftPanelResized", this.onResize); + } this.mounted = true; }, @@ -271,7 +273,10 @@ module.exports = React.createClass({ MatrixClientPeg.get().removeListener("Group.myMembership", this._onGroupMyMembership); MatrixClientPeg.get().removeListener("RoomState.events", this.onRoomStateEvents); } - this.props.resizeNotifier.removeListener("leftPanelResized", this.onResize); + + if (this.props.resizeNotifier) { + this.props.resizeNotifier.removeListener("leftPanelResized", this.onResize); + } if (this._tagStoreToken) { From 30dc6a9150078abc8ddb0156ba956b0dd6812d0a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 28 Mar 2019 17:57:08 +0100 Subject: [PATCH 49/52] remove tests relying on scrollpanel behaviour as BACAT scrolling relies on CSS from riot, which is not included in the karma tests, we're removing these tests in favor of later adding them to the e2e tests. --- .../components/structures/ScrollPanel-test.js | 283 ------------- .../structures/TimelinePanel-test.js | 372 ------------------ 2 files changed, 655 deletions(-) delete mode 100644 test/components/structures/ScrollPanel-test.js delete mode 100644 test/components/structures/TimelinePanel-test.js diff --git a/test/components/structures/ScrollPanel-test.js b/test/components/structures/ScrollPanel-test.js deleted file mode 100644 index 41d0f4469b1..00000000000 --- a/test/components/structures/ScrollPanel-test.js +++ /dev/null @@ -1,283 +0,0 @@ -/* -Copyright 2016 OpenMarket Ltd - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -const React = require('react'); -const ReactDOM = require("react-dom"); -const ReactTestUtils = require('react-addons-test-utils'); -const expect = require('expect'); -import Promise from 'bluebird'; -import { EventEmitter } from "events"; - -const sdk = require('matrix-react-sdk'); - -const ScrollPanel = sdk.getComponent('structures.ScrollPanel'); -const test_utils = require('test-utils'); - -const Tester = React.createClass({ - getInitialState: function() { - return { - tileKeys: [], - resizeNotifier: new EventEmitter(), - }; - }, - - componentWillMount: function() { - this.fillCounts = {'b': 0, 'f': 0}; - this._fillHandlers = {'b': null, 'f': null}; - this._fillDefers = {'b': null, 'f': null}; - this._scrollDefer = null; - - // scrollTop at the last scroll event - this.lastScrollEvent = null; - }, - - _onFillRequest: function(back) { - const dir = back ? 'b': 'f'; - console.log("FillRequest: " + dir); - this.fillCounts[dir]++; - - const handler = this._fillHandlers[dir]; - const defer = this._fillDefers[dir]; - - // don't use the same handler twice - this._fillHandlers[dir] = null; - this._fillDefers[dir] = null; - - let res; - if (handler) { - res = handler(); - } else { - res = Promise.resolve(false); - } - - if (defer) { - defer.resolve(); - } - return res; - }, - - addFillHandler: function(dir, handler) { - this._fillHandlers[dir] = handler; - }, - - /* returns a promise which will resolve when the fill happens */ - awaitFill: function(dir) { - console.log("ScrollPanel Tester: awaiting " + dir + " fill"); - const defer = Promise.defer(); - this._fillDefers[dir] = defer; - return defer.promise; - }, - - _onScroll: function(ev) { - const st = ev.target.scrollTop; - console.log("ScrollPanel Tester: scroll event; scrollTop: " + st); - this.lastScrollEvent = st; - - const d = this._scrollDefer; - if (d) { - this._scrollDefer = null; - d.resolve(); - } - }, - - /* returns a promise which will resolve when a scroll event happens */ - awaitScroll: function() { - console.log("Awaiting scroll"); - this._scrollDefer = Promise.defer(); - return this._scrollDefer.promise; - }, - - setTileKeys: function(keys) { - console.log("Updating keys: len=" + keys.length); - this.setState({tileKeys: keys.slice()}); - }, - - scrollPanel: function() { - return this.refs.sp; - }, - - _mkTile: function(key) { - // each tile is 150 pixels high: - // 98 pixels of body - // 2 pixels of border - // 50 pixels of margin - // - // there is an extra 50 pixels of margin at the bottom. - return ( -
    1. -
      - { key } -
      -
      2. - ); - }, - - render: function() { - const tiles = this.state.tileKeys.map(this._mkTile); - console.log("rendering with " + tiles.length + " tiles"); - return ( - - { tiles } - - ); - }, -}); - -describe('ScrollPanel', function() { - let parentDiv; - let tester; - let scrollingDiv; - - beforeEach(function(done) { - test_utils.beforeEach(this); - - // create a div of a useful size to put our panel in, and attach it to - // the document so that we can interact with it properly. - parentDiv = document.createElement('div'); - parentDiv.style.width = '800px'; - parentDiv.style.height = '600px'; - parentDiv.style.overflow = 'hidden'; - document.body.appendChild(parentDiv); - - tester = ReactDOM.render(, parentDiv); - expect(tester.fillCounts.b).toEqual(1); - expect(tester.fillCounts.f).toEqual(1); - - scrollingDiv = ReactTestUtils.findRenderedDOMComponentWithClass( - tester, "gm-scroll-view"); - - // we need to make sure we don't call done() until q has finished - // running the completion handlers from the fill requests. We can't - // just use .done(), because that will end up ahead of those handlers - // in the queue. We can't use window.setTimeout(0), because that also might - // run ahead of those handlers. - const sp = tester.scrollPanel(); - let retriesRemaining = 1; - const awaitReady = function() { - return Promise.resolve().then(() => { - if (sp._pendingFillRequests.b === false && - sp._pendingFillRequests.f === false - ) { - return; - } - - if (retriesRemaining == 0) { - throw new Error("fillRequests did not complete"); - } - retriesRemaining--; - return awaitReady(); - }); - }; - awaitReady().done(done); - }); - - afterEach(function() { - if (parentDiv) { - document.body.removeChild(parentDiv); - parentDiv = null; - } - }); - - it('should handle scrollEvent strangeness', function() { - const events = []; - - return Promise.resolve().then(() => { - // initialise with a load of events - for (let i = 0; i < 20; i++) { - events.push(i+80); - } - tester.setTileKeys(events); - expect(scrollingDiv.scrollHeight).toEqual(3050); // 20*150 + 50 - expect(scrollingDiv.scrollTop).toEqual(3050 - 600); - return tester.awaitScroll(); - }).then(() => { - expect(tester.lastScrollEvent).toBe(3050 - 600); - - tester.scrollPanel().scrollToToken("92", 0); - - // at this point, ScrollPanel will have updated scrollTop, but - // the event hasn't fired. - expect(tester.lastScrollEvent).toEqual(3050 - 600); - expect(scrollingDiv.scrollTop).toEqual(1950); - - // now stamp over the scrollTop. - console.log('faking #528'); - scrollingDiv.scrollTop = 500; - - return tester.awaitScroll(); - }).then(() => { - expect(tester.lastScrollEvent).toBe(1950); - 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); - }); -}); diff --git a/test/components/structures/TimelinePanel-test.js b/test/components/structures/TimelinePanel-test.js deleted file mode 100644 index 01ea6d84212..00000000000 --- a/test/components/structures/TimelinePanel-test.js +++ /dev/null @@ -1,372 +0,0 @@ -/* -Copyright 2016 OpenMarket Ltd - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -const React = require('react'); -const ReactDOM = require('react-dom'); -const ReactTestUtils = require('react-addons-test-utils'); -const expect = require('expect'); -import Promise from 'bluebird'; -const sinon = require('sinon'); - -const jssdk = require('matrix-js-sdk'); -const EventTimeline = jssdk.EventTimeline; - -const sdk = require('matrix-react-sdk'); -const TimelinePanel = sdk.getComponent('structures.TimelinePanel'); -const peg = require('../../../src/MatrixClientPeg'); - -const test_utils = require('test-utils'); - -const ROOM_ID = '!room:localhost'; -const USER_ID = '@me:localhost'; - -// wrap TimelinePanel with a component which provides the MatrixClient in the context. -const WrappedTimelinePanel = React.createClass({ - childContextTypes: { - matrixClient: React.PropTypes.object, - }, - - getChildContext: function() { - return { - matrixClient: peg.get(), - }; - }, - - render: function() { - return ; - }, -}); - - -describe('TimelinePanel', function() { - let sandbox; - let timelineSet; - let room; - let client; - let timeline; - let parentDiv; - - // make a dummy message. eventNum is put in the message text to help - // identification during debugging, and also in the timestamp so that we - // don't get lots of events with the same timestamp. - function mkMessage(eventNum, opts) { - return test_utils.mkMessage( - { - event: true, room: ROOM_ID, user: USER_ID, - ts: Date.now() + eventNum, - msg: "Event " + eventNum, - ...opts, - }); - } - - function scryEventTiles(panel) { - return ReactTestUtils.scryRenderedComponentsWithType( - panel, sdk.getComponent('rooms.EventTile')); - } - - beforeEach(function() { - test_utils.beforeEach(this); - sandbox = test_utils.stubClient(sandbox); - - room = sinon.createStubInstance(jssdk.Room); - room.currentState = sinon.createStubInstance(jssdk.RoomState); - room.currentState.members = {}; - room.roomId = ROOM_ID; - - timelineSet = sinon.createStubInstance(jssdk.EventTimelineSet); - timelineSet.getPendingEvents.returns([]); - timelineSet.room = room; - - timeline = new jssdk.EventTimeline(timelineSet); - - timelineSet.getLiveTimeline.returns(timeline); - - client = peg.get(); - client.credentials = {userId: USER_ID}; - - // create a div of a useful size to put our panel in, and attach it to - // the document so that we can interact with it properly. - parentDiv = document.createElement('div'); - parentDiv.style.width = '800px'; - - // This has to be slightly carefully chosen. We expect to have to do - // exactly one pagination to fill it. - parentDiv.style.height = '500px'; - - parentDiv.style.overflow = 'hidden'; - document.body.appendChild(parentDiv); - }); - - afterEach(function() { - if (parentDiv) { - ReactDOM.unmountComponentAtNode(parentDiv); - parentDiv.remove(); - parentDiv = null; - } - sandbox.restore(); - }); - - it('should load new events even if you are scrolled up', function(done) { - // this is https://github.com/vector-im/vector-web/issues/1367 - - // enough events to allow us to scroll back - const N_EVENTS = 30; - for (let i = 0; i < N_EVENTS; i++) { - timeline.addEvent(mkMessage(i)); - } - - let scrollDefer; - const onScroll = (e) => { - console.log(`TimelinePanel called onScroll: ${e.target.scrollTop}`); - if (scrollDefer) { - scrollDefer.resolve(); - } - }; - const rendered = ReactDOM.render( - , - parentDiv, - ); - const panel = rendered.refs.panel; - const scrollingDiv = ReactTestUtils.findRenderedDOMComponentWithClass( - panel, "gm-scroll-view"); - - // helper function which will return a promise which resolves when the - // panel isn't paginating - var awaitPaginationCompletion = function() { - if(!panel.state.forwardPaginating) {return Promise.resolve();} else {return Promise.delay(0).then(awaitPaginationCompletion);} - }; - - // helper function which will return a promise which resolves when - // the TimelinePanel fires a scroll event - const awaitScroll = function() { - scrollDefer = Promise.defer(); - return scrollDefer.promise; - }; - - // let the first round of pagination finish off - Promise.delay(5).then(() => { - expect(panel.state.canBackPaginate).toBe(false); - expect(scryEventTiles(panel).length).toEqual(N_EVENTS); - - // scroll up - console.log("setting scrollTop = 0"); - scrollingDiv.scrollTop = 0; - - // wait for the scroll event to land - }).then(awaitScroll).then(() => { - expect(scrollingDiv.scrollTop).toEqual(0); - - // there should be no pagination going on now - expect(panel.state.backPaginating).toBe(false); - expect(panel.state.forwardPaginating).toBe(false); - expect(panel.state.canBackPaginate).toBe(false); - expect(panel.state.canForwardPaginate).toBe(false); - expect(panel.isAtEndOfLiveTimeline()).toBe(false); - expect(scrollingDiv.scrollTop).toEqual(0); - - console.log("adding event"); - - // a new event! - const ev = mkMessage(N_EVENTS+1); - timeline.addEvent(ev); - panel.onRoomTimeline(ev, room, false, false, { - liveEvent: true, - timeline: timeline, - }); - - // that won't make much difference, because we don't paginate - // unless we're at the bottom of the timeline, but a scroll event - // should be enough to set off a pagination. - expect(scryEventTiles(panel).length).toEqual(N_EVENTS); - - scrollingDiv.scrollTop = 10; - - return awaitScroll(); - }).then(awaitPaginationCompletion).then(() => { - expect(scryEventTiles(panel).length).toEqual(N_EVENTS+1); - }).done(done, done); - }); - - it('should not paginate forever if there are no events', function(done) { - // start with a handful of events in the timeline, as would happen when - // joining a room - const d = Date.now(); - for (let i = 0; i < 3; i++) { - timeline.addEvent(mkMessage(i)); - } - timeline.setPaginationToken('tok', EventTimeline.BACKWARDS); - - // back-pagination returns a promise for true, but adds no events - client.paginateEventTimeline = sinon.spy((tl, opts) => { - console.log("paginate:", opts); - expect(opts.backwards).toBe(true); - return Promise.resolve(true); - }); - - const rendered = ReactDOM.render( - , - parentDiv, - ); - const panel = rendered.refs.panel; - - const messagePanel = ReactTestUtils.findRenderedComponentWithType( - panel, sdk.getComponent('structures.MessagePanel')); - - expect(messagePanel.props.backPaginating).toBe(true); - - // let the first round of pagination finish off - setTimeout(() => { - // at this point, the timeline window should have tried to paginate - // 5 times, and we should have given up paginating - expect(client.paginateEventTimeline.callCount).toEqual(5); - expect(messagePanel.props.backPaginating).toBe(false); - expect(messagePanel.props.suppressFirstDateSeparator).toBe(false); - - // now, if we update the events, there shouldn't be any - // more requests. - client.paginateEventTimeline.resetHistory(); - panel.forceUpdate(); - expect(messagePanel.props.backPaginating).toBe(false); - setTimeout(() => { - expect(client.paginateEventTimeline.callCount).toEqual(0); - done(); - }, 0); - }, 10); - }); - - it("should let you scroll down to the bottom after you've scrolled up", function(done) { - const N_EVENTS = 120; // the number of events to simulate being added to the timeline - - // sadly, loading all those events takes a while - this.timeout(N_EVENTS * 50); - - // 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 (let i = 0; i < N_EVENTS; i++) { - timeline.addEvent(mkMessage(i)); - } - console.log("added events to timeline"); - - let scrollDefer; - const rendered = ReactDOM.render( - {scrollDefer.resolve();}} />, - parentDiv, - ); - console.log("TimelinePanel rendered"); - const panel = rendered.refs.panel; - const messagePanel = ReactTestUtils.findRenderedComponentWithType( - panel, sdk.getComponent('structures.MessagePanel')); - const scrollingDiv = ReactTestUtils.findRenderedDOMComponentWithClass( - panel, "gm-scroll-view"); - - // helper function which will return a promise which resolves when - // the TimelinePanel fires a scroll event - const awaitScroll = function() { - scrollDefer = Promise.defer(); - - return scrollDefer.promise.then(() => { - console.log("got scroll event; scrollTop now " + - scrollingDiv.scrollTop); - }); - }; - - function setScrollTop(scrollTop) { - const before = scrollingDiv.scrollTop; - scrollingDiv.scrollTop = scrollTop; - console.log("setScrollTop: before update: " + before + - "; assigned: " + scrollTop + - "; after update: " + scrollingDiv.scrollTop); - } - - function backPaginate() { - console.log("back paginating..."); - setScrollTop(0); - return awaitScroll().then(() => { - const eventTiles = scryEventTiles(panel); - const firstEvent = eventTiles[0].props.mxEvent; - - console.log("TimelinePanel contains " + eventTiles.length + - " events; first is " + - firstEvent.getContent().body); - - if(scrollingDiv.scrollTop > 0) { - // need to go further - return backPaginate(); - } - console.log("paginated to start."); - }); - } - - function scrollDown() { - // Scroll the bottom of the viewport to the bottom of the panel - setScrollTop(scrollingDiv.scrollHeight - scrollingDiv.clientHeight); - console.log("scrolling down... " + scrollingDiv.scrollTop); - return awaitScroll().delay(0).then(() => { - const eventTiles = scryEventTiles(panel); - const events = timeline.getEvents(); - - const lastEventInPanel = eventTiles[eventTiles.length - 1].props.mxEvent; - const lastEventInTimeline = events[events.length - 1]; - - // Scroll until the last event in the panel = the last event in the timeline - if(lastEventInPanel.getId() !== lastEventInTimeline.getId()) { - // need to go further - return scrollDown(); - } - console.log("paginated to end."); - }); - } - - // 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(() => { - // hopefully, we got to the start of the timeline - expect(messagePanel.props.backPaginating).toBe(false); - - expect(messagePanel.props.suppressFirstDateSeparator).toBe(false); - const events = scryEventTiles(panel); - expect(events[0].props.mxEvent).toBe(timeline.getEvents()[0]); - - // 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(); - }).then(() => { - expect(messagePanel.props.backPaginating).toBe(false); - expect(messagePanel.props.forwardPaginating).toBe(false); - - const events = scryEventTiles(panel); - - // Expect to be able to see the most recent event - const lastEventInPanel = events[events.length - 1].props.mxEvent; - const lastEventInTimeline = timeline.getEvents()[timeline.getEvents().length - 1]; - expect(lastEventInPanel.getContent()).toBe(lastEventInTimeline.getContent()); - - console.log("done"); - }).done(done, done); - }); -}); From 0a4ef44bcf8a1e92f6a6c998531e9eeb5b144e35 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 28 Mar 2019 18:29:48 +0100 Subject: [PATCH 50/52] fix lint --- src/components/views/rooms/WhoIsTypingTile.js | 3 +-- src/utils/ResizeNotifier.js | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/views/rooms/WhoIsTypingTile.js b/src/components/views/rooms/WhoIsTypingTile.js index 95cf0717c7d..eb5e14876db 100644 --- a/src/components/views/rooms/WhoIsTypingTile.js +++ b/src/components/views/rooms/WhoIsTypingTile.js @@ -64,8 +64,7 @@ module.exports = React.createClass({ const isVisible = this._isVisible(this.state); if (this.props.onShown && !wasVisible && isVisible) { this.props.onShown(); - } - else if (this.props.onHidden && wasVisible && !isVisible) { + } else if (this.props.onHidden && wasVisible && !isVisible) { this.props.onHidden(); } }, diff --git a/src/utils/ResizeNotifier.js b/src/utils/ResizeNotifier.js index ff4b79091be..35ec1a02692 100644 --- a/src/utils/ResizeNotifier.js +++ b/src/utils/ResizeNotifier.js @@ -22,7 +22,6 @@ import { EventEmitter } from "events"; import { throttle } from "lodash"; export default class ResizeNotifier extends EventEmitter { - constructor() { super(); // with default options, will call fn once at first call, and then every x ms From 1a264006f774bcbd2423d3cdd6d0914f4d722179 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 28 Mar 2019 18:42:49 +0100 Subject: [PATCH 51/52] turn off debug logging --- src/components/structures/ScrollPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 4043631d39f..bb01da03da1 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -21,7 +21,7 @@ import { KeyCode } from '../../Keyboard'; import Timer from '../../utils/Timer'; import AutoHideScrollbar from "./AutoHideScrollbar"; -const DEBUG_SCROLL = true; +const DEBUG_SCROLL = false; // The amount of extra scroll distance to allow prior to unfilling. // See _getExcessHeight. From f15783886432c1439d139ec874eeab6e24e4e0b9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 29 Mar 2019 14:32:26 +0100 Subject: [PATCH 52/52] dispatch supports async dispatching on its own --- src/components/structures/MatrixChat.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index b7a5b943735..83492c2ac5e 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -1702,10 +1702,7 @@ export default React.createClass({ }, _dispatchTimelineResize() { - // prevent dispatch from within dispatch error - setTimeout(() => { - dis.dispatch({ action: 'timeline_resize' }, true); - }, 0); + dis.dispatch({ action: 'timeline_resize' }); }, onRoomCreated: function(roomId) {