Skip to content

Commit a03dd29

Browse files
Lukas742Lukas742
authored
fix(AnalyticalTable): fix vertical scrollbar styles and sync logic (#7839)
__Chrome:__ - scrollbar track height is not updated correctly in some cases <img width="157" height="301" alt="image" src="https://github.com/user-attachments/assets/009128e9-98bb-40d7-a3ed-af5d89c69e82" /> _Here the table is scrolled to the last row_ __Solution:__ force reflow to update scrollbar track height __Firefox:__ - scrollbar container do not automatically adjust to the scrollbar width, leading to hidden scrollbars if no `width` is set. - scrollbars do not scale in Firefox -> scrollbar container width would have to be calculated according to scale value. __Solution:__ Use native scrollbar and add padding in last cells to prevent content from being hidden __General:__ - Improve scroll performance when dragging scrollbar - Fix issue with scroll synchronization when updating `data` ___ Fixes #7831
1 parent acb1709 commit a03dd29

File tree

8 files changed

+163
-35
lines changed

8 files changed

+163
-35
lines changed

‎packages/main/src/components/AnalyticalTable/AnalyticalTable.cy.tsx‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4304,6 +4304,66 @@ describe('AnalyticalTable', () => {
43044304
cy.focused().should('have.text', 'Before');
43054305
});
43064306

4307+
it('vertical scroll sync', () => {
4308+
cy.mount(<AnalyticalTable columns={columns} data={generateMoreData(100)} />);
4309+
4310+
cy.get('[data-component-name="AnalyticalTableBody"]').scrollTo(0, 2000).should('have.prop', 'scrollTop', 2000);
4311+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 2000);
4312+
4313+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]')
4314+
.scrollTo(0, 3000)
4315+
.should('have.prop', 'scrollTop', 3000);
4316+
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 3000);
4317+
4318+
cy.get('[data-component-name="AnalyticalTableContainerWithScrollbar"]').realMouseWheel({ deltaY: 500 });
4319+
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 3500);
4320+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 3500);
4321+
4322+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').realMouseWheel({ deltaY: -1000 });
4323+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 2500);
4324+
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 2500);
4325+
4326+
const TestComp = () => {
4327+
const [_data, setData] = useState([]);
4328+
useEffect(() => {
4329+
setTimeout(() => {
4330+
setData(generateMoreData(100));
4331+
}, 100);
4332+
}, []);
4333+
4334+
return (
4335+
<>
4336+
<div style={{ height: '500px' }}>
4337+
<AnalyticalTable
4338+
columns={columns}
4339+
data={_data}
4340+
header={<div>Header</div>}
4341+
visibleRowCountMode="AutoWithEmptyRows"
4342+
/>
4343+
</div>
4344+
</>
4345+
);
4346+
};
4347+
4348+
cy.mount(<TestComp />);
4349+
4350+
cy.get('[data-component-name="AnalyticalTableBody"]').scrollTo(0, 2000).should('have.prop', 'scrollTop', 2000);
4351+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 2000);
4352+
4353+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]')
4354+
.scrollTo(0, 3000)
4355+
.should('have.prop', 'scrollTop', 3000);
4356+
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 3000);
4357+
4358+
cy.get('[data-component-name="AnalyticalTableContainerWithScrollbar"]').realMouseWheel({ deltaY: 500 });
4359+
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 3500);
4360+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 3500);
4361+
4362+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').realMouseWheel({ deltaY: -1000 });
4363+
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 2500);
4364+
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 2500);
4365+
});
4366+
43074367
cypressPassThroughTestsFactory(AnalyticalTable, { data, columns });
43084368
});
43094369

‎packages/main/src/components/AnalyticalTable/AnalyticalTable.module.css‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,3 +607,17 @@
607607
box-sizing: border-box;
608608
border-inline-end: var(--_ui5wcr-AnalyticalTable-OuterBorderInline);
609609
}
610+
611+
/* ==========================================================================
612+
Firefox scrollbar styles
613+
========================================================================== */
614+
615+
.firefoxCell {
616+
&:last-child {
617+
padding-inline-end: 18px;
618+
}
619+
}
620+
621+
.firefoxNativeScrollbar {
622+
scrollbar-width: inherit;
623+
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { enrichEventWithDetails } from '@ui5/webcomponents-react-base';
2+
import { clsx } from 'clsx';
23
import type { MutableRefObject } from 'react';
34
import { useCallback, useEffect, useRef, useState } from 'react';
45
import type { AnalyticalTablePropTypes, TableInstance } from '../types/index.js';
@@ -20,6 +21,7 @@ interface VirtualTableBodyContainerProps {
2021
rowCollapsedFlag?: boolean;
2122
dispatch: (e: { type: string; payload?: any }) => void;
2223
isGrouped: boolean;
24+
isFirefox: boolean;
2325
}
2426

2527
export const VirtualTableBodyContainer = (props: VirtualTableBodyContainerProps) => {
@@ -39,6 +41,7 @@ export const VirtualTableBodyContainer = (props: VirtualTableBodyContainerProps)
3941
popInRowHeight,
4042
rowCollapsedFlag,
4143
isGrouped,
44+
isFirefox,
4245
dispatch,
4346
} = props;
4447
const [isMounted, setIsMounted] = useState(false);
@@ -114,7 +117,7 @@ export const VirtualTableBodyContainer = (props: VirtualTableBodyContainerProps)
114117

115118
return (
116119
<div
117-
className={classes.tbody}
120+
className={clsx(classes.tbody, isFirefox && classes.firefoxNativeScrollbar)}
118121
ref={parentRef}
119122
onScroll={onScroll}
120123
style={{

‎packages/main/src/components/AnalyticalTable/hooks/useIsFirefox.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// When reused, move to base pkg
2+
import { isFirefox as isFirefoxFn } from '@ui5/webcomponents-react-base/Device';
3+
import { useEffect, useState } from 'react';
4+
5+
/**
6+
* SSR ready `isFirefox` check.
7+
*/
8+
export function useIsFirefox() {
9+
const [isFirefox, setIsFirefox] = useState(false);
10+
11+
useEffect(() => {
12+
setIsFirefox(isFirefoxFn());
13+
}, []);
14+
15+
return isFirefox;
16+
}

‎packages/main/src/components/AnalyticalTable/hooks/useStyling.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { clsx } from 'clsx';
12
import type { CSSProperties } from 'react';
23
import { AnalyticalTableSelectionBehavior } from '../../../enums/AnalyticalTableSelectionBehavior.js';
34
import { AnalyticalTableSelectionMode } from '../../../enums/AnalyticalTableSelectionMode.js';
@@ -85,14 +86,10 @@ const getRowProps = (
8586
};
8687

8788
const getCellProps = (cellProps, { cell: { column }, instance }) => {
88-
const { classes } = instance.webComponentsReactProperties;
89+
const { webComponentsReactProperties, state } = instance;
90+
const { classes, isFirefox } = webComponentsReactProperties;
8991
const style: CSSProperties = { width: `${column.totalWidth}px`, ...resolveCellAlignment(column) };
9092

91-
let className = classes.tableCell;
92-
if (column.className) {
93-
className += ` ${column.className}`;
94-
}
95-
9693
if (
9794
column.id === '__ui5wcr__internal_highlight_column' ||
9895
column.id === '__ui5wcr__internal_selection_column' ||
@@ -104,7 +101,12 @@ const getCellProps = (cellProps, { cell: { column }, instance }) => {
104101
return [
105102
cellProps,
106103
{
107-
className,
104+
className: clsx(
105+
cellProps.className,
106+
classes.tableCell,
107+
column.className,
108+
isFirefox && state.isScrollable && classes.firefoxCell,
109+
),
108110
style,
109111
tabIndex: -1,
110112
},

‎packages/main/src/components/AnalyticalTable/hooks/useSyncScroll.ts‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
import type { MutableRefObject } from 'react';
22
import { useEffect, useRef, useState } from 'react';
33

4-
export function useSyncScroll(refContent: MutableRefObject<HTMLElement>, refScrollbar: MutableRefObject<HTMLElement>) {
5-
const ticking = useRef(false);
4+
export function useSyncScroll(
5+
refContent: MutableRefObject<HTMLElement>,
6+
refScrollbar: MutableRefObject<HTMLElement>,
7+
isScrollable: boolean,
8+
disabled = false,
9+
) {
610
const isProgrammatic = useRef(false);
711
const [isMounted, setIsMounted] = useState(false);
812

913
useEffect(() => {
14+
if (disabled || !isScrollable) {
15+
return;
16+
}
17+
1018
const content = refContent.current;
1119
const scrollbar = refScrollbar.current;
1220

@@ -18,24 +26,15 @@ export function useSyncScroll(refContent: MutableRefObject<HTMLElement>, refScro
1826
scrollbar.scrollTop = content.scrollTop;
1927

2028
const sync = (source: 'content' | 'scrollbar') => {
21-
if (ticking.current) {
22-
return;
29+
const sourceEl = source === 'content' ? content : scrollbar;
30+
const targetEl = source === 'content' ? scrollbar : content;
31+
32+
if (!isProgrammatic.current && targetEl.scrollTop !== sourceEl.scrollTop) {
33+
isProgrammatic.current = true;
34+
targetEl.scrollTop = sourceEl.scrollTop;
35+
// Clear the flag on next frame
36+
requestAnimationFrame(() => (isProgrammatic.current = false));
2337
}
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-
});
3938
};
4039

4140
const onScrollContent = () => sync('content');
@@ -48,5 +47,5 @@ export function useSyncScroll(refContent: MutableRefObject<HTMLElement>, refScro
4847
content.removeEventListener('scroll', onScrollContent);
4948
scrollbar.removeEventListener('scroll', onScrollScrollbar);
5049
};
51-
}, [isMounted, refContent, refScrollbar]);
50+
}, [isMounted, refContent, refScrollbar, disabled, isScrollable]);
5251
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import { useColumnsDeps } from './hooks/useColumnsDeps.js';
6464
import { useColumnDragAndDrop } from './hooks/useDragAndDrop.js';
6565
import { useDynamicColumnWidths } from './hooks/useDynamicColumnWidths.js';
6666
import { useFontsReady } from './hooks/useFontsReady.js';
67+
import { useIsFirefox } from './hooks/useIsFirefox.js';
6768
import { useKeyboardNavigation } from './hooks/useKeyboardNavigation.js';
6869
import { usePopIn } from './hooks/usePopIn.js';
6970
import { useResizeColumnsConfig } from './hooks/useResizeColumnsConfig.js';
@@ -190,6 +191,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
190191
useStylesheet(styleData, AnalyticalTable.displayName);
191192
const isInitialized = useRef(false);
192193
const fontsReady = useFontsReady();
194+
const isFirefox = useIsFirefox();
193195

194196
const alwaysShowSubComponent =
195197
subComponentsBehavior === AnalyticalTableSubComponentsBehavior.Visible ||
@@ -256,6 +258,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
256258
classes: classNames,
257259
fontsReady,
258260
highlightField,
261+
isFirefox,
259262
isTreeTable,
260263
loading,
261264
markNavigatedRow,
@@ -658,7 +661,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
658661
}
659662
}, [tableState.columnResizing, retainColumnWidth, tableState.tableColResized]);
660663

661-
useSyncScroll(parentRef, verticalScrollBarRef);
664+
useSyncScroll(parentRef, verticalScrollBarRef, tableState.isScrollable, isFirefox);
662665

663666
useEffect(() => {
664667
columnVirtualizer.measure();
@@ -844,6 +847,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
844847
handleExternalScroll={onTableScroll}
845848
visibleRows={internalVisibleRowCount}
846849
isGrouped={isGrouped}
850+
isFirefox={isFirefox}
847851
>
848852
<VirtualTableBody
849853
scrollContainerRef={scrollContainerRef}
@@ -871,7 +875,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
871875
</VirtualTableBodyContainer>
872876
)}
873877
</div>
874-
{(additionalEmptyRowsCount || tableState.isScrollable) && (
878+
{!isFirefox && (additionalEmptyRowsCount || tableState.isScrollable) && (
875879
<VerticalScrollbar
876880
tableBodyHeight={tableBodyHeight}
877881
internalRowHeight={internalHeaderRowHeight}

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

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
import { isChrome as isChromeFn } from '@ui5/webcomponents-react-base/Device';
2+
import { useSyncRef } from '@ui5/webcomponents-react-base/internal/hooks';
13
import { clsx } from 'clsx';
24
import type { MutableRefObject } from 'react';
3-
import { forwardRef } from 'react';
5+
import { forwardRef, useEffect, useRef } from 'react';
46
import { FlexBoxDirection } from '../../../enums/FlexBoxDirection.js';
57
import { FlexBox } from '../../FlexBox/index.js';
68
import type { ClassNames } from '../types/index.js';
@@ -13,10 +15,35 @@ interface VerticalScrollbarProps {
1315
classNames: ClassNames;
1416
}
1517

18+
const isChrome = isChromeFn();
19+
1620
export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarProps>((props, ref) => {
1721
const { internalRowHeight, tableRef, tableBodyHeight, scrollContainerRef, classNames } = props;
1822
const hasHorizontalScrollbar = tableRef?.current?.offsetWidth !== tableRef?.current?.scrollWidth;
1923
const horizontalScrollbarSectionStyles = clsx(hasHorizontalScrollbar && classNames.bottomSection);
24+
const [componentRef, scrollbarRef] = useSyncRef<HTMLDivElement>(ref);
25+
const contentRef = useRef<HTMLDivElement>(null);
26+
27+
// Force style recalculation to fix Chrome scrollbar-color bug (track height not updating correctly)
28+
useEffect(() => {
29+
if (!isChrome) {
30+
return;
31+
}
32+
33+
if (scrollbarRef.current && contentRef.current) {
34+
const scrollbarElement = scrollbarRef.current;
35+
36+
const forceScrollbarUpdate = () => {
37+
const originalHeight = scrollbarElement.style.height;
38+
scrollbarElement.style.height = `${tableBodyHeight + 1}px`;
39+
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
40+
scrollbarElement.offsetHeight; // Force reflow
41+
scrollbarElement.style.height = originalHeight ?? `${tableBodyHeight}px`;
42+
};
43+
44+
requestAnimationFrame(forceScrollbarUpdate);
45+
}
46+
}, [tableBodyHeight, scrollContainerRef.current?.scrollHeight, scrollbarRef]);
2047

2148
return (
2249
<FlexBox
@@ -31,7 +58,7 @@ export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarPro
3158
className={classNames.headerSection}
3259
/>
3360
<div
34-
ref={ref}
61+
ref={componentRef}
3562
style={{
3663
height: tableRef.current ? `${tableBodyHeight}px` : '0',
3764
}}
@@ -40,10 +67,13 @@ export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarPro
4067
tabIndex={-1}
4168
>
4269
<div
43-
className={classNames.verticalScroller}
44-
style={{
45-
height: `${scrollContainerRef.current?.scrollHeight}px`,
70+
ref={(node) => {
71+
contentRef.current = node;
72+
if (node && scrollContainerRef.current?.scrollHeight) {
73+
node.style.height = `${scrollContainerRef.current?.scrollHeight}px`;
74+
}
4675
}}
76+
className={classNames.verticalScroller}
4777
/>
4878
</div>
4979
<div className={horizontalScrollbarSectionStyles} />

0 commit comments

Comments
 (0)