fix(ObjectPage): remove top-spacer whitespace on IconTabBar tab switch

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

@@ -44,6 +44,7 @@ import type {
 } from './types/index.js';
 import { useHandleTabSelect } from './useHandleTabSelect.js';
 import { useOnScrollEnd } from './useOnScrollEnd.js';
+import { useSectionSpacer } from './useSectionSpacer.js';
 
 const ObjectPageCssVariables = {
   headerDisplay: '--_ui5wcr_ObjectPage_header_display',
@@ -112,6 +113,8 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
   const scrollTimeout = useRef(0);
   // Set on manual collapse (toggle button/title): header stays collapsed without a scroll spacer and re-expands only at scrollTop 0.
   const manuallyCollapsedRef = useRef(false);
+  // Set on an IconTabBar tab switch while collapsed: the new section is measured a commit later, so the collapse-scroll is deferred until then.
+  const pendingCollapsedTabScrollRef = useRef(false);
   const prevInternalSelectedSectionId = useRef(internalSelectedSectionId);
 
   const [selectedSubSectionId, setSelectedSubSectionId] = useState<undefined | string>(undefined);
@@ -120,6 +123,7 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
   const [headerCollapsedInternal, setHeaderCollapsedInternal] = useState<undefined | boolean>(undefined);
   const [scrolledHeaderExpanded, setScrolledHeaderExpanded] = useState(false);
   const [sectionSpacer, setSectionSpacer] = useState(0);
+  const [isActiveTabSectionTallEnough, setIsActiveTabSectionTallEnough] = useState(false);
   const currentTabModeSection = useMemo(
     () => (mode === ObjectPageMode.IconTabBar ? getSectionById(children, internalSelectedSectionId) : null),
     [mode, children, internalSelectedSectionId],
@@ -358,10 +362,13 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
       // Section swap returns to the default collapsed behavior (spacer present, scroll-up re-expands).
       manuallyCollapsedRef.current = false;
       setToggledCollapsedHeaderWasVisible(false);
-      // When collapsed, land 1px past the expand threshold so the header stays collapsed but scroll-up can re-expand it.
-      objectPageRef.current?.scrollTo({
-        top: headerCollapsed && !headerPinned ? Math.max(headerContentHeight, topHeaderHeight) + 1 : 0,
-      });
+      // The new section is measured a commit later, so defer the collapsed scroll (see the effect keyed on
+      // isActiveTabSectionTallEnough); scrolling here would clamp to 0 and leave the spacer as dead space.
+      setIsActiveTabSectionTallEnough(false);
+      pendingCollapsedTabScrollRef.current = headerCollapsed && !headerPinned;
+      if (!headerCollapsed || headerPinned) {
+        objectPageRef.current?.scrollTo({ top: 0 });
+      }
     }
     setTabSelectId(newSelectionSectionId);
     scrollEvent.current = targetEvent;
@@ -405,6 +412,25 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
     }
   }, [internalSelectedSectionId, mode, selectedSubSectionId, scrollToSection]);
 
+  // Apply the collapse-scroll deferred by a tab switch, once calculateSpacer has measured the new section
+  // (isActiveTabSectionTallEnough): the spacer is rendered and the container scrollable, so the collapsed header lands
+  // just past the spacer (no gap, scroll-up still re-expands). Too-short sections never flip the flag and keep no spacer.
+  useIsomorphicLayoutEffect(() => {
+    if (!pendingCollapsedTabScrollRef.current || !isActiveTabSectionTallEnough) {
+      return;
+    }
+    if (
+      mode === ObjectPageMode.IconTabBar &&
+      !isActiveSectionFitContent &&
+      headerCollapsed &&
+      !headerPinned &&
+      !manuallyCollapsedRef.current
+    ) {
+      objectPageRef.current?.scrollTo({ top: Math.max(headerContentHeight, topHeaderHeight) + 1 });
+    }
+    pendingCollapsedTabScrollRef.current = false;
+  }, [isActiveTabSectionTallEnough]);
+
   // Scrolling for Sub Section Selection
   useEffect(() => {
     if (selectedSubSectionId && isProgrammaticallyScrolled.current === true) {
@@ -489,76 +515,22 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
   }, [props.selectedSubSectionId, isMounted, childrenArray, debouncedOnSectionChange, mode]);
 
   const tabContainerContainerRef = useRef(null);
-  const isHeaderPinnedAndExpanded = headerPinned && !headerCollapsed;
-  useEffect(() => {
-    const objectPage = objectPageRef.current;
-    const tabContainerContainer = tabContainerContainerRef.current;
-
-    if (!objectPage || !tabContainerContainer) {
-      return;
-    }
-
-    const footerElement = objectPage.querySelector<HTMLDivElement>('[data-component-name="ObjectPageFooter"]');
-    const topHeaderElement = objectPage.querySelector('[data-component-name="ObjectPageTopHeader"]');
-
-    const calculateSpacer = ([lastSectionNodeEntry]: ResizeObserverEntry[]) => {
-      const lastSectionNode = lastSectionNodeEntry?.target;
-
-      if (!lastSectionNode) {
-        setSectionSpacer(0);
-        return;
-      }
-
-      const subSections = lastSectionNode.querySelectorAll<HTMLDivElement>('[id^="ObjectPageSubSection"]');
-      const lastSubSection = subSections[subSections.length - 1];
-      const lastSubSectionOrSection = lastSubSection ?? lastSectionNode;
-
-      if ((currentTabModeSection && !lastSubSection) || (sectionNodes.length === 1 && !lastSubSection)) {
-        setSectionSpacer(0);
-        return;
-      }
-
-      // batching DOM-reads together minimizes reflow
-      const footerHeight = footerElement?.offsetHeight ?? 0;
-      const objectPageRect = objectPage.getBoundingClientRect();
-      const tabContainerContainerRect = tabContainerContainer.getBoundingClientRect();
-      const lastSubSectionOrSectionRect = lastSubSectionOrSection.getBoundingClientRect();
-
-      let stickyHeaderBottom = 0;
-      if (!isHeaderPinnedAndExpanded) {
-        const topHeaderBottom = topHeaderElement?.getBoundingClientRect().bottom ?? 0;
-        stickyHeaderBottom = topHeaderBottom + tabContainerContainerRect.height;
-      } else {
-        stickyHeaderBottom = tabContainerContainerRect.bottom;
-      }
-
-      const spacer = Math.ceil(
-        objectPageRect.bottom - stickyHeaderBottom - lastSubSectionOrSectionRect.height - footerHeight, // section padding (8px) not included, so that the intersection observer is triggered correctly
-      );
-      setSectionSpacer(Math.max(spacer, 0));
-    };
-
-    const observer = new ResizeObserver(calculateSpacer);
-    const sectionNodes = objectPage.querySelectorAll<HTMLDivElement>('[id^="ObjectPageSection"]');
-    const lastSectionNode = sectionNodes[sectionNodes.length - 1];
-
-    if (lastSectionNode) {
-      observer.observe(lastSectionNode, { box: 'border-box' });
-    }
-
-    return () => {
-      observer.disconnect();
-    };
-  }, [
+  useSectionSpacer({
+    objectPageRef,
+    tabContainerContainerRef,
+    mode,
+    isActiveSectionFitContent,
     topHeaderHeight,
     headerContentHeight,
+    tabContainerHeaderHeight,
+    headerPinned,
+    headerCollapsed,
     currentTabModeSection,
     children,
-    mode,
-    isHeaderPinnedAndExpanded,
     hasOnlySingleSection,
-    objectPageRef,
-  ]);
+    setSectionSpacer,
+    setIsActiveTabSectionTallEnough,
+  });
 
   const { onScroll: _0, selectedSubSectionId: _1, ...propsWithoutOmitted } = rest;
 
@@ -955,7 +927,10 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
           <div
             style={{
               height:
-                ((headerCollapsed && !headerPinned) || scrolledHeaderExpanded) && !toggledCollapsedHeaderWasVisible
+                !isActiveSectionFitContent &&
+                ((headerCollapsed && !headerPinned) || scrolledHeaderExpanded) &&
+                !toggledCollapsedHeaderWasVisible &&
+                (mode !== ObjectPageMode.IconTabBar || isActiveTabSectionTallEnough)
                   ? `${headerContentHeight}px`
                   : 0,
             }}

packages/main/src/components/ObjectPage/test/ObjectPage.gallery.tsx

@@ -1,3 +1,4 @@
+import { ObjectPageMode } from '../../../enums/ObjectPageMode.js';
 import { Link } from '../../../webComponents/Link/index.js';
 import { MessageStrip } from '../../../webComponents/MessageStrip/index.js';
 import { Title } from '../../../webComponents/Title/index.js';
@@ -49,3 +50,67 @@ export const ObjectPageLongHeaderTestComp = () => {
     </ObjectPage>
   );
 };
+
+// IconTabBar + fitContent sections: switching tabs while the header is collapsed must not
+// reintroduce the top header spacer, which would otherwise leave a gap above the content.
+export const ObjectPageFitContentTabSwitchTestComp = () => {
+  return (
+    <ObjectPage
+      mode={ObjectPageMode.IconTabBar}
+      style={{ height: '600px' }}
+      titleArea={
+        <ObjectPageTitle
+          header={<Title>Denise Smith</Title>}
+          snappedHeader={<Title>Denise Smith (snapped)</Title>}
+          subHeader="Senior UI Developer"
+        />
+      }
+      headerArea={
+        <ObjectPageHeader>
+          <div style={{ height: '200px', width: '100%', background: 'lightyellow' }}>Header content</div>
+        </ObjectPageHeader>
+      }
+    >
+      <ObjectPageSection titleText="Fit 1" id="fit1" aria-label="Fit 1" fitContent>
+        <div style={{ flex: 1, minHeight: 0, background: 'lightblue' }}>Fit section 1</div>
+      </ObjectPageSection>
+      <ObjectPageSection titleText="Fit 2" id="fit2" aria-label="Fit 2" fitContent>
+        <div style={{ flex: 1, minHeight: 0, background: 'lightgreen' }}>Fit section 2</div>
+      </ObjectPageSection>
+      <ObjectPageSection titleText="Normal" id="normal" aria-label="Normal">
+        <div style={{ height: '2000px', width: '100%', background: 'lightsalmon' }}>Normal section</div>
+      </ObjectPageSection>
+    </ObjectPage>
+  );
+};
+
+// IconTabBar + non-fitContent sections: after collapsing the header and switching to a section shorter than the
+// viewport, the top header spacer must not remain as dead space above the section (it can't be scrolled away).
+// A tall section keeps the spacer so scroll-up can re-expand the collapsed header.
+export const ObjectPageShortSectionTabSwitchTestComp = () => {
+  return (
+    <ObjectPage
+      mode={ObjectPageMode.IconTabBar}
+      style={{ height: '600px' }}
+      titleArea={
+        <ObjectPageTitle
+          header={<Title>Denise Smith</Title>}
+          snappedHeader={<Title>Denise Smith (snapped)</Title>}
+          subHeader="Senior UI Developer"
+        />
+      }
+      headerArea={
+        <ObjectPageHeader>
+          <div style={{ height: '200px', width: '100%', background: 'lightyellow' }}>Header content</div>
+        </ObjectPageHeader>
+      }
+    >
+      <ObjectPageSection titleText="Short" id="short" aria-label="Short">
+        <div style={{ height: '40px', width: '100%', background: 'lightblue' }}>Short section</div>
+      </ObjectPageSection>
+      <ObjectPageSection titleText="Tall" id="tall" aria-label="Tall">
+        <div style={{ height: '2000px', width: '100%', background: 'lightsalmon' }}>Tall section</div>
+      </ObjectPageSection>
+    </ObjectPage>
+  );
+};

packages/main/src/components/ObjectPage/test/ObjectPage.spec.tsx

@@ -46,4 +46,84 @@ test.describe('ObjectPage', () => {
     expect(geo.personalBottom).toBeLessThanOrEqual(geo.stickyBottom);
     await expect(page.locator('[data-section-id="employment"]')).toHaveAttribute('selected');
   });
+
+  test('fitContent: switching tabs while the header is collapsed leaves no gap above the content', async ({
+    mount,
+    page,
+  }) => {
+    await page.setViewportSize({ width: 1440, height: 900 });
+    await mount('ObjectPage/ObjectPageFitContentTabSwitchTestComp');
+
+    // height of the top spacer that pads the content down to make room for a re-expandable collapsed header
+    const topSpacerHeight = () =>
+      page.evaluate(() => {
+        const spacer = document.querySelector('[data-component-name="ObjectPageContent"]')
+          ?.firstElementChild as HTMLElement | null;
+        return spacer ? Math.round(spacer.getBoundingClientRect().height) : -1;
+      });
+
+    // collapse the header via the toggle button
+    await page.locator('[data-component-name="ObjectPageAnchorBarExpandBtn"]').click();
+    await expect(page.getByText('Header content')).toBeHidden();
+
+    // switching to another fitContent section must not reintroduce the top spacer (the gap)
+    await page.getByRole('tab', { name: 'Fit 2' }).click();
+    await expect(page.getByText('Header content')).toBeHidden();
+    await expect.poll(topSpacerHeight).toBe(0);
+
+    // a non-fitContent section keeps the spacer so scrolling up can re-expand the collapsed header
+    await page.getByRole('tab', { name: 'Normal' }).click();
+    await expect.poll(topSpacerHeight).toBeGreaterThan(0);
+  });
+
+  test('non-fitContent: collapsed header + switch to a short section leaves no dead spacer above it', async ({
+    mount,
+    page,
+  }) => {
+    await page.setViewportSize({ width: 1440, height: 900 });
+    await mount('ObjectPage/ObjectPageShortSectionTabSwitchTestComp');
+
+    // height of the top spacer that pads the content down to make room for a re-expandable collapsed header
+    const topSpacerHeight = () =>
+      page.evaluate(() => {
+        const spacer = document.querySelector('[data-component-name="ObjectPageContent"]')
+          ?.firstElementChild as HTMLElement | null;
+        return spacer ? Math.round(spacer.getBoundingClientRect().height) : -1;
+      });
+
+    // how far the top spacer is scrolled out of view (>= its height means no gap is visible above the content)
+    const spacerScrolledOutBy = () =>
+      page.evaluate(() => {
+        const op = document.querySelector('[data-component-name="ObjectPage"]');
+        const spacer = document.querySelector('[data-component-name="ObjectPageContent"]')
+          ?.firstElementChild as HTMLElement | null;
+        if (!op || !spacer) {
+          return -1;
+        }
+        return Math.round(op.scrollTop) - Math.round(spacer.getBoundingClientRect().height);
+      });
+
+    // collapse the header via the toggle button
+    await page.locator('[data-component-name="ObjectPageAnchorBarExpandBtn"]').click();
+    await expect(page.getByText('Header content')).toBeHidden();
+
+    // a section too short to scroll must not keep the top spacer (it would be un-scrollable dead space)
+    await page.getByRole('tab', { name: 'Short' }).click();
+    await expect(page.getByText('Header content')).toBeHidden();
+    await expect.poll(topSpacerHeight).toBe(0);
+
+    // a tall section keeps the spacer (so scroll-up can re-expand the header) but lands scrolled past it, leaving no gap
+    await page.getByRole('tab', { name: 'Tall' }).click();
+    await expect(page.getByText('Header content')).toBeHidden();
+    await expect.poll(topSpacerHeight).toBeGreaterThan(0);
+    await expect.poll(spacerScrolledOutBy).toBeGreaterThanOrEqual(0);
+
+    // scrolling back up re-expands the collapsed header
+    await page.locator('[data-component-name="ObjectPage"]').evaluate((op) => op.scrollTo({ top: 0 }));
+    await expect(page.getByText('Header content')).toBeVisible();
+
+    // switching back to the short section drops the spacer again
+    await page.getByRole('tab', { name: 'Short' }).click();
+    await expect.poll(topSpacerHeight).toBe(0);
+  });
 });