From 12131c8e21a95c6e9ce48463083c76d1c7da1576 Mon Sep 17 00:00:00 2001 From: Natalie Tay Date: Tue, 26 Dec 2023 14:31:49 +0800 Subject: [PATCH] FIX: CLS jumpiness in post-stream when ?page=N (#25034) With certain conditions, this issue does not show up. The easiest way to reproduce this is probably to do either of this - Use a 3G slow connection or; - Add a breakpoint to scrolling-post-stream.topRefresh (anon) - (and optionally lock-on.lock) This issue is happening because there are multiple areas that set scroll location in the post stream when loading a topic. In our case, sometimes lock-on is triggering and scrolling to post_1, before ?page=2's post_21 is being scrolled to, due to posts above post_21 can finishing loading at different times. This causes some calculations to not add up, as being in the middle of a post stream has different calculations than being at the top of the post stream. --- .../app/components/scrolling-post-stream.js | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js b/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js index d4635a4f8f1..ed6c637f7d1 100644 --- a/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js +++ b/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js @@ -181,10 +181,7 @@ export default MountWidget.extend({ const first = posts.objectAt(onscreen[0]); if (this._topVisible !== first) { this._topVisible = first; - const elem = postsNodes.item(onscreen[0]); - const elemId = elem.id; - const elemPos = domUtils.position(elem); - const distToElement = elemPos?.top || 0; + const elemId = postsNodes.item(onscreen[0]).id; const topRefresh = () => { refresh(() => { @@ -194,16 +191,30 @@ export default MountWidget.extend({ return; } - const position = domUtils.position(refreshedElem); - const top = position.top - distToElement; - document.documentElement.scroll({ top, left: 0 }); + // The getOffsetTop function calculates the total offset distance of + // an element from the top of the document. Unlike element.offsetTop + // which only returns the offset relative to its nearest positioned + // ancestor, this function recursively accumulates the offsetTop + // of an element and all of its offset parents (ancestors). + // This ensures the total distance is measured from the very top of + // the document, accounting for any nested elements and their + // respective offsets. + const getOffsetTop = (element) => { + if (!element) { + return 0; + } + return element.offsetTop + getOffsetTop(element.offsetParent); + }; + + const top = getOffsetTop(refreshedElem) - offsetCalculator(); + window.scrollTo({ top }); // This seems weird, but somewhat infrequently a rerender // will cause the browser to scroll to the top of the document // in Chrome. This makes sure the scroll works correctly if that // happens. schedule("afterRender", () => { - document.documentElement.scroll({ top, left: 0 }); + window.scrollTo({ top }); }); }); };