From dd33ad11f566db40289575a6e142122b102396e6 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Thu, 23 Jan 2025 15:07:24 +0530 Subject: [PATCH] scroll_util: Account for `$elem` not direct child of scroll container. When using `position` to get the scroll offset, it uses `offsetParent` to calculate the offset which is not necessary the scroll container. To account for this case, we use `offset` to correctly calculate the position of element relative to the document. --- web/src/scroll_util.ts | 9 ++++++++- web/tests/scroll_util.test.cjs | 7 +++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/web/src/scroll_util.ts b/web/src/scroll_util.ts index 1d85821720..827d74cd0c 100644 --- a/web/src/scroll_util.ts +++ b/web/src/scroll_util.ts @@ -77,7 +77,14 @@ export function scroll_element_into_container( // this will be non-intrusive to users when they already have // the element visible. $container = get_scroll_element($container); - const elem_top = $elem.position().top - sticky_header_height; + + // To correctly compute the offset of the element's scroll + // position within our scroll container, we need to substract the + // scroll container's own offset within the document. + const elem_offset = $elem.offset()?.top ?? 0; + const container_offset = $container.offset()?.top ?? 0; + + const elem_top = elem_offset - container_offset - sticky_header_height; const elem_bottom = elem_top + ($elem.innerHeight() ?? 0); const container_height = ($container.height() ?? 0) - sticky_header_height; diff --git a/web/tests/scroll_util.test.cjs b/web/tests/scroll_util.test.cjs index 754ff30afa..cfbe0c7f3f 100644 --- a/web/tests/scroll_util.test.cjs +++ b/web/tests/scroll_util.test.cjs @@ -96,13 +96,16 @@ run_test("scroll_element_into_container", () => { top = arg; return this; }, + offset: () => ({ + top: 0, + }), __zjquery: true, }; })(); const $elem1 = { innerHeight: () => 25, - position: () => ({ + offset: () => ({ top: 0, }), }; @@ -111,7 +114,7 @@ run_test("scroll_element_into_container", () => { const $elem2 = { innerHeight: () => 15, - position: () => ({ + offset: () => ({ top: 250, }), };