From bb57be95f0feed84a45722ecb61b42e12957336a Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Tue, 12 Apr 2022 13:23:57 -0400 Subject: [PATCH] DEV: Cleanup `body.scrollTop` usage (#16445) All current browser treat the HTML document (not the body element) as the scrollable document element. Hence in all current browsers, `document.body.scrollTop` returns 0. This commit removes all usage of this property, because it is effectively 0. Co-authored-by: David Taylor --- .../app/components/scrolling-post-stream.js | 27 +++---------------- .../discourse/app/components/site-header.js | 3 +-- 2 files changed, 4 insertions(+), 26 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 a871521bcd..c6c088decb 100644 --- a/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js +++ b/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js @@ -58,24 +58,6 @@ export default MountWidget.extend({ ); }, - beforePatch() { - this.prevHeight = document.body.clientHeight; - this.prevScrollTop = document.body.scrollTop; - }, - - afterPatch() { - const height = document.body.clientHeight; - - // This hack is for when swapping out many cloaked views at once - // when using keyboard navigation. It could suddenly move the scroll - if ( - this.prevHeight === height && - document.body.scrollTop !== this.prevScrollTop - ) { - document.body.scroll({ left: 0, top: this.prevScrollTop }); - } - }, - scrolled() { if (this.isDestroyed || this.isDestroying) { return; @@ -103,8 +85,7 @@ export default MountWidget.extend({ const slack = Math.round(windowHeight * 5); const onscreen = []; const nearby = []; - const windowTop = - document.documentElement.scrollTop || document.body.scrollTop; + const windowTop = document.scrollingElement.scrollTop; const postsWrapperTop = domUtils.offset( document.querySelector(".posts-wrapper") ).top; @@ -203,9 +184,7 @@ export default MountWidget.extend({ const elem = postsNodes.item(onscreen[0]); const elemId = elem.id; const elemPos = domUtils.position(elem); - const distToElement = elemPos - ? document.body.scrollTop - elemPos.top - : 0; + const distToElement = elemPos?.top || 0; const topRefresh = () => { refresh(() => { @@ -214,7 +193,7 @@ export default MountWidget.extend({ // Quickly going back might mean the element is destroyed const position = domUtils.position(refreshedElem); if (position && position.top) { - let whereY = position.top + distToElement; + let whereY = position.top - distToElement; document.documentElement.scroll({ top: whereY, left: 0 }); // This seems weird, but somewhat infrequently a rerender diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js index 6035a5b583..1f0be66cf6 100644 --- a/app/assets/javascripts/discourse/app/components/site-header.js +++ b/app/assets/javascripts/discourse/app/components/site-header.js @@ -441,6 +441,5 @@ export default SiteHeaderComponent.extend({ export function headerTop() { const header = document.querySelector("header.d-header"); - const headerOffsetTop = header.offsetTop ? header.offsetTop : 0; - return headerOffsetTop - document.body.scrollTop; + return header.offsetTop ? header.offsetTop : 0; }