Skip to content
Draft
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
19 changes: 16 additions & 3 deletions src/FixedHolder/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@ import ColGroup from '../ColGroup';
import TableContext from '../context/TableContext';
import type { HeaderProps } from '../Header/Header';
import devRenderTimes from '../hooks/useRenderTimes';
import type { ColumnsType, ColumnType, Direction, TableLayout } from '../interface';
import type {
ColumnsType,
ColumnType,
Direction,
OnCustomizeScroll,
ScrollSource,
TableLayout,
} from '../interface';

function useColumnWidth(colWidths: readonly number[], columCount: number) {
return useMemo(() => {
Expand Down Expand Up @@ -38,7 +45,9 @@ export interface FixedHeaderProps<RecordType> extends HeaderProps<RecordType> {
stickyClassName?: string;
scrollX?: number | string | true;
tableLayout?: TableLayout;
onScroll: (info: { currentTarget: HTMLDivElement; scrollLeft?: number }) => void;
onScroll: OnCustomizeScroll;
onMouseEnter?: React.MouseEventHandler<HTMLDivElement>;
scrollSource: Extract<ScrollSource, 'header' | 'summary'>;
children: (info: HeaderProps<RecordType>) => React.ReactNode;
colGroup?: React.ReactNode;
}
Expand Down Expand Up @@ -66,6 +75,8 @@ const FixedHolder = React.forwardRef<HTMLDivElement, FixedHeaderProps<any>>((pro
scrollX,
tableLayout = 'fixed',
onScroll,
onMouseEnter,
scrollSource,
maxContentScroll,
children,
...restProps
Expand Down Expand Up @@ -109,6 +120,7 @@ const FixedHolder = React.forwardRef<HTMLDivElement, FixedHeaderProps<any>>((pro
onScroll({
currentTarget,
scrollLeft: nextScroll,
source: scrollSource,
});
e.preventDefault();
}
Expand All @@ -120,7 +132,7 @@ const FixedHolder = React.forwardRef<HTMLDivElement, FixedHeaderProps<any>>((pro
return () => {
scrollEle?.removeEventListener('wheel', onWheel);
};
}, []);
}, [direction, onScroll, scrollSource]);

// Add scrollbar column
const lastColumn = flattenColumns[flattenColumns.length - 1];
Expand Down Expand Up @@ -175,6 +187,7 @@ const FixedHolder = React.forwardRef<HTMLDivElement, FixedHeaderProps<any>>((pro
...style,
}}
ref={setScrollRef}
onMouseEnter={onMouseEnter}
className={clsx(className, {
[stickyClassName]: !!stickyClassName,
})}
Expand Down
62 changes: 57 additions & 5 deletions src/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import type {
PanelRender,
Reference,
RowClassName,
ScrollSource,
TableComponents,
TableLayout,
TableSticky,
Expand All @@ -91,6 +92,18 @@ const EMPTY_DATA = [];
// Used for customize scroll
const EMPTY_SCROLL_TARGET = {};

const getScrollSource = (
source: ScrollSource | undefined,
currentTarget: HTMLElement | undefined,
refs: Record<ScrollSource, HTMLElement | null | undefined>,
): ScrollSource | undefined => {
if (source) {
return source;
}

return (Object.keys(refs) as ScrollSource[]).find(key => refs[key] === currentTarget);
};
Comment on lines +95 to +105

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

In getScrollSource, if currentTarget is undefined or null, and any reference in refs is also undefined or null (e.g., refs.sticky is explicitly undefined, or refs.summary is null when no summary is rendered), the comparison refs[key] === currentTarget will evaluate to true.

This will incorrectly resolve the scroll source to that key (e.g., 'sticky' or 'summary'). To prevent this, we should ensure currentTarget is truthy before performing the comparison.

Suggested change
const getScrollSource = (
source: ScrollSource | undefined,
currentTarget: HTMLElement | undefined,
refs: Record<ScrollSource, HTMLElement | null | undefined>,
): ScrollSource | undefined => {
if (source) {
return source;
}
return (Object.keys(refs) as ScrollSource[]).find(key => refs[key] === currentTarget);
};
const getScrollSource = (
source: ScrollSource | undefined,
currentTarget: HTMLElement | undefined,
refs: Record<ScrollSource, HTMLElement | null | undefined>,
): ScrollSource | undefined => {
if (source) {
return source;
}
return (Object.keys(refs) as ScrollSource[]).find(
key => currentTarget && refs[key] === currentTarget,
);
};


export type SemanticName = 'section' | 'title' | 'footer' | 'content';
export type ComponentsSemantic = 'wrapper' | 'cell' | 'row';

Expand Down Expand Up @@ -350,6 +363,13 @@ const Table = <RecordType extends DefaultRecordType>(
const scrollHeaderRef = React.useRef<HTMLDivElement>(null);
const scrollBodyRef = React.useRef<HTMLDivElement>(null);
const scrollBodyContainerRef = React.useRef<HTMLDivElement>(null);
const [hoverEle, setHoverEle] = React.useState<ScrollSource | null>(null);
const onHoverScrollSource = React.useCallback(
(source: ScrollSource) => () => {
setHoverEle(source);
},
[],
);
Comment on lines +366 to +372

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

hoverEle is set to the hovered scroll source on onMouseEnter, but it is never reset to null when the mouse leaves the table container.

If a user hovers the header (setting hoverEle to 'header'), moves the mouse out of the table, and then uses keyboard navigation (e.g., Arrow keys) to scroll the table body, the scroll event will be triggered with source: 'body'. Since hoverEle is still locked to 'header', the condition hoverEle !== scrollSource will be true, causing the scroll sync to be ignored and breaking synchronization.

To fix this, we should reset hoverEle to null when the mouse leaves the table container by listening to the mouseleave event on fullTableRef.current.

  const [hoverEle, setHoverEle] = React.useState<ScrollSource | null>(null);
  const onHoverScrollSource = React.useCallback(
    (source: ScrollSource) => () => {
      setHoverEle(source);
    },
    [],
  );

  React.useEffect(() => {
    const tableEl = fullTableRef.current;
    const handleMouseLeave = () => {
      setHoverEle(null);
    };
    tableEl?.addEventListener('mouseleave', handleMouseLeave);
    return () => {
      tableEl?.removeEventListener('mouseleave', handleMouseLeave);
    };
  }, []);


React.useImperativeHandle(ref, () => {
return {
Expand Down Expand Up @@ -476,12 +496,32 @@ const Table = <RecordType extends DefaultRecordType>(
const [scrollInfo, setScrollInfo] = React.useState<ScrollInfoType>([0, 0]);

const onInternalScroll = useEvent(
({ currentTarget, scrollLeft }: { currentTarget: HTMLElement; scrollLeft?: number }) => {
({
currentTarget,
scrollLeft,
source,
}: {
currentTarget?: HTMLElement;
scrollLeft?: number;
source?: ScrollSource;
}) => {
const scrollSource = getScrollSource(source, currentTarget, {
body: getDOM(scrollBodyRef.current) as HTMLElement,
header: scrollHeaderRef.current,
summary: scrollSummaryRef.current,
sticky: undefined,
});

if (source && hoverEle && scrollSource && hoverEle !== scrollSource) {
return;
}

const mergedScrollLeft =
typeof scrollLeft === 'number' ? scrollLeft : currentTarget.scrollLeft;
typeof scrollLeft === 'number' ? scrollLeft : (currentTarget?.scrollLeft ?? 0);

const compareTarget = currentTarget || EMPTY_SCROLL_TARGET;
if (!getScrollTarget() || getScrollTarget() === compareTarget) {
const isHoverSource = hoverEle && scrollSource && hoverEle === scrollSource;
if (isHoverSource || !getScrollTarget() || getScrollTarget() === compareTarget) {
setScrollTarget(compareTarget);

forceScroll(mergedScrollLeft, scrollHeaderRef.current);
Expand Down Expand Up @@ -517,8 +557,12 @@ const Table = <RecordType extends DefaultRecordType>(
},
);

const onBodyInternalScroll = useEvent((e: React.UIEvent<HTMLDivElement>) => {
onInternalScroll({ currentTarget: e.currentTarget, source: 'body' });
});

const onBodyScroll = useEvent((e: React.UIEvent<HTMLDivElement>) => {
onInternalScroll(e);
onBodyInternalScroll(e);
onScroll?.(e);
});

Expand Down Expand Up @@ -676,6 +720,7 @@ const Table = <RecordType extends DefaultRecordType>(
scrollbarSize,
ref: scrollBodyRef,
onScroll: onInternalScroll,
onMouseEnter: onHoverScrollSource('body'),
});

headerProps.colWidths = flattenColumns.map(({ width }, index) => {
Expand All @@ -701,6 +746,7 @@ const Table = <RecordType extends DefaultRecordType>(
...scrollYStyle,
}}
onScroll={onBodyScroll}
onMouseEnter={onHoverScrollSource('body')}
ref={scrollBodyRef}
className={`${prefixCls}-body`}
>
Expand Down Expand Up @@ -747,6 +793,8 @@ const Table = <RecordType extends DefaultRecordType>(
className={`${prefixCls}-header`}
ref={scrollHeaderRef}
colGroup={bodyColGroup}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

React Doctor · react-doctor/jsx-no-jsx-as-prop (warning)

This child redraws every render because the prop gets brand new JSX each time.

Fix → Move the JSX outside the component or wrap it in useMemo so memoized children do not redraw every render.

Docs

scrollSource="header"
onMouseEnter={onHoverScrollSource('header')}
>
{renderFixedHeaderTable}
</FixedHolder>
Expand All @@ -763,6 +811,8 @@ const Table = <RecordType extends DefaultRecordType>(
className={`${prefixCls}-summary`}
ref={scrollSummaryRef}
colGroup={bodyColGroup}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

React Doctor · react-doctor/jsx-no-jsx-as-prop (warning)

This child redraws every render because the prop gets brand new JSX each time.

Fix → Move the JSX outside the component or wrap it in useMemo so memoized children do not redraw every render.

Docs

scrollSource="summary"
onMouseEnter={onHoverScrollSource('summary')}
>
{renderFixedFooterTable}
</FixedHolder>
Expand All @@ -774,6 +824,7 @@ const Table = <RecordType extends DefaultRecordType>(
offsetScroll={offsetScroll}
scrollBodyRef={scrollBodyRef}
onScroll={onInternalScroll}
onMouseEnter={onHoverScrollSource('sticky')}
container={container}
direction={direction}
/>
Expand All @@ -786,7 +837,8 @@ const Table = <RecordType extends DefaultRecordType>(
<div
style={{ ...scrollXStyle, ...scrollYStyle, ...styles?.content }}
className={clsx(`${prefixCls}-content`, classNames?.content)}
onScroll={onInternalScroll}
onScroll={onBodyInternalScroll}
onMouseEnter={onHoverScrollSource('body')}
ref={scrollBodyRef}
>
<TableComponent
Expand Down
5 changes: 4 additions & 1 deletion src/VirtualTable/BodyGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const ALIGN_MAP: Record<string, 'top' | 'bottom' | 'auto'> = {
export interface GridProps<RecordType = any> {
data: RecordType[];
onScroll: OnCustomizeScroll;
onMouseEnter?: React.MouseEventHandler;
}

export interface GridRef {
Expand All @@ -30,7 +31,7 @@ export interface GridRef {
}

const Grid = React.forwardRef<GridRef, GridProps>((props, ref) => {
const { data, onScroll } = props;
const { data, onScroll, onMouseEnter } = props;

const {
flattenColumns,
Expand Down Expand Up @@ -276,10 +277,12 @@ const Grid = React.forwardRef<GridRef, GridProps>((props, ref) => {
component={wrapperComponent}
scrollWidth={scrollX as number}
direction={direction}
onMouseEnter={onMouseEnter}
onVirtualScroll={({ x }) => {
onScroll({
currentTarget: listRef.current?.nativeElement,
scrollLeft: x,
source: 'body',
});
}}
onScroll={onTablePropScroll}
Expand Down
6 changes: 4 additions & 2 deletions src/VirtualTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import Grid from './BodyGrid';
import { StaticContext } from './context';

const renderBody: CustomizeScrollBody<any> = (rawData, props) => {
const { ref, onScroll } = props;
return <Grid ref={ref as any} data={rawData as any} onScroll={onScroll} />;
const { ref, onScroll, onMouseEnter } = props;
return (
<Grid ref={ref as any} data={rawData as any} onScroll={onScroll} onMouseEnter={onMouseEnter} />
);
};

export interface VirtualTableProps<RecordType> extends Omit<TableProps<RecordType>, 'scroll'> {
Expand Down
12 changes: 6 additions & 6 deletions src/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,7 @@ export type Direction = 'ltr' | 'rtl';
export type SpecialString<T> = T | (string & NonNullable<unknown>);

export type DataIndex<T = any> =
| DeepNamePath<T>
| SpecialString<T>
| number
| (SpecialString<T> | number)[];
DeepNamePath<T> | SpecialString<T> | number | (SpecialString<T> | number)[];

export type CellEllipsisType = { showTitle?: boolean } | boolean;

Expand Down Expand Up @@ -139,8 +136,7 @@ export interface ColumnType<RecordType> extends ColumnSharedType<RecordType> {
}

export type ColumnsType<RecordType = unknown> = readonly (
| ColumnGroupType<RecordType>
| ColumnType<RecordType>
ColumnGroupType<RecordType> | ColumnType<RecordType>
)[];

export type GetRowKey<RecordType> = (record: RecordType, index?: number) => Key;
Expand All @@ -167,9 +163,12 @@ type Component<P> =

export type CustomizeComponent = Component<any>;

export type ScrollSource = 'body' | 'header' | 'summary' | 'sticky';

export type OnCustomizeScroll = (info: {
currentTarget?: HTMLElement;
scrollLeft?: number;
source?: ScrollSource;
}) => void;

export type CustomizeScrollBody<RecordType> = (
Expand All @@ -178,6 +177,7 @@ export type CustomizeScrollBody<RecordType> = (
scrollbarSize: number;
ref: React.Ref<{ scrollLeft: number; scrollTo?: (scrollConfig: ScrollConfig) => void }>;
onScroll: OnCustomizeScroll;
onMouseEnter?: React.MouseEventHandler;
},
) => React.ReactNode;

Expand Down
8 changes: 6 additions & 2 deletions src/stickyScrollBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { getDOM, getScrollBarSize, raf } from '@rc-component/util';
import * as React from 'react';
import TableContext from './context/TableContext';
import { useLayoutState } from './hooks/useFrame';
import type { OnCustomizeScroll } from './interface';
import { getOffset } from './utils/offsetUtil';

const MOUSEUP_EVENT: keyof WindowEventMap = 'mouseup';
Expand All @@ -13,7 +14,8 @@ const RESIZE_EVENT: keyof WindowEventMap = 'resize';

interface StickyScrollBarProps {
scrollBodyRef: React.RefObject<HTMLDivElement>;
onScroll: (params: { scrollLeft?: number }) => void;
onScroll: OnCustomizeScroll;
onMouseEnter?: React.MouseEventHandler<HTMLDivElement>;
offsetScroll: number;
container: HTMLElement | Window;
direction: string;
Expand All @@ -23,7 +25,7 @@ const StickyScrollBar: React.ForwardRefRenderFunction<unknown, StickyScrollBarPr
props,
ref,
) => {
const { scrollBodyRef, onScroll, offsetScroll, container, direction } = props;
const { scrollBodyRef, onScroll, onMouseEnter, offsetScroll, container, direction } = props;
const prefixCls = useContext(TableContext, 'prefixCls');
const bodyScrollWidth = scrollBodyRef.current?.scrollWidth || 0;
const bodyWidth = scrollBodyRef.current?.clientWidth || 0;
Expand Down Expand Up @@ -80,6 +82,7 @@ const StickyScrollBar: React.ForwardRefRenderFunction<unknown, StickyScrollBarPr
if (shouldScroll) {
onScroll({
scrollLeft: (left / bodyWidth) * (bodyScrollWidth + 2),
source: 'sticky',
});
refState.current.x = event.pageX;
}
Expand Down Expand Up @@ -190,6 +193,7 @@ const StickyScrollBar: React.ForwardRefRenderFunction<unknown, StickyScrollBarPr
<div
style={{ height: getScrollBarSize(), width: bodyWidth, bottom: offsetScroll }}
className={`${prefixCls}-sticky-scroll`}
onMouseEnter={onMouseEnter}
>
<div

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

React Doctor · react-doctor/no-static-element-interactions (warning)

Screen reader users can't tell this click handler is interactive because it has no role, so add a role or use a button or link.

Fix → Give clickable static elements a role, or use a button or link.

Docs

onMouseDown={onMouseDown}
Expand Down
36 changes: 36 additions & 0 deletions tests/Virtual.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,42 @@ describe('Table.Virtual', () => {
expect(scrollLeftCalled).toBeTruthy();
});

it('should skip scroll from non-hovered source', () => {
const { container } = getTable();

resize(container.querySelector('.rc-table')!);

fireEvent.mouseEnter(container.querySelector('.rc-table-header')!);

setScrollLeft.mockClear();

fireEvent.wheel(container.querySelector('.rc-table-tbody-virtual-holder')!, {
deltaX: 10,
});

expect(setScrollLeft).not.toHaveBeenCalled();
});

it('should use hovered body as scroll source even when header is locked', () => {
const { container } = getTable();

resize(container.querySelector('.rc-table')!);

fireEvent.mouseEnter(container.querySelector('.rc-table-header')!);
fireEvent.wheel(container.querySelector('.rc-table-header')!, {
deltaX: 10,
});

setScrollLeft.mockClear();

fireEvent.mouseEnter(container.querySelector('.rc-table-tbody-virtual')!);
fireEvent.wheel(container.querySelector('.rc-table-tbody-virtual-holder')!, {
deltaX: 10,
});

expect(setScrollLeft).toHaveBeenCalled();
});

it('should not reset scroll when data changed', async () => {
const { container, rerender } = getTable();

Expand Down
Loading