From ad9937a6892864ec7cbc0fa84faa4845a38b948c Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Thu, 9 Jan 2025 09:46:24 +0100 Subject: [PATCH] fix(ObjectPage): scroll to section when programmatically selected (#6768) Fixes #6765 --- .../components/ObjectPage/ObjectPage.cy.tsx | 88 +++++++++++++++++-- .../components/ObjectPage/ObjectPageUtils.ts | 6 ++ .../main/src/components/ObjectPage/index.tsx | 71 +++++---------- 3 files changed, 109 insertions(+), 56 deletions(-) diff --git a/packages/main/src/components/ObjectPage/ObjectPage.cy.tsx b/packages/main/src/components/ObjectPage/ObjectPage.cy.tsx index 7f82a618cf3..5808340344a 100644 --- a/packages/main/src/components/ObjectPage/ObjectPage.cy.tsx +++ b/packages/main/src/components/ObjectPage/ObjectPage.cy.tsx @@ -1056,6 +1056,64 @@ describe('ObjectPage', () => { cy.findByText('First Content').should('be.visible'); }); + it('programmatic prop selection', () => { + const TestComp = (props: ObjectPagePropTypes) => { + const [selectedSection, setSelectedSection] = useState(props.selectedSectionId); + const [selectedSubSection, setSelectedSubSection] = useState(props.selectedSubSectionId); + + return ( + <> + + + + {OPContent} + + + ); + }; + + [{ titleArea: DPTitle, headerArea: DPContent }, { titleArea: DPTitle }, { headerArea: DPContent }, {}].forEach( + (props: ObjectPagePropTypes) => { + cy.mount(); + cy.findByText('employment-job-relationship-content').should('be.visible'); + cy.findByText('Job Information').should('not.be.visible'); + cy.get('[ui5-tabcontainer]').findUi5TabByText('Employment').should('have.attr', 'aria-selected', 'true'); + + cy.mount(); + cy.findByText('personal-connect-content').should('be.visible'); + cy.findByText('test-content').should('not.be.visible'); + cy.get('[ui5-tabcontainer]').findUi5TabByText('Personal').should('have.attr', 'aria-selected', 'true'); + + cy.findByText('Select Goals').click(); + cy.findByText('goals-content').should('be.visible'); + cy.findByText('personal-connect-content').should('not.be.visible'); + cy.get('[ui5-tabcontainer]').findUi5TabByText('Goals').should('have.attr', 'aria-selected', 'true'); + + cy.findByText('Select Payment Information').click(); + cy.findByText('personal-payment-information-content').should('be.visible'); + cy.findByText('personal-connect-content').should('not.be.visible'); + cy.get('[ui5-tabcontainer]').findUi5TabByText('Personal').should('have.attr', 'aria-selected', 'true'); + } + ); + }); + cypressPassThroughTestsFactory(ObjectPage); }); @@ -1073,7 +1131,7 @@ const DPTitle = ( Manager Cockpit My Team - Employee Details + Employee Details (Denise Smith) } > @@ -1101,10 +1159,14 @@ const DPContent = ( const OPContent = [ -
+
+ goals-content +
, -
+
+ test-content +
, } > -
+
+ personal-connect-content +
-
+
+ personal-payment-information-content +
, /?|♥`} aria-label="Employment"> -
+
+ employment-job-information-content +
-
+
+ employment-employee-details-content +
-
+
+ employment-job-relationship-content +
]; diff --git a/packages/main/src/components/ObjectPage/ObjectPageUtils.ts b/packages/main/src/components/ObjectPage/ObjectPageUtils.ts index 38d957fd722..71c91bec8e7 100644 --- a/packages/main/src/components/ObjectPage/ObjectPageUtils.ts +++ b/packages/main/src/components/ObjectPage/ObjectPageUtils.ts @@ -11,3 +11,9 @@ export const getSectionById = (sections, id) => { ); }); }; + +export const getSectionElementById = (objectPage: HTMLDivElement, isSubSection: boolean, id: string | undefined) => { + return objectPage?.querySelector( + `#${isSubSection ? 'ObjectPageSubSection' : 'ObjectPageSection'}-${CSS.escape(id)}` + ); +}; diff --git a/packages/main/src/components/ObjectPage/index.tsx b/packages/main/src/components/ObjectPage/index.tsx index 105aa5de383..b986ae167c5 100644 --- a/packages/main/src/components/ObjectPage/index.tsx +++ b/packages/main/src/components/ObjectPage/index.tsx @@ -31,7 +31,7 @@ import type { } from '../ObjectPageTitle/index.js'; import { CollapsedAvatar } from './CollapsedAvatar.js'; import { classNames, styleData } from './ObjectPage.module.css.js'; -import { getSectionById } from './ObjectPageUtils.js'; +import { getSectionById, getSectionElementById } from './ObjectPageUtils.js'; const ObjectPageCssVariables = { headerDisplay: '--_ui5wcr_ObjectPage_header_display', @@ -218,10 +218,10 @@ const ObjectPage = forwardRef((props, ref const [internalSelectedSectionId, setInternalSelectedSectionId] = useState( selectedSectionId ?? firstSectionId ); - const [selectedSubSectionId, setSelectedSubSectionId] = useState(props.selectedSubSectionId); + const [selectedSubSectionId, setSelectedSubSectionId] = useState(undefined); const [headerPinned, setHeaderPinned] = useState(headerPinnedProp); const isProgrammaticallyScrolled = useRef(false); - const prevSelectedSectionId = useRef(undefined); + const [isMounted, setIsMounted] = useState(false); const [componentRef, objectPageRef] = useSyncRef(ref); const topHeaderRef = useRef(null); @@ -326,9 +326,7 @@ const ObjectPage = forwardRef((props, ref }, [image, classNames.headerImage, classNames.image, imageShapeCircle]); const scrollToSectionById = (id: string | undefined, isSubSection = false) => { - const section = objectPageRef.current?.querySelector( - `#${isSubSection ? 'ObjectPageSubSection' : 'ObjectPageSection'}-${CSS.escape(id)}` - ); + const section = getSectionElementById(objectPageRef.current, isSubSection, id); scrollTimeout.current = performance.now() + 500; if (section) { const safeTopHeaderHeight = topHeaderHeight || prevTopHeaderHeight.current; @@ -367,45 +365,6 @@ const ObjectPage = forwardRef((props, ref isProgrammaticallyScrolled.current = false; }; - const programmaticallySetSection = () => { - const currentId = selectedSectionId ?? firstSectionId; - if (currentId !== prevSelectedSectionId.current) { - debouncedOnSectionChange.cancel(); - isProgrammaticallyScrolled.current = true; - setInternalSelectedSectionId(currentId); - prevSelectedSectionId.current = currentId; - const sectionNodes = objectPageRef.current?.querySelectorAll('section[data-component-name="ObjectPageSection"]'); - const currentIndex = childrenArray.findIndex((objectPageSection) => { - return isValidElement(objectPageSection) && objectPageSection.props?.id === currentId; - }); - fireOnSelectedChangedEvent({}, currentIndex, currentId, sectionNodes[0]); - } - }; - - // change selected section when prop is changed (external change) - const [timeStamp, setTimeStamp] = useState(0); - const requestAnimationFrameRef = useRef(undefined); - useEffect(() => { - if (selectedSectionId) { - if (mode === ObjectPageMode.Default) { - // wait for DOM draw, otherwise initial scroll won't work as intended - if (timeStamp < 750 && timeStamp !== undefined) { - requestAnimationFrameRef.current = requestAnimationFrame((internalTimestamp) => { - setTimeStamp(internalTimestamp); - }); - } else { - setTimeStamp(undefined); - programmaticallySetSection(); - } - } else { - programmaticallySetSection(); - } - } - return () => { - cancelAnimationFrame(requestAnimationFrameRef.current); - }; - }, [timeStamp, selectedSectionId, firstSectionId, debouncedOnSectionChange]); - // section was selected by clicking on the tab bar buttons const handleOnSectionSelected = (targetEvent, newSelectionSectionId, index, section) => { isProgrammaticallyScrolled.current = true; @@ -421,12 +380,24 @@ const ObjectPage = forwardRef((props, ref fireOnSelectedChangedEvent(targetEvent, index, newSelectionSectionId, section); }; + useEffect(() => { + if (selectedSectionId) { + const selectedSection = getSectionElementById(objectPageRef.current, false, selectedSectionId); + if (selectedSection) { + const selectedSectionIndex = Array.from( + selectedSection.parentElement.querySelectorAll(':scope > [data-component-name="ObjectPageSection"]') + ).indexOf(selectedSection); + handleOnSectionSelected({}, selectedSectionId, selectedSectionIndex, selectedSection); + } + } + }, [selectedSectionId]); + // do internal scrolling useEffect(() => { if (mode === ObjectPageMode.Default && isProgrammaticallyScrolled.current === true && !selectedSubSectionId) { scrollToSection(internalSelectedSectionId); } - }, [internalSelectedSectionId, mode, isProgrammaticallyScrolled, scrollToSection, selectedSubSectionId]); + }, [internalSelectedSectionId, mode, selectedSubSectionId]); // Scrolling for Sub Section Selection useEffect(() => { @@ -457,11 +428,15 @@ const ObjectPage = forwardRef((props, ref }, [headerPinned, topHeaderHeight]); useEffect(() => { + if (!isMounted) { + setIsMounted(true); + return; + } setSelectedSubSectionId(props.selectedSubSectionId); if (props.selectedSubSectionId) { isProgrammaticallyScrolled.current = true; if (mode === ObjectPageMode.IconTabBar) { - let sectionId; + let sectionId: string; childrenArray.forEach((section) => { if (isValidElement(section) && section.props && section.props.children) { safeGetChildrenArray(section.props.children).forEach((subSection) => { @@ -480,7 +455,7 @@ const ObjectPage = forwardRef((props, ref } } } - }, [props.selectedSubSectionId, childrenArray, mode]); + }, [props.selectedSubSectionId, isMounted]); const tabContainerContainerRef = useRef(null); useEffect(() => {