Skip to content

Commit 05d4681

Browse files
authored
fix: over bound scrollTo limitation (#233)
* refactor: use one more time * test: update test
1 parent 1a347f1 commit 05d4681

File tree

2 files changed

+27
-9
lines changed

2 files changed

+27
-9
lines changed

src/hooks/useScrollTo.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export default function useScrollTo<T>(
4545
offset: number;
4646
originAlign: ScrollAlign;
4747
targetAlign?: 'top' | 'bottom';
48+
lastTop?: number;
4849
}>(null);
4950

5051
// ========================== Sync Scroll ==========================
@@ -63,6 +64,7 @@ export default function useScrollTo<T>(
6364
const height = containerRef.current.clientHeight;
6465
let needCollectHeight = false;
6566
let newTargetAlign: 'top' | 'bottom' | null = targetAlign;
67+
let targetTop: number | null = null;
6668

6769
// Go to next frame if height not exist
6870
if (height) {
@@ -73,7 +75,7 @@ export default function useScrollTo<T>(
7375
let itemTop = 0;
7476
let itemBottom = 0;
7577

76-
const maxLen = Math.min(data.length, index);
78+
const maxLen = Math.min(data.length - 1, index);
7779

7880
for (let i = 0; i <= maxLen; i += 1) {
7981
const key = getKey(data[i]);
@@ -102,9 +104,6 @@ export default function useScrollTo<T>(
102104
}
103105

104106
// Scroll to
105-
let targetTop: number | null = null;
106-
let inView = false;
107-
108107
switch (mergedAlign) {
109108
case 'top':
110109
targetTop = itemTop - offset;
@@ -120,16 +119,16 @@ export default function useScrollTo<T>(
120119
newTargetAlign = 'top';
121120
} else if (itemBottom > scrollBottom) {
122121
newTargetAlign = 'bottom';
123-
} else {
124-
// No need to collect since already in view
125-
inView = true;
126122
}
127123
}
128124
}
129125

130126
if (targetTop !== null) {
131127
syncScrollTop(targetTop);
132-
} else if (!inView) {
128+
}
129+
130+
// One more time for sync
131+
if (targetTop !== syncState.lastTop) {
133132
needCollectHeight = true;
134133
}
135134
}
@@ -140,12 +139,13 @@ export default function useScrollTo<T>(
140139
...ori,
141140
times: ori.times + 1,
142141
targetAlign: newTargetAlign,
142+
lastTop: targetTop,
143143
}));
144144
}
145145
} else if (process.env.NODE_ENV !== 'production' && syncState?.times === MAX_TIMES) {
146146
warning(
147147
false,
148-
'Seems `scrollTo` with `rc-virtual-list` reach toe max limitation. Please fire issue for us. Thanks.',
148+
'Seems `scrollTo` with `rc-virtual-list` reach the max limitation. Please fire issue for us. Thanks.',
149149
);
150150
}
151151
}, [syncState, containerRef.current]);

tests/scroll.test.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { mount } from 'enzyme';
44
import { spyElementPrototypes } from './utils/domHook';
55
import List from '../src';
66
import { createEvent, fireEvent, render } from '@testing-library/react';
7+
import { resetWarned } from 'rc-util/lib/warning';
78

89
function genData(count) {
910
return new Array(count).fill(null).map((_, index) => ({ id: String(index) }));
@@ -24,6 +25,9 @@ describe('List.Scroll', () => {
2425
width: 100,
2526
height: 100,
2627
}),
28+
offsetParent: {
29+
get: () => document.body,
30+
},
2731
});
2832
});
2933

@@ -137,6 +141,20 @@ describe('List.Scroll', () => {
137141
scrollTo({ index: 30 });
138142
expect(container.querySelector('ul').scrollTop).toEqual(600);
139143
});
144+
145+
it('exceed should not warning', () => {
146+
resetWarned();
147+
const errSpy = jest.spyOn(console, 'error');
148+
149+
const { scrollTo } = presetList();
150+
scrollTo({ index: 9999999999, align: 'top' });
151+
152+
errSpy.mock.calls.forEach((msgs) => {
153+
expect(msgs[0]).not.toContain('max limitation');
154+
});
155+
156+
errSpy.mockRestore();
157+
});
140158
});
141159

142160
it('inject wheel', () => {

0 commit comments

Comments
 (0)