fix(ObjectPage): fire onSelectedSectionChange correctly in IconTabBar mode

Author: Lukas742

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

@@ -1284,31 +1284,75 @@ describe('ObjectPage', () => {
           </>
         );
       };
-
+      const changeSpy = cy.spy().as('change');
+      let callCount = 1;
       [{ titleArea: DPTitle, headerArea: DPContent }, { titleArea: DPTitle }, { headerArea: DPContent }, {}].forEach(
         (props: ObjectPagePropTypes) => {
-          cy.mount(<TestComp {...props} mode={mode} selectedSubSectionId={`employment-job-relationship`} />);
+          cy.mount(
+            <TestComp
+              {...props}
+              mode={mode}
+              selectedSubSectionId={`employment-job-relationship`}
+              onSelectedSectionChange={changeSpy}
+            />
+          );
 
           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(<TestComp {...props} selectedSectionId={`personal`} />);
+          cy.mount(
+            <TestComp {...props} mode={mode} selectedSectionId={`personal`} onSelectedSectionChange={changeSpy} />
+          );
           cy.findByText('personal-connect-content').should('be.visible');
-          cy.findByText('test-content').should('not.be.visible');
+
+          if (mode === 'IconTabBar') {
+            cy.findByText('test-content').should('not.exist');
+          } else {
+            cy.findByText('test-content').should('not.be.visible');
+          }
           cy.get('[ui5-tabcontainer]').findUi5TabByText('Personal').should('have.attr', 'aria-selected', 'true');
 
           cy.wait(100);
           cy.findByText('Select Goals').click();
           cy.findByText('goals-content').should('be.visible');
+          cy.get('@change').should('have.callCount', callCount);
+          callCount++;
+          if (mode === 'IconTabBar') {
+            cy.findByText('personal-connect-content').should('not.exist');
+          } else {
+            cy.findByText('personal-connect-content').should('not.be.visible');
+          }
+          cy.get('[ui5-tabcontainer]').findUi5TabByText('Goals').should('have.attr', 'aria-selected', 'true');
 
-          cy.findByText('personal-connect-content').should('not.be.visible');
+          cy.get('[ui5-tabcontainer]').findUi5TabByText('Personal').click();
+          if (mode === 'IconTabBar') {
+            cy.findByText('test-content').should('not.exist');
+          } else {
+            cy.findByText('test-content').should('not.be.visible');
+          }
+          cy.findByText('personal-connect-content').should('be.visible');
+          cy.get('[ui5-tabcontainer]').findUi5TabByText('Personal').should('have.attr', 'aria-selected', 'true');
+          cy.get('@change').should('have.callCount', callCount);
+          callCount++;
+
+          cy.get('[ui5-tabcontainer]').findUi5TabByText('Goals').click();
+          if (mode === 'IconTabBar') {
+            cy.findByText('personal-connect-content').should('not.exist');
+          } else {
+            cy.findByText('personal-connect-content').should('not.be.visible');
+          }
+          cy.findByText('goals-content').should('be.visible');
           cy.get('[ui5-tabcontainer]').findUi5TabByText('Goals').should('have.attr', 'aria-selected', 'true');
+          cy.get('@change').should('have.callCount', callCount);
+          callCount++;
 
           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');
+          cy.get('@change').should('have.callCount', callCount);
+          callCount++;
         }
       );
     });

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

@@ -5,6 +5,7 @@ import {
   debounce,
   enrichEventWithDetails,
   ThemingParameters,
+  useIsomorphicLayoutEffect,
   useStylesheet,
   useSyncRef
 } from '@ui5/webcomponents-react-base';
@@ -108,6 +109,7 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
   const prevInternalSelectedSectionId = useRef(internalSelectedSectionId);
   const fireOnSelectedChangedEvent = (targetEvent, index, id, section) => {
     if (typeof onSelectedSectionChange === 'function' && targetEvent && prevInternalSelectedSectionId.current !== id) {
+      console.log('FIRE!!!');
       onSelectedSectionChange(
         enrichEventWithDetails(targetEvent, {
           selectedSectionIndex: parseInt(index, 10),
@@ -249,18 +251,23 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
     fireOnSelectedChangedEvent(targetEvent, index, newSelectionSectionId, section);
   };
 
-  useEffect(() => {
+  useIsomorphicLayoutEffect(() => {
     if (selectedSectionId) {
+      const fireSelectEvent = () => {
+        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);
+        }
+      };
       if (mode === ObjectPageMode.IconTabBar) {
         setInternalSelectedSectionId(selectedSectionId);
-        return;
-      }
-      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);
+        // In TabBar mode the section is only rendered when selected, therefore delay firing the event until the section is available in the DOM
+        setTimeout(fireSelectEvent);
+      } else {
+        fireSelectEvent();
       }
     }
   }, [selectedSectionId, mode]);
@@ -300,6 +307,7 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
     }
   }, [headerPinned, topHeaderHeight]);
 
+  const isInitialTabBarMode = useRef(true);
   useEffect(() => {
     if (!isMounted) {
       requestAnimationFrame(() => setIsMounted(true));
@@ -311,24 +319,33 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
       isProgrammaticallyScrolled.current = true;
       if (mode === ObjectPageMode.IconTabBar) {
         let sectionId: string;
-        childrenArray.forEach((section) => {
+        let curSection: ReactElement;
+        let sectionIndex: number = -1;
+        childrenArray.forEach((section, index) => {
           if (isValidElement(section) && section.props && section.props.children) {
             safeGetChildrenArray(section.props.children).forEach((subSection) => {
               if (
                 isValidElement(subSection) &&
                 subSection.props &&
                 (subSection as ReactElement<ObjectPageSubSectionPropTypes>).props.id === props.selectedSubSectionId
               ) {
+                curSection = section;
                 sectionId = section.props?.id;
+                sectionIndex = index;
               }
             });
           }
         });
         if (sectionId) {
+          if (!isInitialTabBarMode.current) {
+            //In TabBar mode the section is often not scrolled when subsection changes, thus the onSelectedSectionChange isn't fired
+            debouncedOnSectionChange({}, sectionIndex, sectionId, curSection);
+          }
           setInternalSelectedSectionId(sectionId);
         }
       }
     }
+    isInitialTabBarMode.current = false;
   }, [props.selectedSubSectionId, isMounted]);
 
   const tabContainerContainerRef = useRef(null);