Skip to content
Open
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
77 changes: 72 additions & 5 deletions src/ScrollBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,25 @@ export interface ScrollBarRef {
delayHidden: () => void;
}

function getScrollOffsetByThumbTop(
thumbTop: number,
enabledScrollRange: number,
enabledOffsetRange: number,
) {
if (enabledScrollRange <= 0 || enabledOffsetRange <= 0) {
return 0;
}
Comment on lines +33 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

getScrollOffsetByThumbTop 中,如果 enabledScrollRangeenabledOffsetRangeNaN 或非有限数值(例如在组件初始挂载、尺寸尚未测量完成时),enabledScrollRange <= 0 的判断会返回 false,从而导致后续计算产生 NaN 并传递给 onScroll

建议使用 Number.isFinite 进行更严谨的防御性检查,确保计算结果的安全性。

Suggested change
if (enabledScrollRange <= 0 || enabledOffsetRange <= 0) {
return 0;
}
if (
!Number.isFinite(enabledScrollRange) ||
!Number.isFinite(enabledOffsetRange) ||
enabledScrollRange <= 0 ||
enabledOffsetRange <= 0
) {
return 0;
}


const mergedThumbTop = Math.max(Math.min(thumbTop, enabledOffsetRange), 0);
const ptg: number = mergedThumbTop / enabledOffsetRange;

let nextScrollOffset = Math.ceil(ptg * enabledScrollRange);
nextScrollOffset = Math.max(nextScrollOffset, 0);
nextScrollOffset = Math.min(nextScrollOffset, enabledScrollRange);

return nextScrollOffset;
}

const ScrollBar = React.forwardRef<ScrollBarRef, ScrollBarProps>((props, ref) => {
const {
prefixCls,
Expand Down Expand Up @@ -79,9 +98,57 @@ const ScrollBar = React.forwardRef<ScrollBarRef, ScrollBarProps>((props, ref) =>
}, [scrollOffset, enableScrollRange, enableOffsetRange]);

// ====================== Container =======================
const isThumbTarget = (target: EventTarget | null) => {
return !!target && thumbRef.current?.contains(target as Node);
};

const scrollToTrackPosition = (e: React.MouseEvent | MouseEvent) => {
const scrollbarEle = scrollbarRef.current;

if (!scrollbarEle) {
return;
}

const rect = scrollbarEle.getBoundingClientRect();
const pagePosition = getPageXY(e, horizontal);
let nextTop: number;

if (!Number.isFinite(pagePosition)) {
return;
}

if (horizontal) {
const horizontalStart = isLTR ? rect.left : rect.right;

if (!Number.isFinite(horizontalStart)) {
return;
}

nextTop =
(isLTR ? pagePosition - horizontalStart : horizontalStart - pagePosition) - spinSize / 2;
} else {
if (!Number.isFinite(rect.top)) {
return;
}

nextTop = pagePosition - rect.top - spinSize / 2;
}

onScroll(
getScrollOffsetByThumbTop(nextTop, enableScrollRange, enableOffsetRange),
horizontal,
);
};

const onContainerMouseDown: React.MouseEventHandler = (e) => {
e.stopPropagation();
e.preventDefault();

if (e.button !== 0 || isThumbTarget(e.target)) {
return;
}

scrollToTrackPosition(e);
};

// ======================== Thumb =========================
Expand Down Expand Up @@ -153,11 +220,11 @@ const ScrollBar = React.forwardRef<ScrollBarRef, ScrollBarProps>((props, ref) =>
const tmpEnableScrollRange = enableScrollRangeRef.current;
const tmpEnableOffsetRange = enableOffsetRangeRef.current;

const ptg: number = tmpEnableOffsetRange ? newTop / tmpEnableOffsetRange : 0;

let newScrollTop = Math.ceil(ptg * tmpEnableScrollRange);
newScrollTop = Math.max(newScrollTop, 0);
newScrollTop = Math.min(newScrollTop, tmpEnableScrollRange);
const newScrollTop = getScrollOffsetByThumbTop(
newTop,
tmpEnableScrollRange,
tmpEnableOffsetRange,
);

moveRafId = raf(() => {
onScroll(newScrollTop, horizontal);
Expand Down
17 changes: 17 additions & 0 deletions tests/scroll.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,23 @@ describe('List.Scroll', () => {
expect(container.querySelector('ul').scrollTop > 0).toBeTruthy();
});

it('click track to scroll', () => {
const { container } = genList({
itemHeight: 20,
height: 100,
data: genData(100),
});

act(() => {
const scrollbar = container.querySelector('.rc-virtual-list-scrollbar-vertical');
const mouseDownEvent = createEvent.mouseDown(scrollbar);
Object.defineProperty(mouseDownEvent, 'pageY', { value: 50 });
fireEvent(scrollbar, mouseDownEvent);
});

expect(container.querySelector('ul').scrollTop).toEqual(950);
});
Comment on lines +311 to +326

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

虽然新增了竖向滚动条轨道点击的测试,但水平滚动条(尤其是涉及 RTL 和 LTR 逻辑的分支)也包含了较为复杂的坐标计算。

建议补充水平滚动条在 LTR 和 RTL 模式下轨道点击的测试用例,以确保其正确性并防止后续重构引入回归问题。

    it('click track to scroll', () => {
      const { container } = genList({
        itemHeight: 20,
        height: 100,
        data: genData(100),
      });

      act(() => {
        const scrollbar = container.querySelector('.rc-virtual-list-scrollbar-vertical');
        const mouseDownEvent = createEvent.mouseDown(scrollbar);
        Object.defineProperty(mouseDownEvent, 'pageY', { value: 50 });
        fireEvent(scrollbar, mouseDownEvent);
      });

      expect(container.querySelector('ul').scrollTop).toEqual(950);
    });

    it('click horizontal track to scroll', () => {
      const { container } = genList({
        itemHeight: 20,
        height: 100,
        data: genData(100),
        scrollWidth: 1000,
      });

      act(() => {
        const scrollbar = container.querySelector('.rc-virtual-list-scrollbar-horizontal');
        const mouseDownEvent = createEvent.mouseDown(scrollbar);
        Object.defineProperty(mouseDownEvent, 'pageX', { value: 50 });
        fireEvent(scrollbar, mouseDownEvent);
      });

      const horizontalScrollbar = container.querySelector('.rc-virtual-list-scrollbar-horizontal');
      expect(Number(horizontalScrollbar.parentElement.getAttribute('data-dev-offset'))).toEqual(450);
    });

    it('click horizontal track to scroll in RTL', () => {
      const { container } = genList({
        itemHeight: 20,
        height: 100,
        data: genData(100),
        scrollWidth: 1000,
        direction: 'rtl',
      });

      act(() => {
        const scrollbar = container.querySelector('.rc-virtual-list-scrollbar-horizontal');
        const mouseDownEvent = createEvent.mouseDown(scrollbar);
        Object.defineProperty(mouseDownEvent, 'pageX', { value: 50 });
        fireEvent(scrollbar, mouseDownEvent);
      });

      const horizontalScrollbar = container.querySelector('.rc-virtual-list-scrollbar-horizontal');
      expect(Number(horizontalScrollbar.parentElement.getAttribute('data-dev-offset'))).toEqual(450);
    });


it('should show scrollbar when element has showScrollBar prop set to true', () => {
jest.useFakeTimers();
const listRef = React.createRef();
Expand Down