Skip to content

Commit 7946cb1

Browse files
committed
fix(AnalyticalTable): improve scroll performance
1 parent 3b89dfd commit 7946cb1

File tree

6 files changed

+74
-67
lines changed

6 files changed

+74
-67
lines changed

packages/main/src/components/AnalyticalTable/AnalyticalTable.stories.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ const kitchenSinkArgs = {
146146
visibleRowCountMode: AnalyticalTableVisibleRowCountMode.Interactive,
147147
visibleRows: 5,
148148
withRowHighlight: true,
149+
// sb actions has a huge impact on performance here.
150+
onTableScroll: undefined,
149151
};
150152

151153
const meta = {
@@ -179,6 +181,8 @@ const meta = {
179181
highlightField: 'status',
180182
subRowsKey: 'subRows',
181183
visibleRows: 5,
184+
// sb actions has a huge impact on performance here.
185+
onTableScroll: undefined,
182186
},
183187
argTypes: {
184188
data: { control: { disable: true } },

packages/main/src/components/AnalyticalTable/TableBody/VirtualTableBodyContainer.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ export const VirtualTableBodyContainer = (props: VirtualTableBodyContainerProps)
7474

7575
const onScroll = useCallback(
7676
(event) => {
77-
handleExternalScroll(enrichEventWithDetails(event, { rows, rowElements: event.target.children[0].children }));
77+
if (typeof handleExternalScroll === 'function') {
78+
handleExternalScroll(enrichEventWithDetails(event, { rows, rowElements: event.target.children[0].children }));
79+
}
7880
const scrollOffset = event.target.scrollTop;
7981
const isScrollingDown = lastScrollTop.current < scrollOffset;
8082
const target = event.target;
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import type { RefObject } from 'react';
2+
import { useEffect, useRef, useState } from 'react';
3+
4+
export function useSyncScroll(refContent: RefObject<HTMLElement>, refScrollbar: RefObject<HTMLElement>) {
5+
const ticking = useRef(false);
6+
const isProgrammatic = useRef(false);
7+
const [isMounted, setIsMounted] = useState(false);
8+
9+
useEffect(() => {
10+
const content = refContent.current;
11+
const scrollbar = refScrollbar.current;
12+
13+
if (!content || !scrollbar || !isMounted) {
14+
setIsMounted(true);
15+
return;
16+
}
17+
18+
scrollbar.scrollTop = content.scrollTop;
19+
20+
const sync = (source: 'content' | 'scrollbar') => {
21+
if (ticking.current) {
22+
return;
23+
}
24+
ticking.current = true;
25+
26+
requestAnimationFrame(() => {
27+
const sourceEl = source === 'content' ? content : scrollbar;
28+
const targetEl = source === 'content' ? scrollbar : content;
29+
30+
if (!isProgrammatic.current && targetEl.scrollTop !== sourceEl.scrollTop) {
31+
isProgrammatic.current = true;
32+
targetEl.scrollTop = sourceEl.scrollTop;
33+
// Clear the flag on next frame
34+
requestAnimationFrame(() => (isProgrammatic.current = false));
35+
}
36+
37+
ticking.current = false;
38+
});
39+
};
40+
41+
const onScrollContent = () => sync('content');
42+
const onScrollScrollbar = () => sync('scrollbar');
43+
44+
content.addEventListener('scroll', onScrollContent, { passive: true });
45+
scrollbar.addEventListener('scroll', onScrollScrollbar, { passive: true });
46+
47+
return () => {
48+
content.removeEventListener('scroll', onScrollContent);
49+
scrollbar.removeEventListener('scroll', onScrollScrollbar);
50+
};
51+
}, [isMounted, refContent, refScrollbar]);
52+
}

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

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import { useScrollToRef } from './hooks/useScrollToRef.js';
7373
import { useSelectionChangeCallback } from './hooks/useSelectionChangeCallback.js';
7474
import { useSingleRowStateSelection } from './hooks/useSingleRowStateSelection.js';
7575
import { useStyling } from './hooks/useStyling.js';
76+
import { useSyncScroll } from './hooks/useSyncScroll.js';
7677
import { useToggleRowExpand } from './hooks/useToggleRowExpand.js';
7778
import { useVisibleColumnsWidth } from './hooks/useVisibleColumnsWidth.js';
7879
import { VerticalScrollbar } from './scrollbars/VerticalScrollbar.js';
@@ -655,34 +656,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
655656
}
656657
}, [tableState.columnResizing, retainColumnWidth, tableState.tableColResized]);
657658

658-
const handleBodyScroll = (e) => {
659-
if (typeof onTableScroll === 'function') {
660-
onTableScroll(e);
661-
}
662-
const targetScrollTop = e.currentTarget.scrollTop;
663-
664-
if (verticalScrollBarRef.current) {
665-
const vertScrollbarScrollElement = verticalScrollBarRef.current.firstElementChild as HTMLDivElement;
666-
if (vertScrollbarScrollElement.offsetHeight !== scrollContainerRef.current?.offsetHeight) {
667-
vertScrollbarScrollElement.style.height = `${scrollContainerRef.current.offsetHeight}px`;
668-
}
669-
if (verticalScrollBarRef.current.scrollTop !== targetScrollTop) {
670-
if (!e.currentTarget.isExternalVerticalScroll) {
671-
verticalScrollBarRef.current.scrollTop = targetScrollTop;
672-
verticalScrollBarRef.current.isExternalVerticalScroll = true;
673-
}
674-
e.currentTarget.isExternalVerticalScroll = false;
675-
}
676-
}
677-
};
678-
679-
const handleVerticalScrollBarScroll = useCallback((e) => {
680-
if (parentRef.current && !e.currentTarget.isExternalVerticalScroll) {
681-
parentRef.current.scrollTop = e.currentTarget.scrollTop;
682-
parentRef.current.isExternalVerticalScroll = true;
683-
}
684-
e.currentTarget.isExternalVerticalScroll = false;
685-
}, []);
659+
useSyncScroll(parentRef, verticalScrollBarRef);
686660

687661
useEffect(() => {
688662
columnVirtualizer.measure();
@@ -862,7 +836,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
862836
internalRowHeight={internalRowHeight}
863837
popInRowHeight={popInRowHeight}
864838
rows={rows}
865-
handleExternalScroll={handleBodyScroll}
839+
handleExternalScroll={onTableScroll}
866840
visibleRows={internalVisibleRowCount}
867841
isGrouped={isGrouped}
868842
>
@@ -897,10 +871,8 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
897871
tableBodyHeight={tableBodyHeight}
898872
internalRowHeight={internalHeaderRowHeight}
899873
tableRef={tableRef}
900-
handleVerticalScrollBarScroll={handleVerticalScrollBarScroll}
901874
ref={verticalScrollBarRef}
902875
scrollContainerRef={scrollContainerRef}
903-
parentRef={parentRef}
904876
nativeScrollbar={nativeScrollbar}
905877
classNames={classNames}
906878
/>

packages/main/src/components/AnalyticalTable/scrollbars/VerticalScrollbar.tsx

Lines changed: 3 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,22 @@
1-
import { useSyncRef } from '@ui5/webcomponents-react-base';
21
import { clsx } from 'clsx';
32
import type { MutableRefObject, RefObject } from 'react';
4-
import { forwardRef, useEffect, useRef } from 'react';
3+
import { forwardRef } from 'react';
54
import { FlexBoxDirection } from '../../../enums/FlexBoxDirection.js';
65
import { FlexBox } from '../../FlexBox/index.js';
76
import type { ClassNames } from '../types/index.js';
87

98
interface VerticalScrollbarProps {
109
internalRowHeight: number;
1110
tableRef: RefObject<any>;
12-
handleVerticalScrollBarScroll: any;
1311
tableBodyHeight: number;
1412
scrollContainerRef: MutableRefObject<HTMLDivElement>;
15-
parentRef: MutableRefObject<HTMLDivElement>;
1613
nativeScrollbar: boolean;
1714
classNames: ClassNames;
1815
}
1916

2017
export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarProps>((props, ref) => {
21-
const {
22-
internalRowHeight,
23-
tableRef,
24-
handleVerticalScrollBarScroll,
25-
tableBodyHeight,
26-
scrollContainerRef,
27-
nativeScrollbar,
28-
parentRef,
29-
classNames,
30-
} = props;
31-
const [componentRef, containerRef] = useSyncRef(ref);
32-
const scrollElementRef = useRef(null);
33-
18+
const { internalRowHeight, tableRef, tableBodyHeight, scrollContainerRef, nativeScrollbar, classNames } = props;
3419
const hasHorizontalScrollbar = tableRef?.current?.offsetWidth !== tableRef?.current?.scrollWidth;
35-
36-
useEffect(() => {
37-
const observer = new ResizeObserver(([entry]) => {
38-
if (containerRef.current && parentRef.current && entry.target.getBoundingClientRect().height > 0) {
39-
containerRef.current.scrollTop = parentRef.current.scrollTop;
40-
}
41-
});
42-
if (scrollElementRef.current) {
43-
observer.observe(scrollElementRef.current);
44-
}
45-
return () => {
46-
observer.disconnect();
47-
};
48-
}, []);
4920
const horizontalScrollbarSectionStyles = clsx(hasHorizontalScrollbar && classNames.bottomSection);
5021

5122
return (
@@ -61,11 +32,10 @@ export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarPro
6132
className={classNames.headerSection}
6233
/>
6334
<div
64-
ref={componentRef}
35+
ref={ref}
6536
style={{
6637
height: tableRef.current ? `${tableBodyHeight}px` : '0',
6738
}}
68-
onScroll={handleVerticalScrollBarScroll}
6939
className={clsx(
7040
classNames.scrollbar,
7141
nativeScrollbar
@@ -76,7 +46,6 @@ export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarPro
7646
tabIndex={-1}
7747
>
7848
<div
79-
ref={scrollElementRef}
8049
className={classNames.verticalScroller}
8150
style={{
8251
height: `${scrollContainerRef.current?.scrollHeight}px`,

packages/main/src/components/AnalyticalTable/types/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,11 @@ export interface AnalyticalTableColumnDefinition {
426426
*/
427427
headerTooltip?: string;
428428
/**
429-
* Custom cell renderer. If set, the table will call that component for every cell and pass all required information as props, e.g. the cell value as `props.cell.value`
429+
* Custom cell renderer. If set, the table will use this component or render the provided string for every cell,
430+
* passing all necessary information as props, e.g., the cell value as `props.cell.value`.
431+
*
432+
* __Note:__ Using a custom component __can impact performance__!
433+
* If you pass a component, __memoizing it is strongly recommended__, especially for complex components or large datasets.
430434
*/
431435
Cell?: string | ComponentType<CellInstance> | ((props?: CellInstance) => ReactNode);
432436
/**
@@ -1018,6 +1022,10 @@ export interface AnalyticalTablePropTypes extends Omit<CommonProps, 'title'> {
10181022
onLoadMore?: (e?: CustomEvent<{ rowCount: number; totalRowCount: number }>) => void;
10191023
/**
10201024
* Fired when the body of the table is scrolled.
1025+
*
1026+
* __Note:__ This callback __must be memoized__! Since it is triggered on __every scroll event__,
1027+
* non-memoized or expensive calculations can have a __huge impact on performance__ and cause visible lag.
1028+
* Throttling or debouncing is always recommended to reduce performance overhead.
10211029
*/
10221030
onTableScroll?: (e?: CustomEvent<{ rows: Record<string, any>[]; rowElements: HTMLCollection }>) => void;
10231031
/**

0 commit comments

Comments
 (0)