fix(ObjectPage): keep clicked tab selected with a tall expandable header (#8915)

Fixes #8906

Author: Lukas742

packages/main/src/components/ObjectPage/index.tsx

@@ -102,7 +102,7 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
   const isProgrammaticallyScrolled = useRef(false);
   const [componentRef, objectPageRef] = useSyncRef(ref);
   const topHeaderRef = useRef<HTMLDivElement>(null);
-  const prevTopHeaderHeight = useRef(0);
+  const pendingScrollTargetRef = useRef<number | null>(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<ObjectPageDomRef, ObjectPagePropTypes>((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<ObjectPageDomRef, ObjectPagePropTypes>((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<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
     [
       mode,
       objectPageRef,
+      topHeaderRef,
       topHeaderHeight,
       tabContainerHeaderHeight,
       headerPinned,
@@ -407,6 +413,20 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((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);

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 (
+    <ObjectPage
+      style={{ height: '100vh' }}
+      titleArea={
+        <ObjectPageTitle
+          header={<Title>Denise Smith</Title>}
+          snappedHeader={<Title>Denise Smith (snapped)</Title>}
+          subHeader="Senior UI Developer"
+          snappedSubHeader="Senior UI Developer (snapped)"
+          expandedContent={
+            <MessageStrip hideCloseButton>
+              {Array.from({ length: 18 }, () => 'Information (only visible if header content is expanded)').join(' ')}
+            </MessageStrip>
+          }
+          snappedContent={
+            <MessageStrip hideCloseButton>Information (only visible if header content is snapped)</MessageStrip>
+          }
+        />
+      }
+      headerArea={
+        <ObjectPageHeader>
+          <FlexBox direction="Column">
+            <Link>+33 6 4512 5158</Link>
+            <Link href="mailto:ui5-webcomponents-react@sap.com">DeniseSmith@sap.com</Link>
+          </FlexBox>
+        </ObjectPageHeader>
+      }
+    >
+      <ObjectPageSection titleText="Goals" id="goals" aria-label="Goals">
+        <div style={{ height: '120px', width: '100%', background: 'lightblue' }} />
+      </ObjectPageSection>
+      <ObjectPageSection titleText="Personal" id="personal" aria-label="Personal">
+        <div style={{ height: '400px', width: '100%', background: 'lightyellow' }} />
+      </ObjectPageSection>
+      <ObjectPageSection titleText="Employment" id="employment" aria-label="Employment">
+        <div style={{ height: '120px', width: '100%', background: 'orange' }} />
+      </ObjectPageSection>
+    </ObjectPage>
+  );
+};

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');
+  });
+});