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 b4fbc5406e9..a4f97e0efda 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 9fb872cd321..ce935ddf32a 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 ce935ddf32a..8a5c24f2e0d 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 7ef542a6e1d..0e113b36642 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 a328d478bcd..1098f96a765 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 863dfaaa930..578689b45c4 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 ce28c168b9e..3ab4a22396e 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 3ab4a22396e..2ce79bf5e17 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 2ce79bf5e17..3a0dd0395b1 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 8a5c24f2e0d..7a06cc3da5d 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 1098f96a765..47ae24ba0fd 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 (