Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: range rendered in jump-on-scroll and stick-to-end #148

Merged
merged 9 commits into from
Sep 9, 2024
Merged

fix: range rendered in jump-on-scroll and stick-to-end #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6d46c05
refactor: improve readability of formula
JulienIzz Aug 29, 2024
1e59b8d
refactor: export to a function the computing of start and end indexes…
JulienIzz Aug 29, 2024
28f73d4
fix: compute correct range for every type of scroll
JulienIzz Aug 29, 2024
ed0b15d
doc: add doc on nbOfRenderedItems
JulienIzz Aug 29, 2024
75dc0f7
refactor: simplify getRawStartAndEndIndexes
JulienIzz Aug 29, 2024
86d2f64
chore: fix test readability
JulienIzz Sep 9, 2024
e05ef48
chore: add TSDoc for getRawIndexes
JulienIzz Sep 9, 2024
3ffea94
chore: remove throw and log error instead to avoid breaking change
JulienIzz Sep 9, 2024
6cf44e5
fix: use user input to compute range in jump on scroll
JulienIzz Sep 9, 2024
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
2 changes: 1 addition & 1 deletion docs/api.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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();
JulienIzz marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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
Open in desktop
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 = ({
JulienIzz marked this conversation as resolved.
Show resolved Hide resolved
currentlyFocusedItemIndex,
numberOfItemsVisibleOnScreen,
numberOfItemsNotVisible,
scrollBehavior,
}: {
currentlyFocusedItemIndex: number;
numberOfItemsVisibleOnScreen: number;
numberOfItemsNotVisible: number;
scrollBehavior: ScrollBehavior;
}) => {
const halfNumberOfItemsNotVisible = numberOfItemsNotVisible / 2;

switch (scrollBehavior) {
JulienIzz marked this conversation as resolved.
Show resolved Hide resolved
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
Loading