Skip to content

Commit

Permalink
fix(lists)!: make additionally rendered items less error prone
Browse files Browse the repository at this point in the history
Before this change, putting 1 as additionnally rendered items would
crash the list.
The minimum was 2 and it is not very explicit.

Now, minimum is 0, and default is 2.

This is breaking change because it shifts the
additionaNumberOfItemsRendered value by 2.
  • Loading branch information
pierpo committed Oct 15, 2024
1 parent 5c54400 commit d22488a
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ describe('SpatialNavigationVirtualizedList', () => {
data={data}
itemSize={100}
scrollBehavior="jump-on-scroll"
additionalItemsRendered={0}
/>
</DefaultFocus>
</SpatialNavigationRoot>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,17 @@ export interface VirtualizedListProps<T> {
/** If vertical the height of an item, otherwise the width */
itemSize: number | ((item: T) => number);
currentlyFocusedItemIndex: number;
/** How many items are RENDERED ADDITIONALLY to those already visible (impacts virtualization size). Defaults to 4 for stick-to-start & stick-to-end scrolls, and twice the number of elements visible + 1 for jump-on-scroll. */
/**
* How many items are RENDERED ADDITIONALLY to the minimum amount possible. It impacts virtualization size.
* Defaults to 2.
*
* Should be a POSITIVE number.
*
* Minimal amount possible is 2N + 1 for jump-on-scroll, and N + 2 for the other behaviours, N being the number
* of visible elements on the screen.
*
* By default, you will then have 2N + 2 + 2 elements rendered for stick-to-* behaviours.
*/
additionalItemsRendered?: number;
onEndReached?: () => void;
/** Number of items left to display before triggering onEndReached */
Expand Down Expand Up @@ -127,7 +137,7 @@ export const VirtualizedList = typedMemo(
renderItem,
itemSize,
currentlyFocusedItemIndex,
additionalItemsRendered,
additionalItemsRendered = 2,
onEndReached,
onEndReachedThresholdItemsNumber = 3,
style,
Expand All @@ -145,9 +155,11 @@ export const VirtualizedList = typedMemo(
itemSize,
});

const numberOfItemsToRender = additionalItemsRendered
? additionalItemsRendered + numberOfItemsVisibleOnScreen
: getAdditionalNumberOfItemsRendered(scrollBehavior, numberOfItemsVisibleOnScreen);
const numberOfItemsToRender = getAdditionalNumberOfItemsRendered(
scrollBehavior,
numberOfItemsVisibleOnScreen,
additionalItemsRendered,
);

const range = getRange({
data,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,36 @@
import { ScrollBehavior } from '../VirtualizedList';

/**
* If list rendered elements is too small, it creates spatial navigation bugs
* like not being able to go back on the left
*
* There are other ways to fix this than forcing a minimum number of additional items to render
* but having a minimum number items to render inferior to the window size makes no sense anyway
*/
const MINIMUM_ADDITIONAL_ITEMS_TO_WORK = 2;

export const getAdditionalNumberOfItemsRendered = (
scrollBehavior: ScrollBehavior,
numberOfElementsVisibleOnScreen: number,
additionalNumberOfItemsRendered: number,
) => {
if (additionalNumberOfItemsRendered < 0) {
console.error(
'[VirtualizedList] Negative number of additional items to render was given, no elements will be rendered',
);
}

switch (scrollBehavior) {
case 'stick-to-start':
case 'stick-to-end':
return 4 + numberOfElementsVisibleOnScreen;
case 'jump-on-scroll':
return 2 * numberOfElementsVisibleOnScreen + 1;
// This is a special case
// Since we're jumping on scroll, we need to render more items to make sure that a whole
// window is ready when we jump!
return 2 * numberOfElementsVisibleOnScreen + 1 + additionalNumberOfItemsRendered;
default:
return (
numberOfElementsVisibleOnScreen +
MINIMUM_ADDITIONAL_ITEMS_TO_WORK +
additionalNumberOfItemsRendered
);
}
};

0 comments on commit d22488a

Please sign in to comment.