fix(ObjectPage): improve selection & scroll behavior (#6492) (#6507)

Fixes #6478

Author: Lukas742

packages/main/src/components/ObjectPage/ObjectPageUtils.ts

@@ -6,8 +6,3 @@ export const getSectionById = (sections, id) => {
     return isValidElement(objectPageSection) && objectPageSection.props?.id === id;
   });
 };
-
-export const extractSectionIdFromHtmlId = (id: string) => {
-  if (!id) return null;
-  return id.replace(/^ObjectPageSection-/, '');
-};

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

@@ -25,7 +25,7 @@ import { DynamicPageHeader } from '../DynamicPageHeader/index.js';
 import type { ObjectPageSectionPropTypes } from '../ObjectPageSection/index.js';
 import { CollapsedAvatar } from './CollapsedAvatar.js';
 import { classNames, styleData } from './ObjectPage.module.css.js';
-import { extractSectionIdFromHtmlId, getSectionById } from './ObjectPageUtils.js';
+import { getSectionById } from './ObjectPageUtils.js';
 
 addCustomCSSWithScoping(
   'ui5-tabcontainer',
@@ -216,7 +216,6 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
   const anchorBarRef = useRef<HTMLDivElement>(null);
   const objectPageContentRef = useRef<HTMLDivElement>(null);
   const selectionScrollTimeout = useRef(null);
-  const [isAfterScroll, setIsAfterScroll] = useState(false);
   const isToggledRef = useRef(false);
   const [headerCollapsedInternal, setHeaderCollapsedInternal] = useState<undefined | boolean>(undefined);
   const [scrolledHeaderExpanded, setScrolledHeaderExpanded] = useState(false);
@@ -245,7 +244,7 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
 
   const prevInternalSelectedSectionId = useRef(internalSelectedSectionId);
   const fireOnSelectedChangedEvent = (targetEvent, index, id, section) => {
-    if (typeof onSelectedSectionChange === 'function' && prevInternalSelectedSectionId.current !== id) {
+    if (typeof onSelectedSectionChange === 'function' && targetEvent && prevInternalSelectedSectionId.current !== id) {
       onSelectedSectionChange(
         enrichEventWithDetails(targetEvent, {
           selectedSectionIndex: parseInt(index, 10),
@@ -318,7 +317,7 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
         safeTopHeaderHeight +
         anchorBarHeight +
         TAB_CONTAINER_HEADER_HEIGHT +
-        (headerPinned ? headerContentHeight : 0) +
+        (headerPinned && !headerCollapsed ? headerContentHeight : 0) +
         'px';
       section.focus();
       section.scrollIntoView({ behavior: 'smooth' });
@@ -527,73 +526,53 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
 
   const { onScroll: _0, selectedSubSectionId: _1, ...propsWithoutOmitted } = rest;
 
+  const visibleSectionIds = useRef<Set<string>>(new Set());
   useEffect(() => {
     const sectionNodes = objectPageRef.current?.querySelectorAll('section[data-component-name="ObjectPageSection"]');
-    const objectPageHeight = objectPageRef.current?.clientHeight ?? 1000;
-    const marginBottom = objectPageHeight - totalHeaderHeight - /*TabContainer*/ TAB_CONTAINER_HEADER_HEIGHT;
-    const rootMargin = `-${totalHeaderHeight}px 0px -${marginBottom < 0 ? 0 : marginBottom}px 0px`;
+    // only the sticky part of the header must be added as margin
+    const rootMargin = `-${(headerPinned && !headerCollapsed ? totalHeaderHeight : topHeaderHeight) + TAB_CONTAINER_HEADER_HEIGHT}px 0px 0px 0px`;
+
     const observer = new IntersectionObserver(
-      ([section]) => {
-        if (section.isIntersecting && isProgrammaticallyScrolled.current === false) {
-          if (
-            objectPageRef.current.getBoundingClientRect().top + totalHeaderHeight + TAB_CONTAINER_HEADER_HEIGHT <=
-            section.target.getBoundingClientRect().bottom
-          ) {
-            const currentId = extractSectionIdFromHtmlId(section.target.id);
-            setInternalSelectedSectionId(currentId);
-            const currentIndex = safeGetChildrenArray(children).findIndex((objectPageSection) => {
-              return isValidElement(objectPageSection) && objectPageSection.props?.id === currentId;
-            });
-            debouncedOnSectionChange(scrollEvent.current, currentIndex, currentId, section.target);
+      (entries) => {
+        entries.forEach((entry) => {
+          const sectionId = entry.target.id;
+          if (entry.isIntersecting) {
+            visibleSectionIds.current.add(sectionId);
+          } else {
+            visibleSectionIds.current.delete(sectionId);
           }
-        }
+
+          let currentIndex: undefined | number;
+          const sortedVisibleSections = Array.from(sectionNodes).filter((section, index) => {
+            const isVisibleSection = visibleSectionIds.current.has(section.id);
+            if (currentIndex === undefined && isVisibleSection) {
+              currentIndex = index;
+            }
+            return visibleSectionIds.current.has(section.id);
+          });
+
+          if (sortedVisibleSections.length > 0) {
+            const section = sortedVisibleSections[0];
+            const id = sortedVisibleSections[0].id.slice(18);
+            setInternalSelectedSectionId(id);
+            debouncedOnSectionChange(scrollEvent.current, currentIndex, id, section);
+          }
+        });
       },
       {
         root: objectPageRef.current,
         rootMargin,
         threshold: [0]
       }
     );
-
     sectionNodes.forEach((el) => {
       observer.observe(el);
     });
 
     return () => {
       observer.disconnect();
     };
-  }, [children, totalHeaderHeight, setInternalSelectedSectionId, isProgrammaticallyScrolled]);
-
-  // Fallback when scrolling faster than the IntersectionObserver can observe (in most cases faster than 60fps)
-  useEffect(() => {
-    const sectionNodes = objectPageRef.current?.querySelectorAll('section[data-component-name="ObjectPageSection"]');
-    if (isAfterScroll) {
-      let currentSection = sectionNodes[sectionNodes.length - 1];
-      let currentIndex: number;
-      for (let i = 0; i <= sectionNodes.length - 1; i++) {
-        const sectionNode = sectionNodes[i];
-        if (
-          objectPageRef.current.getBoundingClientRect().top + totalHeaderHeight + TAB_CONTAINER_HEADER_HEIGHT <=
-          sectionNode.getBoundingClientRect().bottom
-        ) {
-          currentSection = sectionNode;
-          currentIndex = i;
-          break;
-        }
-      }
-      const currentSectionId = extractSectionIdFromHtmlId(currentSection?.id);
-      if (currentSectionId !== internalSelectedSectionId) {
-        setInternalSelectedSectionId(currentSectionId);
-        debouncedOnSectionChange(
-          scrollEvent.current,
-          currentIndex ?? sectionNodes.length - 1,
-          currentSectionId,
-          currentSection
-        );
-      }
-      setIsAfterScroll(false);
-    }
-  }, [isAfterScroll]);
+  }, [children, totalHeaderHeight, setInternalSelectedSectionId, headerPinned, debouncedOnSectionChange]);
 
   const titleHeaderNotClickable =
     (alwaysShowContentHeader && !headerContentPinnable) ||
@@ -744,9 +723,6 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
       if (selectionScrollTimeout.current) {
         clearTimeout(selectionScrollTimeout.current);
       }
-      selectionScrollTimeout.current = setTimeout(() => {
-        setIsAfterScroll(true);
-      }, 100);
       if (!headerPinned || e.target.scrollTop === 0) {
         objectPageRef.current?.classList.remove(classNames.headerCollapsed);
       }
@@ -907,7 +883,7 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
         </div>
       )}
       <div data-component-name="ObjectPageContent" className={classNames.content} ref={objectPageContentRef}>
-        <div style={{ height: headerCollapsed ? `${headerContentHeight}px` : 0 }} aria-hidden />
+        <div style={{ height: headerCollapsed && !headerPinned ? `${headerContentHeight}px` : 0 }} aria-hidden />
         {placeholder ? placeholder : sections}
         <div style={{ height: `${sectionSpacer}px` }} aria-hidden />
       </div>