Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4304,6 +4304,66 @@ describe('AnalyticalTable', () => {
cy.focused().should('have.text', 'Before');
});

it('vertical scroll sync', () => {
cy.mount(<AnalyticalTable columns={columns} data={generateMoreData(100)} />);

cy.get('[data-component-name="AnalyticalTableBody"]').scrollTo(0, 2000).should('have.prop', 'scrollTop', 2000);
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 2000);

cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]')
.scrollTo(0, 3000)
.should('have.prop', 'scrollTop', 3000);
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 3000);

cy.get('[data-component-name="AnalyticalTableContainerWithScrollbar"]').realMouseWheel({ deltaY: 500 });
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 3500);
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 3500);

cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').realMouseWheel({ deltaY: -1000 });
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 2500);
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 2500);

const TestComp = () => {
const [_data, setData] = useState([]);
useEffect(() => {
setTimeout(() => {
setData(generateMoreData(100));
}, 100);
}, []);

return (
<>
<div style={{ height: '500px' }}>
<AnalyticalTable
columns={columns}
data={_data}
header={<div>Header</div>}
visibleRowCountMode="AutoWithEmptyRows"
/>
</div>
</>
);
};

cy.mount(<TestComp />);

cy.get('[data-component-name="AnalyticalTableBody"]').scrollTo(0, 2000).should('have.prop', 'scrollTop', 2000);
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 2000);

cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]')
.scrollTo(0, 3000)
.should('have.prop', 'scrollTop', 3000);
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 3000);

cy.get('[data-component-name="AnalyticalTableContainerWithScrollbar"]').realMouseWheel({ deltaY: 500 });
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 3500);
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 3500);

cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').realMouseWheel({ deltaY: -1000 });
cy.get('[data-component-name="AnalyticalTableVerticalScrollbar"]').should('have.prop', 'scrollTop', 2500);
cy.get('[data-component-name="AnalyticalTableBody"]').should('have.prop', 'scrollTop', 2500);
});

cypressPassThroughTestsFactory(AnalyticalTable, { data, columns });
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -607,3 +607,17 @@
box-sizing: border-box;
border-inline-end: var(--_ui5wcr-AnalyticalTable-OuterBorderInline);
}

/* ==========================================================================
Firefox scrollbar styles
========================================================================== */

.firefoxCell {
&:last-child {
padding-inline-end: 18px;
}
}

.firefoxNativeScrollbar {
scrollbar-width: inherit;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { enrichEventWithDetails } from '@ui5/webcomponents-react-base';
import { clsx } from 'clsx';
import type { MutableRefObject } from 'react';
import { useCallback, useEffect, useRef, useState } from 'react';
import type { AnalyticalTablePropTypes, TableInstance } from '../types/index.js';
Expand All @@ -20,6 +21,7 @@ interface VirtualTableBodyContainerProps {
rowCollapsedFlag?: boolean;
dispatch: (e: { type: string; payload?: any }) => void;
isGrouped: boolean;
isFirefox: boolean;
}

export const VirtualTableBodyContainer = (props: VirtualTableBodyContainerProps) => {
Expand All @@ -39,6 +41,7 @@ export const VirtualTableBodyContainer = (props: VirtualTableBodyContainerProps)
popInRowHeight,
rowCollapsedFlag,
isGrouped,
isFirefox,
dispatch,
} = props;
const [isMounted, setIsMounted] = useState(false);
Expand Down Expand Up @@ -114,7 +117,7 @@ export const VirtualTableBodyContainer = (props: VirtualTableBodyContainerProps)

return (
<div
className={classes.tbody}
className={clsx(classes.tbody, isFirefox && classes.firefoxNativeScrollbar)}
ref={parentRef}
onScroll={onScroll}
style={{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// When reused, move to base pkg
import { isFirefox as isFirefoxFn } from '@ui5/webcomponents-react-base/Device';
import { useEffect, useState } from 'react';

/**
* SSR ready `isFirefox` check.
*/
export function useIsFirefox() {
const [isFirefox, setIsFirefox] = useState(false);

useEffect(() => {
setIsFirefox(isFirefoxFn());
}, []);

return isFirefox;
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { clsx } from 'clsx';
import type { CSSProperties } from 'react';
import { AnalyticalTableSelectionBehavior } from '../../../enums/AnalyticalTableSelectionBehavior.js';
import { AnalyticalTableSelectionMode } from '../../../enums/AnalyticalTableSelectionMode.js';
Expand Down Expand Up @@ -85,14 +86,10 @@ const getRowProps = (
};

const getCellProps = (cellProps, { cell: { column }, instance }) => {
const { classes } = instance.webComponentsReactProperties;
const { webComponentsReactProperties, state } = instance;
const { classes, isFirefox } = webComponentsReactProperties;
const style: CSSProperties = { width: `${column.totalWidth}px`, ...resolveCellAlignment(column) };

let className = classes.tableCell;
if (column.className) {
className += ` ${column.className}`;
}

if (
column.id === '__ui5wcr__internal_highlight_column' ||
column.id === '__ui5wcr__internal_selection_column' ||
Expand All @@ -104,7 +101,12 @@ const getCellProps = (cellProps, { cell: { column }, instance }) => {
return [
cellProps,
{
className,
className: clsx(
cellProps.className,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

className from cell props was not added previously, could that cause issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn’t change how things work right now, since plugin hooks run in the order they’re added. I added it mainly for consistency - if we ever add another hook before useStyling that also uses className, its classes would otherwise get overwritten.

classes.tableCell,
column.className,
isFirefox && state.isScrollable && classes.firefoxCell,
),
style,
tabIndex: -1,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import type { MutableRefObject } from 'react';
import { useEffect, useRef, useState } from 'react';

export function useSyncScroll(refContent: MutableRefObject<HTMLElement>, refScrollbar: MutableRefObject<HTMLElement>) {
const ticking = useRef(false);
export function useSyncScroll(
refContent: MutableRefObject<HTMLElement>,
refScrollbar: MutableRefObject<HTMLElement>,
isScrollable: boolean,
disabled = false,
) {
const isProgrammatic = useRef(false);
const [isMounted, setIsMounted] = useState(false);

useEffect(() => {
if (disabled || !isScrollable) {
return;
}

const content = refContent.current;
const scrollbar = refScrollbar.current;

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

const sync = (source: 'content' | 'scrollbar') => {
if (ticking.current) {
return;
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 = 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');
Expand All @@ -48,5 +47,5 @@ export function useSyncScroll(refContent: MutableRefObject<HTMLElement>, refScro
content.removeEventListener('scroll', onScrollContent);
scrollbar.removeEventListener('scroll', onScrollScrollbar);
};
}, [isMounted, refContent, refScrollbar]);
}, [isMounted, refContent, refScrollbar, disabled, isScrollable]);
}
8 changes: 6 additions & 2 deletions packages/main/src/components/AnalyticalTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import { useColumnsDeps } from './hooks/useColumnsDeps.js';
import { useColumnDragAndDrop } from './hooks/useDragAndDrop.js';
import { useDynamicColumnWidths } from './hooks/useDynamicColumnWidths.js';
import { useFontsReady } from './hooks/useFontsReady.js';
import { useIsFirefox } from './hooks/useIsFirefox.js';
import { useKeyboardNavigation } from './hooks/useKeyboardNavigation.js';
import { usePopIn } from './hooks/usePopIn.js';
import { useResizeColumnsConfig } from './hooks/useResizeColumnsConfig.js';
Expand Down Expand Up @@ -190,6 +191,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
useStylesheet(styleData, AnalyticalTable.displayName);
const isInitialized = useRef(false);
const fontsReady = useFontsReady();
const isFirefox = useIsFirefox();

const alwaysShowSubComponent =
subComponentsBehavior === AnalyticalTableSubComponentsBehavior.Visible ||
Expand Down Expand Up @@ -256,6 +258,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
classes: classNames,
fontsReady,
highlightField,
isFirefox,
isTreeTable,
loading,
markNavigatedRow,
Expand Down Expand Up @@ -658,7 +661,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
}
}, [tableState.columnResizing, retainColumnWidth, tableState.tableColResized]);

useSyncScroll(parentRef, verticalScrollBarRef);
useSyncScroll(parentRef, verticalScrollBarRef, tableState.isScrollable, isFirefox);

useEffect(() => {
columnVirtualizer.measure();
Expand Down Expand Up @@ -844,6 +847,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
handleExternalScroll={onTableScroll}
visibleRows={internalVisibleRowCount}
isGrouped={isGrouped}
isFirefox={isFirefox}
>
<VirtualTableBody
scrollContainerRef={scrollContainerRef}
Expand Down Expand Up @@ -871,7 +875,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
</VirtualTableBodyContainer>
)}
</div>
{(additionalEmptyRowsCount || tableState.isScrollable) && (
{!isFirefox && (additionalEmptyRowsCount || tableState.isScrollable) && (
<VerticalScrollbar
tableBodyHeight={tableBodyHeight}
internalRowHeight={internalHeaderRowHeight}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { isChrome as isChromeFn } from '@ui5/webcomponents-react-base/Device';
import { useSyncRef } from '@ui5/webcomponents-react-base/internal/hooks';
import { clsx } from 'clsx';
import type { MutableRefObject } from 'react';
import { forwardRef } from 'react';
import { forwardRef, useEffect, useRef } from 'react';
import { FlexBoxDirection } from '../../../enums/FlexBoxDirection.js';
import { FlexBox } from '../../FlexBox/index.js';
import type { ClassNames } from '../types/index.js';
Expand All @@ -13,10 +15,35 @@ interface VerticalScrollbarProps {
classNames: ClassNames;
}

const isChrome = isChromeFn();

export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarProps>((props, ref) => {
const { internalRowHeight, tableRef, tableBodyHeight, scrollContainerRef, classNames } = props;
const hasHorizontalScrollbar = tableRef?.current?.offsetWidth !== tableRef?.current?.scrollWidth;
const horizontalScrollbarSectionStyles = clsx(hasHorizontalScrollbar && classNames.bottomSection);
const [componentRef, scrollbarRef] = useSyncRef<HTMLDivElement>(ref);
const contentRef = useRef<HTMLDivElement>(null);

// Force style recalculation to fix Chrome scrollbar-color bug (track height not updating correctly)
useEffect(() => {
if (!isChrome) {
return;
}

if (scrollbarRef.current && contentRef.current) {
const scrollbarElement = scrollbarRef.current;

const forceScrollbarUpdate = () => {
const originalHeight = scrollbarElement.style.height;
scrollbarElement.style.height = `${tableBodyHeight + 1}px`;
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
scrollbarElement.offsetHeight; // Force reflow
scrollbarElement.style.height = originalHeight ?? `${tableBodyHeight}px`;
};

requestAnimationFrame(forceScrollbarUpdate);
}
}, [tableBodyHeight, scrollContainerRef.current?.scrollHeight, scrollbarRef]);

return (
<FlexBox
Expand All @@ -31,7 +58,7 @@ export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarPro
className={classNames.headerSection}
/>
<div
ref={ref}
ref={componentRef}
style={{
height: tableRef.current ? `${tableBodyHeight}px` : '0',
}}
Expand All @@ -40,10 +67,13 @@ export const VerticalScrollbar = forwardRef<HTMLDivElement, VerticalScrollbarPro
tabIndex={-1}
>
<div
className={classNames.verticalScroller}
style={{
height: `${scrollContainerRef.current?.scrollHeight}px`,
ref={(node) => {
contentRef.current = node;
if (node && scrollContainerRef.current?.scrollHeight) {
node.style.height = `${scrollContainerRef.current?.scrollHeight}px`;
}
}}
className={classNames.verticalScroller}
/>
</div>
<div className={horizontalScrollbarSectionStyles} />
Expand Down