From d22488abc1150f15c5a16b932f17848b002d8577 Mon Sep 17 00:00:00 2001 From: Pierre Poupin Date: Tue, 15 Oct 2024 16:24:47 +0200 Subject: [PATCH] fix(lists)!: make additionally rendered items less error prone 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. --- .../SpatialNavigationVirtualizedList.test.tsx | 1 + .../virtualizedList/VirtualizedList.tsx | 22 ++++++++++---- .../getAdditionalNumberOfItemsRendered.ts | 30 ++++++++++++++++--- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/packages/lib/src/spatial-navigation/components/virtualizedList/SpatialNavigationVirtualizedList.test.tsx b/packages/lib/src/spatial-navigation/components/virtualizedList/SpatialNavigationVirtualizedList.test.tsx index 3684b941..22c51d8d 100644 --- a/packages/lib/src/spatial-navigation/components/virtualizedList/SpatialNavigationVirtualizedList.test.tsx +++ b/packages/lib/src/spatial-navigation/components/virtualizedList/SpatialNavigationVirtualizedList.test.tsx @@ -412,6 +412,7 @@ describe('SpatialNavigationVirtualizedList', () => { data={data} itemSize={100} scrollBehavior="jump-on-scroll" + additionalItemsRendered={0} /> , diff --git a/packages/lib/src/spatial-navigation/components/virtualizedList/VirtualizedList.tsx b/packages/lib/src/spatial-navigation/components/virtualizedList/VirtualizedList.tsx index dd9a4dfb..023f8095 100644 --- a/packages/lib/src/spatial-navigation/components/virtualizedList/VirtualizedList.tsx +++ b/packages/lib/src/spatial-navigation/components/virtualizedList/VirtualizedList.tsx @@ -19,7 +19,17 @@ export interface VirtualizedListProps { /** 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 */ @@ -127,7 +137,7 @@ export const VirtualizedList = typedMemo( renderItem, itemSize, currentlyFocusedItemIndex, - additionalItemsRendered, + additionalItemsRendered = 2, onEndReached, onEndReachedThresholdItemsNumber = 3, style, @@ -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, diff --git a/packages/lib/src/spatial-navigation/components/virtualizedList/helpers/getAdditionalNumberOfItemsRendered.ts b/packages/lib/src/spatial-navigation/components/virtualizedList/helpers/getAdditionalNumberOfItemsRendered.ts index dcbd1f89..1f96d6ab 100644 --- a/packages/lib/src/spatial-navigation/components/virtualizedList/helpers/getAdditionalNumberOfItemsRendered.ts +++ b/packages/lib/src/spatial-navigation/components/virtualizedList/helpers/getAdditionalNumberOfItemsRendered.ts @@ -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 + ); } };