Skip to content

Commit

Permalink
fix(lists): range rendered in jump-on-scroll and stick-to-end (#148)
Browse files Browse the repository at this point in the history 
* refactor: improve readability of formula

* refactor: export to a function the computing of start and end indexes of range to render

* fix: compute correct range for every type of scroll

* doc: add doc on nbOfRenderedItems

* refactor: simplify getRawStartAndEndIndexes

* chore: fix test readability

* chore: add TSDoc for getRawIndexes

* chore: remove throw and log error instead to avoid breaking change

* fix: use user input to compute range in jump on scroll
  • Loading branch information
@JulienIzz
JulienIzz committed Sep 9, 2024
1 parent 1bafa52 commit d9509dc
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 31 deletions.
2 changes: 1 addition & 1 deletion docs/api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ It also ensures that the scroll event is propagated properly to parent ScrollVie
| `data` | `Array<T>` | The array of data items to render. ⚠️ You should memoize this array for maximum performance. A costly memo depends on it. |
| `renderItem` | `(args: { item: T }) => JSX.Element` | A function that returns the JSX element to render for each item in the data array. The function receives an object with the item as a parameter. |
| `itemSize` | `number \| ((item: T) => number)` | In case you specify a number it will behave like this : ff vertical, the height of an item; otherwise, the width. You can also specify a function which needs to return for each item of `data` its size in pixel in order for the list to handle various item sizes. ⚠️ You should memoize this function for maximal performances. An important memo depends on it. |
| `numberOfRenderedItems` | `number` | The number of items to be rendered (virtualization size). |
| `numberOfRenderedItems` | `number` | The number of items to be rendered (virtualization size). ⚠️ It must be at least equal to `numberOfItemsVisibleOnScreen +2` or when using jump-on-scroll : `(2 * numberOfItemsVisibleOnScreen) + 1` to ensure correct rendering. |
| `numberOfItemsVisibleOnScreen` | `number` | The number of items visible on the screen. This helps determine how to slice the data and when to stop the scroll at the end of the list. |
| `onEndReached` | `() => void` | An optional callback function that is called when the user reaches the end of the list. Helps with pagination. |
| `onEndReachedThresholdItemsNumber` | `number` | The number of items left to display before triggering the `onEndReached` callback. Defaults to 3. |
Expand Down
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
3 changes: 3 additions & 0 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 @@ -36,6 +36,7 @@ describe('getRange for custom virtualized list', () => {
currentlyFocusedItemIndex: focusIndex,
numberOfRenderedItems,
numberOfItemsVisibleOnScreen,
scrollBehavior: 'stick-to-start',
});

expect(expectedResult).toEqual(result);
Expand All @@ -51,6 +52,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 +68,7 @@ describe('getRange for custom virtualized list', () => {
currentlyFocusedItemIndex: 5,
numberOfRenderedItems: 6,
numberOfItemsVisibleOnScreen: 8,
scrollBehavior: 'stick-to-start',
}),
).toThrowError();
});
Expand Down
75 changes: 71 additions & 4 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,23 @@ const getRangeWithoutFloatHandling = ({
);
}

const halfNumberOfItemsNotVisible = numberOfItemsNotVisible / 2;
if (
scrollBehavior === 'jump-on-scroll' &&
numberOfRenderedItems < 2 * numberOfItemsVisibleOnScreen + 1
) {
console.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 = currentlyFocusedItemIndex - halfNumberOfItemsNotVisible;
const rawEndIndex =
currentlyFocusedItemIndex + halfNumberOfItemsNotVisible - 1 + numberOfItemsVisibleOnScreen;
const { rawStartIndex, rawEndIndex } = getRawStartAndEndIndexes({
currentlyFocusedItemIndex,
numberOfItemsVisibleOnScreen,
numberOfItemsNotVisible,
scrollBehavior,
});

/*
* if sum does not fit the window size, then we are in of these cases:
Expand All @@ -52,17 +67,66 @@ 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) };
}

return { start: rawStartIndex, end: rawEndIndex };
};

/**
* Computes the raw start and end indexes for the virtualization.
* "raw" means that the indexes are subject to be out of bounds
* which will be handled in the getRange function.
*/
const getRawStartAndEndIndexes = ({
currentlyFocusedItemIndex,
numberOfItemsVisibleOnScreen,
numberOfItemsNotVisible,
scrollBehavior,
}: {
currentlyFocusedItemIndex: number;
numberOfItemsVisibleOnScreen: number;
numberOfItemsNotVisible: number;
scrollBehavior: ScrollBehavior;
}) => {
const halfNumberOfItemsNotVisible = numberOfItemsNotVisible / 2;

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

/**
* Computes an array slice for virtualization
* Have a look at the tests to get examples!
Expand All @@ -75,11 +139,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 @@ -93,6 +159,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 d9509dc

Please sign in to comment.