From 6e208505674e4070f2bb1ffc1c0676dc5dee11de Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 12:17:51 -0600 Subject: [PATCH 1/3] Remove sanity check from requestAnimationFrame It should be in all major browsers as of years ago, and we use it unguarded elsewhere in the app. The performance benefits of it appear to be worthwhile enough to keep it, though it's not a perfect solution. --- src/components/structures/LeftPanel2.tsx | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index e92b0fa9ae..0875914b78 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -115,20 +115,12 @@ export default class LeftPanel2 extends React.Component { }; private handleStickyHeaders(list: HTMLDivElement) { - // TODO: Evaluate if this has any performance benefit or detriment. - // See https://github.com/vector-im/riot-web/issues/14035 - if (this.isDoingStickyHeaders) return; this.isDoingStickyHeaders = true; - if (window.requestAnimationFrame) { - window.requestAnimationFrame(() => { - this.doStickyHeaders(list); - this.isDoingStickyHeaders = false; - }); - } else { + window.requestAnimationFrame(() => { this.doStickyHeaders(list); this.isDoingStickyHeaders = false; - } + }); } private doStickyHeaders(list: HTMLDivElement) { From f9aca7c05e381022598f73953761b22d89546e2e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 14:36:55 -0600 Subject: [PATCH 2/3] Avoid bounding box usage in sticky headers & improve reliability We now use offsets and scroll information to determine where the headers should be stuck to, still supporting the transparent background. Some scroll jumps were originally introduced as part of the change in numbering, so they have been fixed here. By proxy, some additional scroll jump/instability should be fixed as well. This has a lingering problem of still causing a huge number of no-op UI updates though, which will be dealt with in a future commit. --- res/css/structures/_LeftPanel2.scss | 7 ++- res/css/views/rooms/_RoomSublist2.scss | 12 ++--- src/components/structures/LeftPanel2.tsx | 62 +++++++++++------------- 3 files changed, 39 insertions(+), 42 deletions(-) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index eaa22a3efa..b3f7fcc8ee 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -122,16 +122,19 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations } .mx_LeftPanel2_roomListWrapper { + // Create a flexbox to ensure the containing items cause appropriate overflow. display: flex; + flex-grow: 1; overflow: hidden; min-height: 0; + margin-top: 12px; // so we're not up against the search/filter - &.stickyBottom { + &.mx_LeftPanel2_roomListWrapper_stickyBottom { padding-bottom: 32px; } - &.stickyTop { + &.mx_LeftPanel2_roomListWrapper_stickyTop { padding-top: 32px; } } diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 30389a4ec5..1d13f25b8f 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -24,10 +24,6 @@ limitations under the License. margin-left: 8px; width: 100%; - &:first-child { - margin-top: 12px; // so we're not up against the search/filter - } - .mx_RoomSublist2_headerContainer { // Create a flexbox to make alignment easy display: flex; @@ -49,10 +45,15 @@ limitations under the License. padding-bottom: 8px; height: 24px; + // Hide the header container if the contained element is stickied. + // We don't use display:none as that causes the header to go away too. + &.mx_RoomSublist2_headerContainer_hasSticky { + height: 0; + } + .mx_RoomSublist2_stickable { flex: 1; max-width: 100%; - z-index: 2; // Prioritize headers in the visible list over sticky ones // Create a flexbox to make ordering easy display: flex; @@ -64,7 +65,6 @@ limitations under the License. // when sticky scrolls instead of collapses the list. &.mx_RoomSublist2_headerContainer_sticky { position: fixed; - z-index: 1; // over top of other elements, but still under the ones in the visible list height: 32px; // to match the header container // width set by JS } diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 0875914b78..316b590aea 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -124,42 +124,42 @@ export default class LeftPanel2 extends React.Component { } private doStickyHeaders(list: HTMLDivElement) { - const rlRect = list.getBoundingClientRect(); - const bottom = rlRect.bottom; - const top = rlRect.top; + const topEdge = list.scrollTop; + const bottomEdge = list.offsetHeight + list.scrollTop; const sublists = list.querySelectorAll(".mx_RoomSublist2"); - const headerRightMargin = 24; // calculated from margins and widths to align with non-sticky tiles - const headerStickyWidth = rlRect.width - headerRightMargin; + const headerRightMargin = 16; // calculated from margins and widths to align with non-sticky tiles + const headerStickyWidth = list.clientWidth - headerRightMargin; - let gotBottom = false; let lastTopHeader; + let firstBottomHeader; for (const sublist of sublists) { - const slRect = sublist.getBoundingClientRect(); - const header = sublist.querySelector(".mx_RoomSublist2_stickable"); + const headerContainer = header.parentElement; // .mx_RoomSublist2_headerContainer header.style.removeProperty("display"); // always clear display:none first + headerContainer.classList.remove("mx_RoomSublist2_headerContainer_hasSticky"); - if (slRect.top + HEADER_HEIGHT > bottom && !gotBottom) { - header.classList.add("mx_RoomSublist2_headerContainer_sticky"); - header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); - header.style.width = `${headerStickyWidth}px`; - header.style.removeProperty("top"); - gotBottom = true; - } else if (((slRect.top - (HEADER_HEIGHT * 0.6) + HEADER_HEIGHT) < top) || sublist === sublists[0]) { - // the header should become sticky once it is 60% or less out of view at the top. - // We also add HEADER_HEIGHT because the sticky header is put above the scrollable area, - // into the padding of .mx_LeftPanel2_roomListWrapper, - // by subtracting HEADER_HEIGHT from the top below. - // We also always try to make the first sublist header sticky. + // When an element is <=40% off screen, make it take over + const offScreenFactor = 0.4; + const isOffTop = (sublist.offsetTop + (offScreenFactor * HEADER_HEIGHT)) <= topEdge; + const isOffBottom = (sublist.offsetTop + (offScreenFactor * HEADER_HEIGHT)) >= bottomEdge; + + if (isOffTop || sublist === sublists[0]) { header.classList.add("mx_RoomSublist2_headerContainer_sticky"); header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); + headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); header.style.width = `${headerStickyWidth}px`; - header.style.top = `${rlRect.top - HEADER_HEIGHT}px`; + header.style.top = `${list.parentElement.offsetTop}px`; if (lastTopHeader) { lastTopHeader.style.display = "none"; } lastTopHeader = header; + } else if (isOffBottom && !firstBottomHeader) { + header.classList.add("mx_RoomSublist2_headerContainer_sticky"); + header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); + headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); + header.style.width = `${headerStickyWidth}px`; + firstBottomHeader = header; } else { header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); @@ -171,22 +171,16 @@ export default class LeftPanel2 extends React.Component { // add appropriate sticky classes to wrapper so it has // the necessary top/bottom padding to put the sticky header in - const listWrapper = list.parentElement; - if (gotBottom) { - listWrapper.classList.add("stickyBottom"); - } else { - listWrapper.classList.remove("stickyBottom"); - } + const listWrapper = list.parentElement; // .mx_LeftPanel2_roomListWrapper if (lastTopHeader) { - listWrapper.classList.add("stickyTop"); + listWrapper.classList.add("mx_LeftPanel2_roomListWrapper_stickyTop"); } else { - listWrapper.classList.remove("stickyTop"); + listWrapper.classList.remove("mx_LeftPanel2_roomListWrapper_stickyTop"); } - - // ensure scroll doesn't go above the gap left by the header of - // the first sublist always being sticky if no other header is sticky - if (list.scrollTop < HEADER_HEIGHT) { - list.scrollTop = HEADER_HEIGHT; + if (firstBottomHeader) { + listWrapper.classList.add("mx_LeftPanel2_roomListWrapper_stickyBottom"); + } else { + listWrapper.classList.remove("mx_LeftPanel2_roomListWrapper_stickyBottom"); } } From 74ca0618ac8e500fc95ef88615b8e652bc487450 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 14:53:52 -0600 Subject: [PATCH 3/3] Improve scrolling performance for sticky headers The layout updates are anecdotal based on devtools flagging the values which are "changing" even if they aren't. The scrolling feels better with this as well, though this might be placebo. --- src/components/structures/LeftPanel2.tsx | 90 +++++++++++++++++++----- 1 file changed, 74 insertions(+), 16 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 316b590aea..f1f1ffd01f 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -131,13 +131,19 @@ export default class LeftPanel2 extends React.Component { const headerRightMargin = 16; // calculated from margins and widths to align with non-sticky tiles const headerStickyWidth = list.clientWidth - headerRightMargin; + // We track which styles we want on a target before making the changes to avoid + // excessive layout updates. + const targetStyles = new Map(); + let lastTopHeader; let firstBottomHeader; for (const sublist of sublists) { const header = sublist.querySelector(".mx_RoomSublist2_stickable"); - const headerContainer = header.parentElement; // .mx_RoomSublist2_headerContainer header.style.removeProperty("display"); // always clear display:none first - headerContainer.classList.remove("mx_RoomSublist2_headerContainer_hasSticky"); // When an element is <=40% off screen, make it take over const offScreenFactor = 0.4; @@ -145,27 +151,79 @@ export default class LeftPanel2 extends React.Component { const isOffBottom = (sublist.offsetTop + (offScreenFactor * HEADER_HEIGHT)) >= bottomEdge; if (isOffTop || sublist === sublists[0]) { - header.classList.add("mx_RoomSublist2_headerContainer_sticky"); - header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); - headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); - header.style.width = `${headerStickyWidth}px`; - header.style.top = `${list.parentElement.offsetTop}px`; + targetStyles.set(header, { stickyTop: true }); if (lastTopHeader) { lastTopHeader.style.display = "none"; + targetStyles.set(lastTopHeader, { makeInvisible: true }); } lastTopHeader = header; } else if (isOffBottom && !firstBottomHeader) { - header.classList.add("mx_RoomSublist2_headerContainer_sticky"); - header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); - headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); - header.style.width = `${headerStickyWidth}px`; + targetStyles.set(header, { stickyBottom: true }); firstBottomHeader = header; } else { - header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); - header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); - header.classList.remove("mx_RoomSublist2_headerContainer_stickyBottom"); - header.style.removeProperty("width"); - header.style.removeProperty("top"); + targetStyles.set(header, {}); // nothing == clear + } + } + + // Run over the style changes and make them reality. We check to see if we're about to + // cause a no-op update, as adding/removing properties that are/aren't there cause + // layout updates. + for (const header of targetStyles.keys()) { + const style = targetStyles.get(header); + const headerContainer = header.parentElement; // .mx_RoomSublist2_headerContainer + + if (style.makeInvisible) { + // we will have already removed the 'display: none', so add it back. + header.style.display = "none"; + continue; // nothing else to do, even if sticky somehow + } + + if (style.stickyTop) { + if (!header.classList.contains("mx_RoomSublist2_headerContainer_stickyTop")) { + header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); + } + + const newTop = `${list.parentElement.offsetTop}px`; + if (header.style.top !== newTop) { + header.style.top = newTop; + } + } else if (style.stickyBottom) { + if (!header.classList.contains("mx_RoomSublist2_headerContainer_stickyBottom")) { + header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); + } + } + + if (style.stickyTop || style.stickyBottom) { + if (!header.classList.contains("mx_RoomSublist2_headerContainer_sticky")) { + header.classList.add("mx_RoomSublist2_headerContainer_sticky"); + } + if (!headerContainer.classList.contains("mx_RoomSublist2_headerContainer_hasSticky")) { + headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); + } + + const newWidth = `${headerStickyWidth}px`; + if (header.style.width !== newWidth) { + header.style.width = newWidth; + } + } else if (!style.stickyTop && !style.stickyBottom) { + if (header.classList.contains("mx_RoomSublist2_headerContainer_sticky")) { + header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); + } + if (header.classList.contains("mx_RoomSublist2_headerContainer_stickyTop")) { + header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); + } + if (header.classList.contains("mx_RoomSublist2_headerContainer_stickyBottom")) { + header.classList.remove("mx_RoomSublist2_headerContainer_stickyBottom"); + } + if (headerContainer.classList.contains("mx_RoomSublist2_headerContainer_hasSticky")) { + headerContainer.classList.remove("mx_RoomSublist2_headerContainer_hasSticky"); + } + if (header.style.width) { + header.style.removeProperty('width'); + } + if (header.style.top) { + header.style.removeProperty('top'); + } } }