From b359a2edeef727ccc513ffdaf0058c6cae53d0ad Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 10:56:00 +0100 Subject: [PATCH 01/10] call header clicked callback after rerendering, so resizer has DOM nodes --- src/components/structures/RoomSubList.js | 5 +++-- src/components/views/rooms/RoomList.js | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/components/structures/RoomSubList.js b/src/components/structures/RoomSubList.js index b4fbc5406e..a4f97e0efd 100644 --- a/src/components/structures/RoomSubList.js +++ b/src/components/structures/RoomSubList.js @@ -110,8 +110,9 @@ const RoomSubList = React.createClass({ if (this.isCollapsableOnClick()) { // The header isCollapsable, so the click is to be interpreted as collapse and truncation logic const isHidden = !this.state.hidden; - this.setState({hidden: isHidden}); - this.props.onHeaderClick(isHidden); + this.setState({hidden: isHidden}, () => { + this.props.onHeaderClick(isHidden); + }); } else { // The header is stuck, so the click is to be interpreted as a scroll to the header this.props.onHeaderClick(this.state.hidden, this.refs.header.dataset.originalPosition); diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index 9fb872cd32..ce935ddf32 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -485,9 +485,21 @@ module.exports = React.createClass({ (filter[0] === '#' && room.getAliases().some((alias) => alias.toLowerCase().startsWith(lcFilter)))); }, - _persistCollapsedState: function(key, collapsed) { + _handleCollapsedState: function(key, collapsed) { + // persist collapsed state this.collapsedState[key] = collapsed; window.localStorage.setItem("mx_roomlist_collapsed", JSON.stringify(this.collapsedState)); + // load the persisted size configuration of the expanded sub list + if (!collapsed) { + const size = this.subListSizes[key]; + const handle = this.resizer.forHandleWithId(key); + if (handle) { + handle.resize(size); + } + } + // check overflow, as sub lists sizes have changed + // important this happens after calling resize above + Object.values(this._subListRefs).forEach(l => l.checkOverflow()); }, _subListRef: function(key, ref) { @@ -520,7 +532,7 @@ module.exports = React.createClass({ const {key, label, onHeaderClick, ... otherProps} = props; const chosenKey = key || label; const onSubListHeaderClick = (collapsed) => { - this._persistCollapsedState(chosenKey, collapsed); + this._handleCollapsedState(chosenKey, collapsed); if (onHeaderClick) { onHeaderClick(collapsed); } From cdcb3c1a55dfc1f2affdbe20a5cd8001a6873e03 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 14:26:33 +0100 Subject: [PATCH 02/10] check overflow and restore sizes in more places inside RoomList: check overflow on mount restore size on query change (in case a sublist appeared) check overflow when updating rooms avoid duplicating for restoring size and checking overflow --- src/components/views/rooms/RoomList.js | 41 +++++++++++++++++++------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index ce935ddf32..8a5c24f2e0 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -152,6 +152,8 @@ module.exports = React.createClass({ } this.subListSizes[id] = newSize; window.localStorage.setItem("mx_roomlist_sizes", JSON.stringify(this.subListSizes)); + // update overflow indicators + this._checkSubListsOverflow(); }, componentDidMount: function() { @@ -167,12 +169,10 @@ module.exports = React.createClass({ }); // load stored sizes - Object.entries(this.subListSizes).forEach(([id, size]) => { - const handle = this.resizer.forHandleWithId(id); - if (handle) { - handle.resize(size); - } + Object.keys(this.subListSizes).forEach((key) => { + this._restoreSubListSize(key); }); + this._checkSubListsOverflow(); this.resizer.attach(); this.mounted = true; @@ -181,7 +181,11 @@ module.exports = React.createClass({ componentDidUpdate: function(prevProps) { this._repositionIncomingCallBox(undefined, false); if (this.props.searchFilter !== prevProps.searchFilter) { - Object.values(this._subListRefs).forEach(l => l.checkOverflow()); + // restore sizes + Object.keys(this.subListSizes).forEach((key) => { + this._restoreSubListSize(key); + }); + this._checkSubListsOverflow(); } }, @@ -354,6 +358,12 @@ module.exports = React.createClass({ // Do this here so as to not render every time the selected tags // themselves change. selectedTags: TagOrderStore.getSelectedTags(), + }, () => { + // we don't need to restore any size here, do we? + // i guess we could have triggered a new group to appear + // that already an explicit size the last time it appeared ... + // + this._checkSubListsOverflow(); }); // this._lastRefreshRoomListTs = Date.now(); @@ -491,14 +501,23 @@ module.exports = React.createClass({ window.localStorage.setItem("mx_roomlist_collapsed", JSON.stringify(this.collapsedState)); // load the persisted size configuration of the expanded sub list if (!collapsed) { - const size = this.subListSizes[key]; - const handle = this.resizer.forHandleWithId(key); - if (handle) { - handle.resize(size); - } + this._restoreSubListSize(key); } // check overflow, as sub lists sizes have changed // important this happens after calling resize above + this._checkSubListsOverflow(); + }, + + _restoreSubListSize(key) { + const size = this.subListSizes[key]; + const handle = this.resizer.forHandleWithId(key); + if (handle) { + handle.resize(size); + } + }, + + // check overflow for scroll indicator gradient + _checkSubListsOverflow() { Object.values(this._subListRefs).forEach(l => l.checkOverflow()); }, From 3ddc8baed138ab23c180b1ec31a8ee91cc0f2188 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 14:27:10 +0100 Subject: [PATCH 03/10] fix resizing sometimes not working (and selecting text) Last friday a child
was added inside the ResizeHandle component, which made the parentElement/classList checks fail on the event.target here. This would only fail (and select all the text) when dragging exactly on the grey line (the div), not the transparent margin around it. use closest to make sure we have the root element of the handle. --- src/resizer/resizer.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/resizer/resizer.js b/src/resizer/resizer.js index 7ef542a6e1..0e113b3664 100644 --- a/src/resizer/resizer.js +++ b/src/resizer/resizer.js @@ -84,8 +84,10 @@ export class Resizer { } _onMouseDown(event) { - const target = event.target; - if (!this._isResizeHandle(target) || target.parentElement !== this.container) { + // use closest in case the resize handle contains + // child dom nodes that can be the target + const resizeHandle = event.target && event.target.closest(`.${this.classNames.handle}`); + if (!resizeHandle || resizeHandle.parentElement !== this.container) { return; } // prevent starting a drag operation @@ -96,7 +98,7 @@ export class Resizer { this.container.classList.add(this.classNames.resizing); } - const {sizer, distributor} = this._createSizerAndDistributor(target); + const {sizer, distributor} = this._createSizerAndDistributor(resizeHandle); const onMouseMove = (event) => { const offset = sizer.offsetFromEvent(event); From e67d9c6d4f04bc518f18af413bc72252c4b2b3f2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 14:29:42 +0100 Subject: [PATCH 04/10] forward checkOverflow to AutoHideScrollbar, fix over/underflow detection the overflow/underflow events are not always reliable in nooverlay browsers (FF), so forward the checkOverflow call we need anyway for the scroll indicator gradients to see if we need to do the margin trick for the on-hover scrollbar we use in nooverlay browsers. this fixes on hover jumping in a subroomlist --- .../structures/AutoHideScrollbar.js | 33 ++++++++++++----- .../structures/IndicatorScrollbar.js | 37 ++++++++++++------- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/components/structures/AutoHideScrollbar.js b/src/components/structures/AutoHideScrollbar.js index a328d478bc..1098f96a76 100644 --- a/src/components/structures/AutoHideScrollbar.js +++ b/src/components/structures/AutoHideScrollbar.js @@ -69,6 +69,7 @@ export default class AutoHideScrollbar extends React.Component { this.onOverflow = this.onOverflow.bind(this); this.onUnderflow = this.onUnderflow.bind(this); this._collectContainerRef = this._collectContainerRef.bind(this); + this._needsOverflowListener = null; } onOverflow() { @@ -81,21 +82,32 @@ export default class AutoHideScrollbar extends React.Component { this.containerRef.classList.add("mx_AutoHideScrollbar_underflow"); } + checkOverflow() { + if (!this._needsOverflowListener) { + return; + } + if (this.containerRef.scrollHeight > this.containerRef.clientHeight) { + this.onOverflow(); + } else { + this.onUnderflow(); + } + } + + componentDidUpdate() { + this.checkOverflow(); + } + + componentDidMount() { + this.checkOverflow(); + } + _collectContainerRef(ref) { if (ref && !this.containerRef) { this.containerRef = ref; - const needsOverflowListener = - document.body.classList.contains("mx_scrollbar_nooverlay"); - - if (needsOverflowListener) { + if (this._needsOverflowListener) { this.containerRef.addEventListener("overflow", this.onOverflow); this.containerRef.addEventListener("underflow", this.onUnderflow); } - if (ref.scrollHeight > ref.clientHeight) { - this.onOverflow(); - } else { - this.onUnderflow(); - } } if (this.props.wrappedRef) { this.props.wrappedRef(ref); @@ -111,6 +123,9 @@ export default class AutoHideScrollbar extends React.Component { render() { installBodyClassesIfNeeded(); + if (this._needsOverflowListener === null) { + this._needsOverflowListener = document.body.classList.contains("mx_scrollbar_nooverlay"); + } return (
0; - const hasBottomOverflow = this._scroller.scrollHeight > - (this._scroller.scrollTop + this._scroller.clientHeight); + const hasTopOverflow = this._scrollElement.scrollTop > 0; + const hasBottomOverflow = this._scrollElement.scrollHeight > + (this._scrollElement.scrollTop + this._scrollElement.clientHeight); if (hasTopOverflow) { - this._scroller.classList.add("mx_IndicatorScrollbar_topOverflow"); + this._scrollElement.classList.add("mx_IndicatorScrollbar_topOverflow"); } else { - this._scroller.classList.remove("mx_IndicatorScrollbar_topOverflow"); + this._scrollElement.classList.remove("mx_IndicatorScrollbar_topOverflow"); } if (hasBottomOverflow) { - this._scroller.classList.add("mx_IndicatorScrollbar_bottomOverflow"); + this._scrollElement.classList.add("mx_IndicatorScrollbar_bottomOverflow"); } else { - this._scroller.classList.remove("mx_IndicatorScrollbar_bottomOverflow"); + this._scrollElement.classList.remove("mx_IndicatorScrollbar_bottomOverflow"); + } + + if (this._autoHideScrollbar) { + this._autoHideScrollbar.checkOverflow(); } } componentWillUnmount() { - if (this._scroller) { - this._scroller.removeEventListener("scroll", this.checkOverflow); + if (this._scrollElement) { + this._scrollElement.removeEventListener("scroll", this.checkOverflow); } } render() { - return ( + return ( { this.props.children } ); } From 279521cab4ddfab78e57ced2ebffb02b12d2955c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 14:31:38 +0100 Subject: [PATCH 05/10] add id to props for completeness --- src/components/views/elements/ResizeHandle.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/elements/ResizeHandle.js b/src/components/views/elements/ResizeHandle.js index 863dfaaa93..578689b45c 100644 --- a/src/components/views/elements/ResizeHandle.js +++ b/src/components/views/elements/ResizeHandle.js @@ -21,6 +21,7 @@ const ResizeHandle = (props) => { ResizeHandle.propTypes = { vertical: PropTypes.bool, reverse: PropTypes.bool, + id: PropTypes.string, }; export default ResizeHandle; From affe75fd3fab281326086af320dfe3b5fa731425 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 14:31:59 +0100 Subject: [PATCH 06/10] make scroll indicator gradient smaller (40px->30px) --- res/css/structures/_RoomSubList.scss | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/res/css/structures/_RoomSubList.scss b/res/css/structures/_RoomSubList.scss index ce28c168b9..3ab4a22396 100644 --- a/res/css/structures/_RoomSubList.scss +++ b/res/css/structures/_RoomSubList.scss @@ -154,7 +154,7 @@ limitations under the License. position: sticky; left: 0; right: 0; - height: 40px; + height: 30px; content: ""; display: block; z-index: 100; @@ -162,10 +162,10 @@ limitations under the License. } &.mx_IndicatorScrollbar_topOverflow > .mx_AutoHideScrollbar_offset { - margin-top: -40px; + margin-top: -30px; } &.mx_IndicatorScrollbar_bottomOverflow > .mx_AutoHideScrollbar_offset { - margin-bottom: -40px; + margin-bottom: -30px; } &.mx_IndicatorScrollbar_topOverflow::before { From 12a339fe10c7e1e76917328e95a2429e649b2a28 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 14:32:26 +0100 Subject: [PATCH 07/10] change subroomlist min height, as roomtiles are smaller now --- res/css/structures/_RoomSubList.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/res/css/structures/_RoomSubList.scss b/res/css/structures/_RoomSubList.scss index 3ab4a22396..2ce79bf5e1 100644 --- a/res/css/structures/_RoomSubList.scss +++ b/res/css/structures/_RoomSubList.scss @@ -39,7 +39,7 @@ limitations under the License. } .mx_RoomSubList_nonEmpty { - min-height: 76px; + min-height: 70px; .mx_AutoHideScrollbar_offset { padding-bottom: 4px; From f0a412e7214f8f8e51a31a7470ea2fc5849059f1 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 14:32:46 +0100 Subject: [PATCH 08/10] fix docs --- res/css/structures/_RoomSubList.scss | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/res/css/structures/_RoomSubList.scss b/res/css/structures/_RoomSubList.scss index 2ce79bf5e1..3a0dd0395b 100644 --- a/res/css/structures/_RoomSubList.scss +++ b/res/css/structures/_RoomSubList.scss @@ -19,14 +19,14 @@ limitations under the License. each with a flex-shrink difference of 4 order of magnitude, so they ideally wouldn't affect each other. lowest category: .mx_RoomSubList - flex:-shrink: 10000000 + flex-shrink: 10000000 distribute size of items within the same categery by their size middle category: .mx_RoomSubList.resized-sized - flex:-shrink: 1000 + flex-shrink: 1000 applied when using the resizer, will have a max-height set to it, to limit the size highest category: .mx_RoomSubList.resized-all - flex:-shrink: 1 + flex-shrink: 1 small flex-shrink value (1), is only added if you can drag the resizer so far so in practice you can only assign this category if there is enough space. */ From 2cda6d8f35cc58a0d59d945b167c9c9b4da14d24 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 15:06:56 +0100 Subject: [PATCH 09/10] fix empty comment line --- src/components/views/rooms/RoomList.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index 8a5c24f2e0..7a06cc3da5 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -362,7 +362,6 @@ module.exports = React.createClass({ // we don't need to restore any size here, do we? // i guess we could have triggered a new group to appear // that already an explicit size the last time it appeared ... - // this._checkSubListsOverflow(); }); From 31c13adaba2da09ccb65d5bf6cb6ab1fe2c76b5a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Dec 2018 15:10:57 +0100 Subject: [PATCH 10/10] cleanup: do initialization in componentDidMount instead of render --- src/components/structures/AutoHideScrollbar.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/components/structures/AutoHideScrollbar.js b/src/components/structures/AutoHideScrollbar.js index 1098f96a76..47ae24ba0f 100644 --- a/src/components/structures/AutoHideScrollbar.js +++ b/src/components/structures/AutoHideScrollbar.js @@ -98,16 +98,19 @@ export default class AutoHideScrollbar extends React.Component { } componentDidMount() { + installBodyClassesIfNeeded(); + this._needsOverflowListener = + document.body.classList.contains("mx_scrollbar_nooverlay"); + if (this._needsOverflowListener) { + this.containerRef.addEventListener("overflow", this.onOverflow); + this.containerRef.addEventListener("underflow", this.onUnderflow); + } this.checkOverflow(); } _collectContainerRef(ref) { if (ref && !this.containerRef) { this.containerRef = ref; - if (this._needsOverflowListener) { - this.containerRef.addEventListener("overflow", this.onOverflow); - this.containerRef.addEventListener("underflow", this.onUnderflow); - } } if (this.props.wrappedRef) { this.props.wrappedRef(ref); @@ -115,17 +118,13 @@ export default class AutoHideScrollbar extends React.Component { } componentWillUnmount() { - if (this.containerRef) { + if (this._needsOverflowListener && this.containerRef) { this.containerRef.removeEventListener("overflow", this.onOverflow); this.containerRef.removeEventListener("underflow", this.onUnderflow); } } render() { - installBodyClassesIfNeeded(); - if (this._needsOverflowListener === null) { - this._needsOverflowListener = document.body.classList.contains("mx_scrollbar_nooverlay"); - } return (