Skip to content

Commit 1e70896

Browse files
authored
fix(AnalyticalTable): improve scroll performance (#7777)
Fixes #6615 Scrolling improvements in general, but especially noticeable for scrollbar-based scrolling, which was previously laggy.
1 parent 5348913 commit 1e70896

File tree

6 files changed

+76
-69
lines changed

6 files changed

+76
-69
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ const kitchenSinkArgs: AnalyticalTablePropTypes = {
158158
visibleRowCountMode: AnalyticalTableVisibleRowCountMode.Interactive,
159159
visibleRows: 5,
160160
withRowHighlight: true,
161+
// sb actions has a huge impact on performance here.
162+
onTableScroll: undefined,
161163
};
162164

163165
const meta = {
@@ -191,6 +193,8 @@ const meta = {
191193
highlightField: 'status',
192194
subRowsKey: 'subRows',
193195
visibleRows: 5,
196+
// sb actions has a huge impact on performance here.
197+
onTableScroll: undefined,
194198
},
195199
argTypes: {
196200
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 { MutableRefObject } from 'react';
2+
import { useEffect, useRef, useState } from 'react';
3+
4+
export function useSyncScroll(refContent: MutableRefObject<HTMLElement>, refScrollbar: MutableRefObject<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
@@ -74,6 +74,7 @@ import { useScrollToRef } from './hooks/useScrollToRef.js';
7474
import { useSelectionChangeCallback } from './hooks/useSelectionChangeCallback.js';
7575
import { useSingleRowStateSelection } from './hooks/useSingleRowStateSelection.js';
7676
import { useStyling } from './hooks/useStyling.js';
77+
import { useSyncScroll } from './hooks/useSyncScroll.js';
7778
import { useToggleRowExpand } from './hooks/useToggleRowExpand.js';
7879
import { useVisibleColumnsWidth } from './hooks/useVisibleColumnsWidth.js';
7980
import { VerticalScrollbar } from './scrollbars/VerticalScrollbar.js';
@@ -656,34 +657,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
656657
}
657658
}, [tableState.columnResizing, retainColumnWidth, tableState.tableColResized]);
658659

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

688662
useEffect(() => {
689663
columnVirtualizer.measure();
@@ -870,7 +844,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
870844
internalRowHeight={internalRowHeight}
871845
popInRowHeight={popInRowHeight}
872846
rows={rows}
873-
handleExternalScroll={handleBodyScroll}
847+
handleExternalScroll={onTableScroll}
874848
visibleRows={internalVisibleRowCount}
875849
isGrouped={isGrouped}
876850
>
@@ -905,10 +879,8 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
905879
tableBodyHeight={tableBodyHeight}
906880
internalRowHeight={internalHeaderRowHeight}
907881
tableRef={tableRef}
908-
handleVerticalScrollBarScroll={handleVerticalScrollBarScroll}
909882
ref={verticalScrollBarRef}
910883
scrollContainerRef={scrollContainerRef}
911-
parentRef={parentRef}
912884
nativeScrollbar={nativeScrollbar}
913885
classNames={classNames}
914886
/>

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

Lines changed: 5 additions & 36 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';
3-
import type { MutableRefObject, RefObject } from 'react';
4-
import { forwardRef, useEffect, useRef } from 'react';
2+
import type { MutableRefObject } 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;
11-
tableRef: RefObject<any>;
12-
handleVerticalScrollBarScroll: any;
10+
tableRef: MutableRefObject<HTMLDivElement>;
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
@@ -427,7 +427,11 @@ export interface AnalyticalTableColumnDefinition {
427427
*/
428428
headerTooltip?: string;
429429
/**
430-
* 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`
430+
* Custom cell renderer. If set, the table will use this component or render the provided string for every cell,
431+
* passing all necessary information as props, e.g., the cell value as `props.cell.value`.
432+
*
433+
* __Note:__ Using a custom component __can impact performance__!
434+
* If you pass a component, __memoizing it is strongly recommended__, especially for complex components or large datasets.
431435
*/
432436
Cell?: string | ComponentType<CellInstance> | ((props?: CellInstance) => ReactNode);
433437
/**
@@ -1019,6 +1023,10 @@ export interface AnalyticalTablePropTypes extends Omit<CommonProps, 'title'> {
10191023
onLoadMore?: (e?: CustomEvent<{ rowCount: number; totalRowCount: number }>) => void;
10201024
/**
10211025
* Fired when the body of the table is scrolled.
1026+
*
1027+
* __Note:__ This callback __must be memoized__! Since it is triggered on __every scroll event__,
1028+
* non-memoized or expensive calculations can have a __huge impact on performance__ and cause visible lag.
1029+
* Throttling or debouncing is always recommended to reduce performance overhead.
10221030
*/
10231031
onTableScroll?: (e?: CustomEvent<{ rows: Record<string, any>[]; rowElements: HTMLCollection }>) => void;
10241032
/**

0 commit comments

Comments
 (0)