diff --git a/packages/main/src/components/ObjectPage/index.tsx b/packages/main/src/components/ObjectPage/index.tsx index bceee0fd3b2..982ffecd8d9 100644 --- a/packages/main/src/components/ObjectPage/index.tsx +++ b/packages/main/src/components/ObjectPage/index.tsx @@ -102,7 +102,7 @@ const ObjectPage = forwardRef((props, ref const isProgrammaticallyScrolled = useRef(false); const [componentRef, objectPageRef] = useSyncRef(ref); const topHeaderRef = useRef(null); - const prevTopHeaderHeight = useRef(0); + const pendingScrollTargetRef = useRef(null); // @ts-expect-error: useSyncRef will create a ref if not present const [componentRefHeaderContent, headerContentRef] = useSyncRef(headerArea?.ref); const scrollEvent = useRef(undefined); @@ -278,7 +278,8 @@ const ObjectPage = forwardRef((props, ref return; } - const safeTopHeaderHeight = topHeaderHeight || prevTopHeaderHeight.current; + // header collapses in this commit but topHeaderHeight state lags a tick, so measure live + const safeTopHeaderHeight = topHeaderRef.current?.getBoundingClientRect().height || topHeaderHeight; const scrollMargin = -1 /* reduce margin-block so that intersection observer detects correct section*/ + @@ -295,10 +296,14 @@ const ObjectPage = forwardRef((props, ref const objectPageRect = objectPageElement.getBoundingClientRect(); // Calculate the top position of the section relative to the container - objectPageElement.scrollTop = - sectionRect.top - objectPageRect.top + objectPageElement.scrollTop - scrollMargin; + const targetScrollTop = sectionRect.top - objectPageRect.top + objectPageElement.scrollTop - scrollMargin; + objectPageElement.scrollTop = targetScrollTop; section.style.scrollMarginBlockStart = ''; + + // remember a target the browser clamped because the bottom spacer hasn't grown yet + pendingScrollTargetRef.current = + targetScrollTop > objectPageElement.scrollHeight - objectPageElement.clientHeight ? targetScrollTop : null; } }; // In TabBar mode the section is only rendered when selected: delay scroll for subsection @@ -311,6 +316,7 @@ const ObjectPage = forwardRef((props, ref [ mode, objectPageRef, + topHeaderRef, topHeaderHeight, tabContainerHeaderHeight, headerPinned, @@ -407,6 +413,20 @@ const ObjectPage = forwardRef((props, ref } }, [selectedSubSectionId, sectionSpacer, scrollToSectionById]); + // re-apply the clamped target once sectionSpacer has grown enough + useEffect(() => { + const target = pendingScrollTargetRef.current; + if (target == null) { + return; + } + pendingScrollTargetRef.current = null; + const objectPage = objectPageRef.current; + if (objectPage && objectPage.scrollHeight - objectPage.clientHeight + 1 >= target) { + objectPage.scrollTop = target; + } + // eslint-disable-next-line react-hooks/exhaustive-deps -- refs are stable; only sectionSpacer should re-trigger + }, [sectionSpacer]); + useEffect(() => { if (headerPinnedProp !== undefined) { setHeaderPinned(headerPinnedProp); diff --git a/packages/main/src/components/ObjectPage/test/ObjectPage.gallery.tsx b/packages/main/src/components/ObjectPage/test/ObjectPage.gallery.tsx new file mode 100644 index 00000000000..f06d2163685 --- /dev/null +++ b/packages/main/src/components/ObjectPage/test/ObjectPage.gallery.tsx @@ -0,0 +1,51 @@ +import { Link } from '../../../webComponents/Link/index.js'; +import { MessageStrip } from '../../../webComponents/MessageStrip/index.js'; +import { Title } from '../../../webComponents/Title/index.js'; +import { FlexBox } from '../../FlexBox/index.js'; +import { ObjectPageHeader } from '../../ObjectPageHeader/index.js'; +import { ObjectPageSection } from '../../ObjectPageSection/index.js'; +import { ObjectPageTitle } from '../../ObjectPageTitle/index.js'; +import { ObjectPage } from '../index.js'; + +// long expandable header + a last section shorter than the viewport (relies on the bottom spacer) +export const ObjectPageLongHeaderTestComp = () => { + return ( + Denise Smith} + snappedHeader={Denise Smith (snapped)} + subHeader="Senior UI Developer" + snappedSubHeader="Senior UI Developer (snapped)" + expandedContent={ + + {Array.from({ length: 18 }, () => 'Information (only visible if header content is expanded)').join(' ')} + + } + snappedContent={ + Information (only visible if header content is snapped) + } + /> + } + headerArea={ + + + +33 6 4512 5158 + DeniseSmith@sap.com + + + } + > + +
+ + +
+ + +
+ + + ); +}; diff --git a/packages/main/src/components/ObjectPage/test/ObjectPage.spec.tsx b/packages/main/src/components/ObjectPage/test/ObjectPage.spec.tsx new file mode 100644 index 00000000000..61252356844 --- /dev/null +++ b/packages/main/src/components/ObjectPage/test/ObjectPage.spec.tsx @@ -0,0 +1,49 @@ +import type { Page } from '@playwright/test'; +import { expect, test } from '../../../../../../playwright/fixtures/gallery-fixtures.js'; + +// Poll until scrollTop has moved and gone stable, so we assert the final selection and not the tab lock held mid-scroll. +async function waitForScrollSettled(page: Page) { + await page.evaluate(() => ((window as unknown as { __opScroll: number[] }).__opScroll = [])); + await page.waitForFunction( + () => { + const op = document.querySelector('[data-component-name="ObjectPage"]'); + if (!op) { + return false; + } + const hist = (window as unknown as { __opScroll: number[] }).__opScroll; + hist.push(op.scrollTop); + const moved = hist.some((value) => value > 0); + const count = hist.length; + return moved && count >= 2 && Math.abs(hist[count - 1] - hist[count - 2]) <= 1; + }, + undefined, + { polling: 100 }, + ); +} + +test.describe('ObjectPage', () => { + test('selects last section with long header', async ({ mount, page }) => { + await page.setViewportSize({ width: 950, height: 800 }); + await mount('ObjectPage/ObjectPageLongHeaderTestComp'); + + await page.getByRole('tab', { name: 'Employment' }).click(); + await waitForScrollSettled(page); + + const geo = await page.evaluate(() => { + const op = document.querySelector('[data-component-name="ObjectPage"]'); + const tabs = document.querySelector('[data-component-name="ObjectPageTabContainer"]'); + const opTop = op.getBoundingClientRect().top; + const rect = (id: string) => document.getElementById(id).getBoundingClientRect(); + return { + stickyBottom: tabs.getBoundingClientRect().bottom - opTop, + employmentTop: rect('ObjectPageSection-employment').top - opTop, + personalBottom: rect('ObjectPageSection-personal').bottom - opTop, + }; + }); + + // Employment scrolled to just under the sticky header, Personal scrolled above it (out of the selection zone) + expect(Math.abs(geo.employmentTop - geo.stickyBottom)).toBeLessThanOrEqual(4); + expect(geo.personalBottom).toBeLessThanOrEqual(geo.stickyBottom); + await expect(page.locator('[data-section-id="employment"]')).toHaveAttribute('selected'); + }); +});