From 7946cb19c1720152cc02ce3ccf422dd75a671078 Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Thu, 25 Sep 2025 10:39:40 +0200 Subject: [PATCH 1/2] fix(AnalyticalTable): improve scroll performance --- .../AnalyticalTable.stories.tsx | 4 ++ .../TableBody/VirtualTableBodyContainer.tsx | 4 +- .../AnalyticalTable/hooks/useSyncScroll.ts | 52 +++++++++++++++++++ .../src/components/AnalyticalTable/index.tsx | 34 ++---------- .../scrollbars/VerticalScrollbar.tsx | 37 ++----------- .../components/AnalyticalTable/types/index.ts | 10 +++- 6 files changed, 74 insertions(+), 67 deletions(-) create mode 100644 packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts diff --git a/packages/main/src/components/AnalyticalTable/AnalyticalTable.stories.tsx b/packages/main/src/components/AnalyticalTable/AnalyticalTable.stories.tsx index 6228fa2d062..6937374c769 100644 --- a/packages/main/src/components/AnalyticalTable/AnalyticalTable.stories.tsx +++ b/packages/main/src/components/AnalyticalTable/AnalyticalTable.stories.tsx @@ -146,6 +146,8 @@ const kitchenSinkArgs = { visibleRowCountMode: AnalyticalTableVisibleRowCountMode.Interactive, visibleRows: 5, withRowHighlight: true, + // sb actions has a huge impact on performance here. + onTableScroll: undefined, }; const meta = { @@ -179,6 +181,8 @@ const meta = { highlightField: 'status', subRowsKey: 'subRows', visibleRows: 5, + // sb actions has a huge impact on performance here. + onTableScroll: undefined, }, argTypes: { data: { control: { disable: true } }, diff --git a/packages/main/src/components/AnalyticalTable/TableBody/VirtualTableBodyContainer.tsx b/packages/main/src/components/AnalyticalTable/TableBody/VirtualTableBodyContainer.tsx index 3a068b056e5..19eb7075d19 100644 --- a/packages/main/src/components/AnalyticalTable/TableBody/VirtualTableBodyContainer.tsx +++ b/packages/main/src/components/AnalyticalTable/TableBody/VirtualTableBodyContainer.tsx @@ -74,7 +74,9 @@ export const VirtualTableBodyContainer = (props: VirtualTableBodyContainerProps) const onScroll = useCallback( (event) => { - handleExternalScroll(enrichEventWithDetails(event, { rows, rowElements: event.target.children[0].children })); + if (typeof handleExternalScroll === 'function') { + handleExternalScroll(enrichEventWithDetails(event, { rows, rowElements: event.target.children[0].children })); + } const scrollOffset = event.target.scrollTop; const isScrollingDown = lastScrollTop.current < scrollOffset; const target = event.target; diff --git a/packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts b/packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts new file mode 100644 index 00000000000..d0a9eb5a71b --- /dev/null +++ b/packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts @@ -0,0 +1,52 @@ +import type { RefObject } from 'react'; +import { useEffect, useRef, useState } from 'react'; + +export function useSyncScroll(refContent: RefObject, refScrollbar: RefObject) { + const ticking = useRef(false); + const isProgrammatic = useRef(false); + const [isMounted, setIsMounted] = useState(false); + + useEffect(() => { + const content = refContent.current; + const scrollbar = refScrollbar.current; + + if (!content || !scrollbar || !isMounted) { + setIsMounted(true); + return; + } + + scrollbar.scrollTop = content.scrollTop; + + const sync = (source: 'content' | 'scrollbar') => { + if (ticking.current) { + return; + } + ticking.current = true; + + requestAnimationFrame(() => { + const sourceEl = source === 'content' ? content : scrollbar; + const targetEl = source === 'content' ? scrollbar : content; + + if (!isProgrammatic.current && targetEl.scrollTop !== sourceEl.scrollTop) { + isProgrammatic.current = true; + targetEl.scrollTop = sourceEl.scrollTop; + // Clear the flag on next frame + requestAnimationFrame(() => (isProgrammatic.current = false)); + } + + ticking.current = false; + }); + }; + + const onScrollContent = () => sync('content'); + const onScrollScrollbar = () => sync('scrollbar'); + + content.addEventListener('scroll', onScrollContent, { passive: true }); + scrollbar.addEventListener('scroll', onScrollScrollbar, { passive: true }); + + return () => { + content.removeEventListener('scroll', onScrollContent); + scrollbar.removeEventListener('scroll', onScrollScrollbar); + }; + }, [isMounted, refContent, refScrollbar]); +} diff --git a/packages/main/src/components/AnalyticalTable/index.tsx b/packages/main/src/components/AnalyticalTable/index.tsx index a2bae2c6988..baf05b6807c 100644 --- a/packages/main/src/components/AnalyticalTable/index.tsx +++ b/packages/main/src/components/AnalyticalTable/index.tsx @@ -73,6 +73,7 @@ import { useScrollToRef } from './hooks/useScrollToRef.js'; import { useSelectionChangeCallback } from './hooks/useSelectionChangeCallback.js'; import { useSingleRowStateSelection } from './hooks/useSingleRowStateSelection.js'; import { useStyling } from './hooks/useStyling.js'; +import { useSyncScroll } from './hooks/useSyncScroll.js'; import { useToggleRowExpand } from './hooks/useToggleRowExpand.js'; import { useVisibleColumnsWidth } from './hooks/useVisibleColumnsWidth.js'; import { VerticalScrollbar } from './scrollbars/VerticalScrollbar.js'; @@ -655,34 +656,7 @@ const AnalyticalTable = forwardRef { - if (typeof onTableScroll === 'function') { - onTableScroll(e); - } - const targetScrollTop = e.currentTarget.scrollTop; - - if (verticalScrollBarRef.current) { - const vertScrollbarScrollElement = verticalScrollBarRef.current.firstElementChild as HTMLDivElement; - if (vertScrollbarScrollElement.offsetHeight !== scrollContainerRef.current?.offsetHeight) { - vertScrollbarScrollElement.style.height = `${scrollContainerRef.current.offsetHeight}px`; - } - if (verticalScrollBarRef.current.scrollTop !== targetScrollTop) { - if (!e.currentTarget.isExternalVerticalScroll) { - verticalScrollBarRef.current.scrollTop = targetScrollTop; - verticalScrollBarRef.current.isExternalVerticalScroll = true; - } - e.currentTarget.isExternalVerticalScroll = false; - } - } - }; - - const handleVerticalScrollBarScroll = useCallback((e) => { - if (parentRef.current && !e.currentTarget.isExternalVerticalScroll) { - parentRef.current.scrollTop = e.currentTarget.scrollTop; - parentRef.current.isExternalVerticalScroll = true; - } - e.currentTarget.isExternalVerticalScroll = false; - }, []); + useSyncScroll(parentRef, verticalScrollBarRef); useEffect(() => { columnVirtualizer.measure(); @@ -862,7 +836,7 @@ const AnalyticalTable = forwardRef @@ -897,10 +871,8 @@ const AnalyticalTable = forwardRef diff --git a/packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx b/packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx index e114a4b2254..164204fd9c1 100644 --- a/packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx +++ b/packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx @@ -1,7 +1,6 @@ -import { useSyncRef } from '@ui5/webcomponents-react-base'; import { clsx } from 'clsx'; import type { MutableRefObject, RefObject } from 'react'; -import { forwardRef, useEffect, useRef } from 'react'; +import { forwardRef } from 'react'; import { FlexBoxDirection } from '../../../enums/FlexBoxDirection.js'; import { FlexBox } from '../../FlexBox/index.js'; import type { ClassNames } from '../types/index.js'; @@ -9,43 +8,15 @@ import type { ClassNames } from '../types/index.js'; interface VerticalScrollbarProps { internalRowHeight: number; tableRef: RefObject; - handleVerticalScrollBarScroll: any; tableBodyHeight: number; scrollContainerRef: MutableRefObject; - parentRef: MutableRefObject; nativeScrollbar: boolean; classNames: ClassNames; } export const VerticalScrollbar = forwardRef((props, ref) => { - const { - internalRowHeight, - tableRef, - handleVerticalScrollBarScroll, - tableBodyHeight, - scrollContainerRef, - nativeScrollbar, - parentRef, - classNames, - } = props; - const [componentRef, containerRef] = useSyncRef(ref); - const scrollElementRef = useRef(null); - + const { internalRowHeight, tableRef, tableBodyHeight, scrollContainerRef, nativeScrollbar, classNames } = props; const hasHorizontalScrollbar = tableRef?.current?.offsetWidth !== tableRef?.current?.scrollWidth; - - useEffect(() => { - const observer = new ResizeObserver(([entry]) => { - if (containerRef.current && parentRef.current && entry.target.getBoundingClientRect().height > 0) { - containerRef.current.scrollTop = parentRef.current.scrollTop; - } - }); - if (scrollElementRef.current) { - observer.observe(scrollElementRef.current); - } - return () => { - observer.disconnect(); - }; - }, []); const horizontalScrollbarSectionStyles = clsx(hasHorizontalScrollbar && classNames.bottomSection); return ( @@ -61,11 +32,10 @@ export const VerticalScrollbar = forwardRef
| ((props?: CellInstance) => ReactNode); /** @@ -1018,6 +1022,10 @@ export interface AnalyticalTablePropTypes extends Omit { onLoadMore?: (e?: CustomEvent<{ rowCount: number; totalRowCount: number }>) => void; /** * Fired when the body of the table is scrolled. + * + * __Note:__ This callback __must be memoized__! Since it is triggered on __every scroll event__, + * non-memoized or expensive calculations can have a __huge impact on performance__ and cause visible lag. + * Throttling or debouncing is always recommended to reduce performance overhead. */ onTableScroll?: (e?: CustomEvent<{ rows: Record[]; rowElements: HTMLCollection }>) => void; /** From 2814b0abc077569356ee39eab7270ab5ae5a325a Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Thu, 25 Sep 2025 10:48:25 +0200 Subject: [PATCH 2/2] React18 compatible types --- .../src/components/AnalyticalTable/hooks/useSyncScroll.ts | 4 ++-- .../AnalyticalTable/scrollbars/VerticalScrollbar.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts b/packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts index d0a9eb5a71b..d6d39b3f902 100644 --- a/packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts +++ b/packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts @@ -1,7 +1,7 @@ -import type { RefObject } from 'react'; +import type { MutableRefObject } from 'react'; import { useEffect, useRef, useState } from 'react'; -export function useSyncScroll(refContent: RefObject, refScrollbar: RefObject) { +export function useSyncScroll(refContent: MutableRefObject, refScrollbar: MutableRefObject) { const ticking = useRef(false); const isProgrammatic = useRef(false); const [isMounted, setIsMounted] = useState(false); diff --git a/packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx b/packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx index 164204fd9c1..9d755783dc2 100644 --- a/packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx +++ b/packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx @@ -1,5 +1,5 @@ import { clsx } from 'clsx'; -import type { MutableRefObject, RefObject } from 'react'; +import type { MutableRefObject } from 'react'; import { forwardRef } from 'react'; import { FlexBoxDirection } from '../../../enums/FlexBoxDirection.js'; import { FlexBox } from '../../FlexBox/index.js'; @@ -7,7 +7,7 @@ import type { ClassNames } from '../types/index.js'; interface VerticalScrollbarProps { internalRowHeight: number; - tableRef: RefObject; + tableRef: MutableRefObject; tableBodyHeight: number; scrollContainerRef: MutableRefObject; nativeScrollbar: boolean;