Skip to content

Commit

Permalink
fix: compute correct range for every type of scroll
Browse files Browse the repository at this point in the history
  • Loading branch information
@JulienIzz
JulienIzz committed Aug 29, 2024
1 parent 1e59b8d commit 28f73d4
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 45 deletions.
2 changes: 1 addition & 1 deletion ...c/spatial-navigation/components/virtualizedGrid/SpatialNavigationVirtualizedGrid.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ describe('SpatialNavigationVirtualizedGrid', () => {
renderItem={renderItem}
data={createDataArray(19)}
itemHeight={100}
numberOfRenderedRows={5}
numberOfRenderedRows={7}
numberOfRowsVisibleOnScreen={3}
numberOfColumns={3}
testID="test-grid"
Expand Down
67 changes: 42 additions & 25 deletions ...c/spatial-navigation/components/virtualizedList/SpatialNavigationVirtualizedList.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,24 +351,27 @@ describe('SpatialNavigationVirtualizedList', () => {
expectButtonToHaveFocus(component, 'button 3');
expectListToHaveScroll(listElement, 0);

expect(screen.queryByText('button 1')).toBeFalsy();
expect(screen.getByText('button 2')).toBeTruthy();
expect(screen.getByText('button 6')).toBeTruthy();
expect(screen.queryByText('button 7')).toBeFalsy();
expect(screen.getByText('button 1')).toBeTruthy();
expect(screen.getByText('button 5')).toBeTruthy();
expect(screen.queryByText('button 6')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 4');
expectListToHaveScroll(listElement, -100);

expect(screen.queryByText('button 2')).toBeFalsy();
expect(screen.getByText('button 3')).toBeTruthy();
expect(screen.getByText('button 7')).toBeTruthy();
expect(screen.queryByText('button 8')).toBeFalsy();
expect(screen.getByText('button 1')).toBeTruthy();
expect(screen.getByText('button 5')).toBeTruthy();
expect(screen.queryByText('button 6')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 5');
expectListToHaveScroll(listElement, -200);

expect(screen.queryByText('button 1')).toBeFalsy();
expect(screen.getByText('button 2')).toBeTruthy();
expect(screen.getByText('button 6')).toBeTruthy();
expect(screen.queryByText('button 7')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 6');
expectListToHaveScroll(listElement, -300);
Expand Down Expand Up @@ -399,7 +402,7 @@ describe('SpatialNavigationVirtualizedList', () => {
renderItem={renderItem}
data={data}
itemSize={100}
numberOfRenderedItems={5}
numberOfRenderedItems={7}
numberOfItemsVisibleOnScreen={3}
scrollBehavior="jump-on-scroll"
/>
Expand All @@ -420,31 +423,34 @@ describe('SpatialNavigationVirtualizedList', () => {
expectListToHaveScroll(listElement, 0);

expect(screen.getByText('button 1')).toBeTruthy();
expect(screen.getByText('button 5')).toBeTruthy();
expect(screen.queryByText('button 6')).toBeFalsy();
expect(screen.getByText('button 7')).toBeTruthy();
expect(screen.queryByText('button 8')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 3');
expectListToHaveScroll(listElement, 0);

expect(screen.queryByText('button 1')).toBeFalsy();
expect(screen.getByText('button 2')).toBeTruthy();
expect(screen.getByText('button 6')).toBeTruthy();
expect(screen.queryByText('button 7')).toBeFalsy();
expect(screen.getByText('button 1')).toBeTruthy();
expect(screen.getByText('button 7')).toBeTruthy();
expect(screen.queryByText('button 8')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 4');
expectListToHaveScroll(listElement, -300);

expect(screen.queryByText('button 2')).toBeFalsy();
expect(screen.getByText('button 3')).toBeTruthy();
expect(screen.getByText('button 1')).toBeTruthy();
expect(screen.getByText('button 7')).toBeTruthy();
expect(screen.queryByText('button 8')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 5');
expectListToHaveScroll(listElement, -300);

expect(screen.queryByText('button 1')).toBeFalsy();
expect(screen.getByText('button 2')).toBeTruthy();
expect(screen.getByText('button 8')).toBeTruthy();
expect(screen.queryByText('button 9')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 6');
expectListToHaveScroll(listElement, -300);
Expand All @@ -464,6 +470,10 @@ describe('SpatialNavigationVirtualizedList', () => {
testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 10');
expectListToHaveScroll(listElement, -700);

expect(screen.queryByText('button 3')).toBeFalsy();
expect(screen.getByText('button 4')).toBeTruthy();
expect(screen.getByText('button 10')).toBeTruthy();
});

it('handles correctly different item sizes', async () => {
Expand Down Expand Up @@ -552,19 +562,26 @@ describe('SpatialNavigationVirtualizedList', () => {
expectButtonToHaveFocus(component, 'button 3');
expectListToHaveScroll(listElement, -100);

expect(screen.queryByText('button 1')).toBeFalsy();
expect(screen.getByText('button 2')).toBeTruthy();
expect(screen.getByText('button 6')).toBeTruthy();
expect(screen.queryByText('button 7')).toBeFalsy();
expect(screen.getByText('button 1')).toBeTruthy();
expect(screen.getByText('button 5')).toBeTruthy();
expect(screen.queryByText('button 6')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 4');
expectListToHaveScroll(listElement, -300);

expect(screen.queryByText('button 2')).toBeFalsy();
expect(screen.getByText('button 3')).toBeTruthy();
expect(screen.getByText('button 7')).toBeTruthy();
expect(screen.queryByText('button 8')).toBeFalsy();
expect(screen.getByText('button 1')).toBeTruthy();
expect(screen.getByText('button 5')).toBeTruthy();
expect(screen.queryByText('button 6')).toBeFalsy();

testRemoteControlManager.handleRight();
expectButtonToHaveFocus(component, 'button 5');
expectListToHaveScroll(listElement, -400);

expect(screen.queryByText('button 1')).toBeFalsy();
expect(screen.getByText('button 5')).toBeTruthy();
expect(screen.getByText('button 5')).toBeTruthy();
expect(screen.queryByText('button 7')).toBeFalsy();
});

it('jumps to first element on go to first button press', async () => {
Expand Down
1 change: 1 addition & 0 deletions packages/lib/src/spatial-navigation/components/virtualizedList/VirtualizedList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export const VirtualizedList = typedMemo(
currentlyFocusedItemIndex,
numberOfRenderedItems,
numberOfItemsVisibleOnScreen,
scrollBehavior,
});

const vertical = orientation === 'vertical';
Expand Down
43 changes: 27 additions & 16 deletions packages/lib/src/spatial-navigation/components/virtualizedList/helpers/getRange.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { getRange } from './getRange';

const defaultNumberOfRenderedItems = 8;
const defaultNumberOfItemsVisibleOnScreen = 4;
const defaultScrollBehavior = 'stick-to-start';

describe('getRange for custom virtualized list', () => {
beforeAll(() => {
Expand All @@ -12,30 +13,38 @@ describe('getRange for custom virtualized list', () => {
});

it.each`
description | arraySize | focusIndex | result | numberOfRenderedItems | numberOfItemsVisibleOnScreen
${'empty array'} | ${0} | ${0} | ${{ start: 0, end: 0 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'empty array and out of bounds focus'} | ${0} | ${3} | ${{ start: 0, end: 0 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'one element focused'} | ${1} | ${0} | ${{ start: 0, end: 0 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'one element and out of bounds focus'} | ${1} | ${12} | ${{ start: 0, end: 0 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'small array with focus in the middle'} | ${5} | ${3} | ${{ start: 0, end: 4 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'bigger number of rendered items than data with range out of bound'} | ${5} | ${12} | ${{ start: 0, end: 4 }} | ${8} | ${defaultNumberOfItemsVisibleOnScreen}
${'focus at the beginning of big array'} | ${30} | ${0} | ${{ start: 0, end: 7 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'focus at the beginning of big array, before scroll'} | ${30} | ${4} | ${{ start: 2, end: 9 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'focus at the beginning of big array, first scroll'} | ${30} | ${5} | ${{ start: 3, end: 10 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'focus in the middle of big array, must scroll'} | ${30} | ${12} | ${{ start: 10, end: 17 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'focus at the end of big array'} | ${30} | ${29} | ${{ start: 22, end: 29 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'big array and focus out of bounds'} | ${30} | ${31} | ${{ start: 22, end: 29 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen}
${'odd number of rendered items'} | ${30} | ${0} | ${{ start: 0, end: 6 }} | ${7} | ${defaultNumberOfItemsVisibleOnScreen}
${'odd number of item visible on screen'} | ${30} | ${5} | ${{ start: 3, end: 10 }} | ${defaultNumberOfRenderedItems} | ${3}
description | arraySize | focusIndex | result | numberOfRenderedItems | numberOfItemsVisibleOnScreen | scrollBehavior
${'empty array'} | ${0} | ${0} | ${{ start: 0, end: 0 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'empty array and out of bounds focus'} | ${0} | ${3} | ${{ start: 0, end: 0 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'one element focused'} | ${1} | ${0} | ${{ start: 0, end: 0 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'one element and out of bounds focus'} | ${1} | ${12} | ${{ start: 0, end: 0 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'small array with focus in the middle'} | ${5} | ${3} | ${{ start: 0, end: 4 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'bigger number of rendered items than data with range out of bound'} | ${5} | ${12} | ${{ start: 0, end: 4 }} | ${8} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'focus at the beginning of big array'} | ${30} | ${0} | ${{ start: 0, end: 7 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'focus at the beginning of big array, before scroll'} | ${30} | ${4} | ${{ start: 2, end: 9 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'focus at the beginning of big array, first scroll'} | ${30} | ${5} | ${{ start: 3, end: 10 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'focus in the middle of big array, must scroll'} | ${30} | ${12} | ${{ start: 10, end: 17 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'focus at the end of big array'} | ${30} | ${29} | ${{ start: 22, end: 29 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'big array and focus out of bounds'} | ${30} | ${31} | ${{ start: 22, end: 29 }} | ${defaultNumberOfRenderedItems} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'odd number of rendered items'} | ${30} | ${0} | ${{ start: 0, end: 6 }} | ${7} | ${defaultNumberOfItemsVisibleOnScreen} | ${defaultScrollBehavior}
${'odd number of item visible on screen'} | ${30} | ${5} | ${{ start: 3, end: 10 }} | ${defaultNumberOfRenderedItems} | ${3} | ${defaultScrollBehavior}
`(
'should return proper range for array size $arraySize at index $focusIndex (case description: "$description")',
({ arraySize, focusIndex, numberOfRenderedItems, numberOfItemsVisibleOnScreen, result }) => {
({
arraySize,
focusIndex,
numberOfRenderedItems,
numberOfItemsVisibleOnScreen,
scrollBehavior,
result,
}) => {
const input = new Array(arraySize).fill(1);
const expectedResult = getRange({
data: input,
currentlyFocusedItemIndex: focusIndex,
numberOfRenderedItems,
numberOfItemsVisibleOnScreen,
scrollBehavior,
});

expect(expectedResult).toEqual(result);
Expand All @@ -51,6 +60,7 @@ describe('getRange for custom virtualized list', () => {
currentlyFocusedItemIndex: 5,
numberOfRenderedItems: -1,
numberOfItemsVisibleOnScreen: defaultNumberOfItemsVisibleOnScreen,
scrollBehavior: 'stick-to-start',
});

expect(expectedResult).toEqual(result);
Expand All @@ -66,6 +76,7 @@ describe('getRange for custom virtualized list', () => {
currentlyFocusedItemIndex: 5,
numberOfRenderedItems: 6,
numberOfItemsVisibleOnScreen: 8,
scrollBehavior: 'stick-to-start',
}),
).toThrowError();
});
Expand Down
45 changes: 42 additions & 3 deletions packages/lib/src/spatial-navigation/components/virtualizedList/helpers/getRange.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { ScrollBehavior } from '../VirtualizedList';

const positiveValueOrZero = (x: number): number => Math.max(x, 0);

/**
Expand All @@ -20,11 +22,13 @@ const getRangeWithoutFloatHandling = ({
currentlyFocusedItemIndex,
numberOfRenderedItems = 8,
numberOfItemsVisibleOnScreen,
scrollBehavior,
}: {
data: Array<unknown>;
currentlyFocusedItemIndex: number;
numberOfRenderedItems?: number;
numberOfItemsVisibleOnScreen: number;
scrollBehavior: ScrollBehavior;
}) => {
const numberOfItemsNotVisible = numberOfRenderedItems - numberOfItemsVisibleOnScreen;

Expand All @@ -37,12 +41,22 @@ const getRangeWithoutFloatHandling = ({
);
}

if (
scrollBehavior === 'jump-on-scroll' &&
numberOfRenderedItems < 2 * numberOfItemsVisibleOnScreen + 1
) {
throw new Error(
'You have set a numberOfRenderedItems inferior to 2 * numberOfItemsVisibleOnScreen + 1 in your SpatialNavigationVirtualizedList with the jump-on-scroll scroll behavior. You must change it.',
);
}

const lastDataIndex = data.length - 1;

const { rawStartIndex, rawEndIndex } = getRawStartAndEndIndexes({
currentlyFocusedItemIndex,
numberOfItemsVisibleOnScreen,
numberOfItemsNotVisible,
scrollBehavior,
});

/*
Expand All @@ -53,11 +67,13 @@ const getRangeWithoutFloatHandling = ({
*/
if (rawStartIndex < 0) {
const finalEndIndex = numberOfRenderedItems - 1;

return { start: 0, end: positiveValueOrZero(Math.min(finalEndIndex, lastDataIndex)) };
}

if (rawEndIndex > data.length - 1) {
const finalStartIndex = lastDataIndex - numberOfRenderedItems + 1;

return { start: positiveValueOrZero(finalStartIndex), end: positiveValueOrZero(lastDataIndex) };
}

Expand All @@ -68,16 +84,36 @@ const getRawStartAndEndIndexes = ({
currentlyFocusedItemIndex,
numberOfItemsVisibleOnScreen,
numberOfItemsNotVisible,
scrollBehavior,
}: {
currentlyFocusedItemIndex: number;
numberOfItemsVisibleOnScreen: number;
numberOfItemsNotVisible: number;
scrollBehavior: ScrollBehavior;
}) => {
const halfNumberOfItemsNotVisible = numberOfItemsNotVisible / 2;

const rawStartIndex = currentlyFocusedItemIndex - halfNumberOfItemsNotVisible;
const rawEndIndex =
currentlyFocusedItemIndex + numberOfItemsVisibleOnScreen - 1 + halfNumberOfItemsNotVisible;
let rawStartIndex: number;
let rawEndIndex: number;

switch (scrollBehavior) {
case 'stick-to-start':
rawStartIndex = currentlyFocusedItemIndex - halfNumberOfItemsNotVisible;
rawEndIndex =
currentlyFocusedItemIndex + numberOfItemsVisibleOnScreen - 1 + halfNumberOfItemsNotVisible;
break;
case 'stick-to-end':
rawStartIndex =
currentlyFocusedItemIndex - numberOfItemsVisibleOnScreen + 1 - halfNumberOfItemsNotVisible;
rawEndIndex = currentlyFocusedItemIndex + halfNumberOfItemsNotVisible;
break;
case 'jump-on-scroll':
rawStartIndex = currentlyFocusedItemIndex - numberOfItemsVisibleOnScreen;
rawEndIndex = currentlyFocusedItemIndex + numberOfItemsVisibleOnScreen;
break;
default:
throw new Error(`Unknown scroll behavior: ${scrollBehavior}`);
}

return { rawStartIndex, rawEndIndex };
};
Expand All @@ -94,11 +130,13 @@ export const getRange = ({
currentlyFocusedItemIndex,
numberOfRenderedItems = 8,
numberOfItemsVisibleOnScreen,
scrollBehavior,
}: {
data: Array<unknown>;
currentlyFocusedItemIndex: number;
numberOfRenderedItems?: number;
numberOfItemsVisibleOnScreen: number;
scrollBehavior: ScrollBehavior;
}): { start: number; end: number } => {
if (numberOfRenderedItems <= 0) {
console.error(
Expand All @@ -112,6 +150,7 @@ export const getRange = ({
currentlyFocusedItemIndex,
numberOfRenderedItems,
numberOfItemsVisibleOnScreen,
scrollBehavior,
});

return { start: Math.ceil(result.start), end: Math.ceil(result.end) };
Expand Down

0 comments on commit 28f73d4

Please sign in to comment.