From 38d2cd1cae124c8d695bd650f1227ac1146a7c78 Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Mon, 21 Sep 2026 15:00:00 +0200 Subject: [PATCH 1/3] fix(ObjectPage): remove top-spacer whitespace on IconTabBar tab switch --- .../main/src/components/ObjectPage/index.tsx | 50 ++++++++++-- .../ObjectPage/test/ObjectPage.gallery.tsx | 65 +++++++++++++++ .../ObjectPage/test/ObjectPage.spec.tsx | 80 +++++++++++++++++++ 3 files changed, 190 insertions(+), 5 deletions(-) diff --git a/packages/main/src/components/ObjectPage/index.tsx b/packages/main/src/components/ObjectPage/index.tsx index 982ffecd8d9..6a77eff9b36 100644 --- a/packages/main/src/components/ObjectPage/index.tsx +++ b/packages/main/src/components/ObjectPage/index.tsx @@ -112,6 +112,8 @@ const ObjectPage = forwardRef((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); @@ -120,6 +122,7 @@ const ObjectPage = forwardRef((props, ref const [headerCollapsedInternal, setHeaderCollapsedInternal] = useState(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 +361,13 @@ const ObjectPage = forwardRef((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 +411,25 @@ const ObjectPage = forwardRef((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) { @@ -513,6 +538,16 @@ const ObjectPage = forwardRef((props, ref const lastSubSection = subSections[subSections.length - 1]; const lastSubSectionOrSection = lastSubSection ?? lastSectionNode; + // Only keep the top spacer when the non-fit section is tall enough to scroll it out of view; a shorter section + // can't scroll, so the reserved headerContentHeight would stay on screen as dead space. + if (mode === ObjectPageMode.IconTabBar && !isActiveSectionFitContent) { + const footerHeight = footerElement?.offsetHeight ?? 0; + const availableViewport = + objectPage.getBoundingClientRect().height - topHeaderHeight - tabContainerHeaderHeight - footerHeight; + const sectionHeight = (lastSectionNode as HTMLElement).getBoundingClientRect().height; + setIsActiveTabSectionTallEnough(sectionHeight >= availableViewport); + } + if ((currentTabModeSection && !lastSubSection) || (sectionNodes.length === 1 && !lastSubSection)) { setSectionSpacer(0); return; @@ -558,6 +593,8 @@ const ObjectPage = forwardRef((props, ref isHeaderPinnedAndExpanded, hasOnlySingleSection, objectPageRef, + isActiveSectionFitContent, + tabContainerHeaderHeight, ]); const { onScroll: _0, selectedSubSectionId: _1, ...propsWithoutOmitted } = rest; @@ -955,7 +992,10 @@ const ObjectPage = forwardRef((props, ref
{ ); }; + +// 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 ( + Denise Smith} + snappedHeader={Denise Smith (snapped)} + subHeader="Senior UI Developer" + /> + } + headerArea={ + +
Header content
+ + } + > + +
Fit section 1
+ + +
Fit section 2
+ + +
Normal section
+ + + ); +}; + +// 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 ( + Denise Smith} + snappedHeader={Denise Smith (snapped)} + subHeader="Senior UI Developer" + /> + } + headerArea={ + +
Header content
+ + } + > + +
Short section
+ + +
Tall section
+ + + ); +}; diff --git a/packages/main/src/components/ObjectPage/test/ObjectPage.spec.tsx b/packages/main/src/components/ObjectPage/test/ObjectPage.spec.tsx index 61252356844..00f76fdede6 100644 --- a/packages/main/src/components/ObjectPage/test/ObjectPage.spec.tsx +++ b/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); + }); }); From f54a456ced83325d14c42df0d452b67c91adcd3e Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Mon, 21 Sep 2026 16:05:13 +0200 Subject: [PATCH 2/3] extract section-spacer measurement into useSectionSpacer --- .../main/src/components/ObjectPage/index.tsx | 89 ++---------- .../components/ObjectPage/useSectionSpacer.ts | 130 ++++++++++++++++++ 2 files changed, 142 insertions(+), 77 deletions(-) create mode 100644 packages/main/src/components/ObjectPage/useSectionSpacer.ts diff --git a/packages/main/src/components/ObjectPage/index.tsx b/packages/main/src/components/ObjectPage/index.tsx index 6a77eff9b36..3b01850cc0b 100644 --- a/packages/main/src/components/ObjectPage/index.tsx +++ b/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', @@ -514,88 +515,22 @@ const ObjectPage = forwardRef((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('[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('[id^="ObjectPageSubSection"]'); - const lastSubSection = subSections[subSections.length - 1]; - const lastSubSectionOrSection = lastSubSection ?? lastSectionNode; - - // Only keep the top spacer when the non-fit section is tall enough to scroll it out of view; a shorter section - // can't scroll, so the reserved headerContentHeight would stay on screen as dead space. - if (mode === ObjectPageMode.IconTabBar && !isActiveSectionFitContent) { - const footerHeight = footerElement?.offsetHeight ?? 0; - const availableViewport = - objectPage.getBoundingClientRect().height - topHeaderHeight - tabContainerHeaderHeight - footerHeight; - const sectionHeight = (lastSectionNode as HTMLElement).getBoundingClientRect().height; - setIsActiveTabSectionTallEnough(sectionHeight >= availableViewport); - } - - 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('[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, - isActiveSectionFitContent, - tabContainerHeaderHeight, - ]); + setSectionSpacer, + setIsActiveTabSectionTallEnough, + }); const { onScroll: _0, selectedSubSectionId: _1, ...propsWithoutOmitted } = rest; diff --git a/packages/main/src/components/ObjectPage/useSectionSpacer.ts b/packages/main/src/components/ObjectPage/useSectionSpacer.ts new file mode 100644 index 00000000000..d0d30d083cc --- /dev/null +++ b/packages/main/src/components/ObjectPage/useSectionSpacer.ts @@ -0,0 +1,130 @@ +import type { Dispatch, ReactElement, ReactNode, RefObject, SetStateAction } from 'react'; +import { useEffect } from 'react'; +import { ObjectPageMode } from '../../enums/ObjectPageMode.js'; +import type { ObjectPageDomRef, ObjectPagePropTypes } from './types/index.js'; + +interface UseSectionSpacerProps { + objectPageRef: RefObject; + tabContainerContainerRef: RefObject; + mode: ObjectPagePropTypes['mode']; + isActiveSectionFitContent: boolean; + topHeaderHeight: number; + headerContentHeight: number; + tabContainerHeaderHeight: number; + headerPinned: boolean; + headerCollapsed: boolean; + currentTabModeSection: ReactElement | null; + children: ReactNode; + hasOnlySingleSection: boolean; + setSectionSpacer: Dispatch>; + setIsActiveTabSectionTallEnough: Dispatch>; +} + +/** + * Observes the last section and, from a single measurement pass, derives both spacer values the content needs: + * + * - `sectionSpacer`: bottom padding so the last section can scroll up under the sticky header. + * - `isActiveTabSectionTallEnough`: whether a non-fit `IconTabBar` section is tall enough to scroll the top spacer out of view (a shorter one can't, so its reserved `headerContentHeight` would stay on screen as dead space). + */ +export const useSectionSpacer = ({ + objectPageRef, + tabContainerContainerRef, + mode, + isActiveSectionFitContent, + topHeaderHeight, + headerContentHeight, + tabContainerHeaderHeight, + headerPinned, + headerCollapsed, + currentTabModeSection, + children, + hasOnlySingleSection, + setSectionSpacer, + setIsActiveTabSectionTallEnough, +}: UseSectionSpacerProps) => { + const isHeaderPinnedAndExpanded = headerPinned && !headerCollapsed; + useEffect(() => { + const objectPage = objectPageRef.current; + const tabContainerContainer = tabContainerContainerRef.current; + + if (!objectPage || !tabContainerContainer) { + return; + } + + const footerElement = objectPage.querySelector('[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('[id^="ObjectPageSubSection"]'); + const lastSubSection = subSections[subSections.length - 1]; + const lastSubSectionOrSection = lastSubSection ?? lastSectionNode; + + // Only keep the top spacer when the non-fit section is tall enough to scroll it out of view; a shorter section + // can't scroll, so the reserved headerContentHeight would stay on screen as dead space. + if (mode === ObjectPageMode.IconTabBar && !isActiveSectionFitContent) { + const footerHeight = footerElement?.offsetHeight ?? 0; + const availableViewport = + objectPage.getBoundingClientRect().height - topHeaderHeight - tabContainerHeaderHeight - footerHeight; + const sectionHeight = (lastSectionNode as HTMLElement).getBoundingClientRect().height; + setIsActiveTabSectionTallEnough(sectionHeight >= availableViewport); + } + + 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('[id^="ObjectPageSection"]'); + const lastSectionNode = sectionNodes[sectionNodes.length - 1]; + + if (lastSectionNode) { + observer.observe(lastSectionNode, { box: 'border-box' }); + } + + return () => { + observer.disconnect(); + }; + }, [ + topHeaderHeight, + headerContentHeight, + currentTabModeSection, + children, + mode, + isHeaderPinnedAndExpanded, + hasOnlySingleSection, + objectPageRef, + isActiveSectionFitContent, + tabContainerHeaderHeight, + tabContainerContainerRef, + setSectionSpacer, + setIsActiveTabSectionTallEnough, + ]); +}; From 3c90c98aa8d432d001fad8c0a19437f3fd9475d0 Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Tue, 22 Sep 2026 13:01:08 +0200 Subject: [PATCH 3/3] Update useSectionSpacer.ts --- packages/main/src/components/ObjectPage/useSectionSpacer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/main/src/components/ObjectPage/useSectionSpacer.ts b/packages/main/src/components/ObjectPage/useSectionSpacer.ts index d0d30d083cc..d3a76b4362a 100644 --- a/packages/main/src/components/ObjectPage/useSectionSpacer.ts +++ b/packages/main/src/components/ObjectPage/useSectionSpacer.ts @@ -1,4 +1,4 @@ -import type { Dispatch, ReactElement, ReactNode, RefObject, SetStateAction } from 'react'; +import type { Dispatch, ReactNode, RefObject, SetStateAction } from 'react'; import { useEffect } from 'react'; import { ObjectPageMode } from '../../enums/ObjectPageMode.js'; import type { ObjectPageDomRef, ObjectPagePropTypes } from './types/index.js'; @@ -13,7 +13,7 @@ interface UseSectionSpacerProps { tabContainerHeaderHeight: number; headerPinned: boolean; headerCollapsed: boolean; - currentTabModeSection: ReactElement | null; + currentTabModeSection: ReactNode; children: ReactNode; hasOnlySingleSection: boolean; setSectionSpacer: Dispatch>;