Skip to content

Commit f866014

Browse files
authored
Fix ListView arrow navigation scrolling (#2560)
1 parent 2e8faba commit f866014

File tree

2 files changed

+70
-2
lines changed

2 files changed

+70
-2
lines changed

packages/@react-spectrum/list/test/ListView.test.js

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ describe('ListView', function () {
232232
expect(document.activeElement).toBe(start);
233233
});
234234

235-
it('should mov focus to above row', function () {
235+
it('should move focus to above row', function () {
236236
let tree = renderListWithFocusables();
237237
let start = getCell(tree, 'Bar');
238238
let end = getCell(tree, 'Foo');
@@ -372,4 +372,71 @@ describe('ListView', function () {
372372
});
373373
});
374374
});
375+
376+
describe('scrolling', function () {
377+
beforeAll(() => {
378+
jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get')
379+
.mockImplementation(function () {
380+
return 40;
381+
});
382+
});
383+
384+
let moveFocus = (key, opts = {}) => {
385+
fireEvent.keyDown(document.activeElement, {key, ...opts});
386+
fireEvent.keyUp(document.activeElement, {key, ...opts});
387+
};
388+
389+
it('should scroll to a cell when it is focused', function () {
390+
let onSelectionChange = jest.fn();
391+
392+
let tree = render(
393+
<ListView
394+
width="250px"
395+
height="60px"
396+
aria-label="List"
397+
data-testid="test"
398+
selectionStyle="highlight"
399+
selectionMode="multiple"
400+
onSelectionChange={onSelectionChange}
401+
items={[...Array(20).keys()].map(k => ({key: k, name: `Item ${k}`}))}>
402+
{item => <Item>{item.name}</Item>}
403+
</ListView>
404+
);
405+
let grid = tree.getByRole('grid');
406+
Object.defineProperty(grid, 'clientHeight', {
407+
get() {
408+
return 60;
409+
}
410+
});
411+
// fire resize so the new clientHeight is requested
412+
act(() => {
413+
fireEvent(window, new Event('resize'));
414+
});
415+
userEvent.tab();
416+
expect(grid.scrollTop).toBe(0);
417+
418+
let rows = tree.getAllByRole('row');
419+
let rowWrappers = rows.map(item => item.parentElement);
420+
421+
expect(rowWrappers[0].style.top).toBe('0px');
422+
expect(rowWrappers[0].style.height).toBe('40px');
423+
expect(rowWrappers[1].style.top).toBe('40px');
424+
expect(rowWrappers[1].style.height).toBe('40px');
425+
expect(rowWrappers[2].style.top).toBe('80px');
426+
expect(rowWrappers[2].style.height).toBe('40px');
427+
428+
// scroll us down far enough that item 0 isn't in the view
429+
moveFocus('ArrowDown');
430+
moveFocus('ArrowDown');
431+
moveFocus('ArrowDown');
432+
expect(document.activeElement).toBe(getCell(tree, 'Item 3'));
433+
expect(grid.scrollTop).toBe(100);
434+
435+
moveFocus('ArrowUp');
436+
moveFocus('ArrowUp');
437+
moveFocus('ArrowUp');
438+
expect(document.activeElement).toBe(getCell(tree, 'Item 0'));
439+
expect(grid.scrollTop).toBe(0);
440+
});
441+
});
375442
});

packages/@react-stately/virtualizer/src/Virtualizer.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -935,7 +935,8 @@ export class Virtualizer<T extends object, V, W> {
935935
* @param duration The duration of the scroll animation.
936936
*/
937937
scrollToItem(key: Key, options?: ScrollToItemOptions) {
938-
if (!key) {
938+
// key can be 0, so check if null or undefined
939+
if (key == null) {
939940
return;
940941
}
941942

0 commit comments

Comments
 (0)