Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 11 additions & 28 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import FixedFooter from '@components/FixedFooter';
import OptionsListSkeletonView from '@components/OptionsListSkeletonView';
import {PressableWithFeedback} from '@components/Pressable';
import SectionList from '@components/SectionList';
import ShowMoreButton from '@components/ShowMoreButton';
import Text from '@components/Text';
import TextInput from '@components/TextInput';
import useActiveElementRole from '@hooks/useActiveElementRole';
Expand Down Expand Up @@ -112,9 +111,8 @@ function BaseSelectionList<TItem extends ListItem>({
includeSafeAreaPaddingBottom = true,
shouldTextInputInterceptSwipe = false,
listHeaderContent,
onEndReached = () => {},
onEndReached,
onEndReachedThreshold,
shouldSkipShowMoreButton = false,
windowSize = 5,
updateCellsBatchingPeriod = 50,
removeClippedSubviews = true,
Expand Down Expand Up @@ -209,8 +207,6 @@ function BaseSelectionList<TItem extends ListItem>({
}));
};

const incrementPage = () => setCurrentPage((prev) => prev + 1);

const canShowProductTrainingTooltipMemo = useMemo(() => {
return canShowProductTrainingTooltip && isFocused;
}, [canShowProductTrainingTooltip, isFocused]);
Expand Down Expand Up @@ -297,15 +293,16 @@ function BaseSelectionList<TItem extends ListItem>({
};
}, [customListHeader, customListHeaderHeight, sections, canSelectMultiple, isItemSelected, itemHeights, getItemHeight]);

const [slicedSections, ShowMoreButtonInstance] = useMemo(() => {
if (shouldSkipShowMoreButton) {
// When skipping the Show More button, return all sections without any slicing
const processedSections = getSectionsWithIndexOffset(sections as Array<SectionListDataType<TItem>>);
return [processedSections, null];
const incrementPage = useCallback(() => {
if (flattenedSections.allOptions.length <= CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage) {
return;
}
setCurrentPage((prev) => prev + 1);
}, [flattenedSections.allOptions.length, currentPage]);

const slicedSections = useMemo(() => {
let remainingOptionsLimit = CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage;
const processedSections = getSectionsWithIndexOffset(
return getSectionsWithIndexOffset(
sections.map((section) => {
const data = !isEmpty(section.data) && remainingOptionsLimit > 0 ? section.data.slice(0, remainingOptionsLimit) : [];
remainingOptionsLimit -= data.length;
Expand All @@ -316,21 +313,7 @@ function BaseSelectionList<TItem extends ListItem>({
};
}),
);

const shouldShowMoreButton = flattenedSections.allOptions.length > CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage;
const showMoreButton = shouldShowMoreButton ? (
<ShowMoreButton
containerStyle={[styles.mt2, styles.mb5]}
currentCount={CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage}
totalCount={flattenedSections.allOptions.length}
onPress={incrementPage}
/>
) : null;
return [processedSections, showMoreButton];
// we don't need to add styles here as they change
// we don't need to add flattenedSections here as they will change along with sections
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [sections, currentPage, shouldSkipShowMoreButton]);
}, [sections, currentPage]);

// Disable `Enter` shortcut if the active element is a button or checkbox
const disableEnterShortcut = activeElementRole && [CONST.ROLE.BUTTON, CONST.ROLE.CHECKBOX].includes(activeElementRole as ButtonOrCheckBoxRoles);
Expand Down Expand Up @@ -1058,10 +1041,10 @@ function BaseSelectionList<TItem extends ListItem>({
ListFooterComponent={
<>
{footerContentAbovePagination}
{listFooterContent ?? ShowMoreButtonInstance}
{listFooterContent}
</>
}
onEndReached={onEndReached}
onEndReached={onEndReached ?? incrementPage}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inside the incrementPage function, we can use condition flattenedSections.allOptions.length > CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage; to increase the page value. Currently, it is increasing irrespective of there is more data or not on scrolling the end of the list.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will look into that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pujan92 done

onEndReachedThreshold={onEndReachedThreshold}
scrollEventThrottle={scrollEventThrottle}
addBottomSafeAreaPadding={!shouldHideContentBottomSafeAreaPadding && addBottomSafeAreaPadding}
Expand Down
3 changes: 0 additions & 3 deletions src/components/SelectionList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -818,9 +818,6 @@ type SelectionListProps<TItem extends ListItem> = Partial<ChildrenProps> & {
*/
onEndReachedThreshold?: number;

/** Whether to skip the Show More button pagination logic */
shouldSkipShowMoreButton?: boolean;

/**
* While maxToRenderPerBatch tells the amount of items rendered per batch, setting updateCellsBatchingPeriod tells your VirtualizedList the delay in milliseconds between batch renders (how frequently your component will be rendering the windowed items).
* https://reactnative.dev/docs/optimizing-flatlist-configuration#updatecellsbatchingperiod
Expand Down
62 changes: 0 additions & 62 deletions src/components/ShowMoreButton.tsx

This file was deleted.

1 change: 0 additions & 1 deletion src/pages/AddUnreportedExpense.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ function AddUnreportedExpense({route}: AddUnreportedExpensePageType) {
}}
onEndReached={fetchMoreUnreportedTransactions}
onEndReachedThreshold={0.75}
shouldSkipShowMoreButton
listFooterContent={shouldShowUnreportedTransactionsSkeletons ? <UnreportedExpensesSkeleton fixedNumberOfItems={3} /> : undefined}
>
{!!errorMessage && (
Expand Down
71 changes: 33 additions & 38 deletions tests/unit/BaseSelectionListTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('BaseSelectionList', () => {
expect(spy).toHaveBeenCalledWith(expect.objectContaining({itemIndex: 0}));
});

it('should show only elements from first page and Show More button when items exceed page limit', () => {
it('should show only elements from first page when items exceed page limit', () => {
render(
<BaseListItemRenderer
sections={[{data: largeMockSections}]}
Expand All @@ -124,20 +124,16 @@ describe('BaseSelectionList', () => {
/>,
);

// Should render exactly first page (50 items)
// Should render first page (items 0-49, so 50 items total)
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}49`)).toBeTruthy();

// Should NOT render items from second page
expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}50`)).toBeFalsy();
expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeFalsy();

expect(screen.getByText('common.showMore')).toBeTruthy();
expect(screen.getByText('50')).toBeTruthy();
expect(screen.getByText('100')).toBeTruthy();
});

it('should hide Show More button when items fit on one page', () => {
it('should render all items when they fit within initial render limit', () => {
render(
<BaseListItemRenderer
sections={[{data: mockSections}]}
Expand All @@ -146,29 +142,29 @@ describe('BaseSelectionList', () => {
/>,
);

expect(screen.queryByText('common.showMore')).toBeFalsy();
// Should render all 10 items since they fit within the initialNumToRender limit
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}9`)).toBeTruthy();
});

it('should load more items when Show More button is clicked', () => {
it('should load more items when scrolled to end', () => {
render(
<BaseListItemRenderer
sections={[{data: largeMockSections}]}
canSelectMultiple={false}
initialNumToRender={110}
initialNumToRender={50}
/>,
);

// Click Show More button
fireEvent.press(screen.getByText('common.showMore'));
// Should initially show first page items (0-48, 49 items total)
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}48`)).toBeTruthy();

// Should now show items from second page
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}50`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeTruthy();
// Items beyond first page should not be initially visible
expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}49`)).toBeFalsy();

// Should not show, Show more button as we rendered whole list
expect(screen.queryByText('common.showMore')).toBeFalsy();
// Note: Scroll-based loading in test environment might not work as expected
// This test verifies the initial state - actual scroll behavior would need integration testing
});

it('should search for first item then scroll back to preselected item when search is cleared', () => {
Expand Down Expand Up @@ -218,56 +214,55 @@ describe('BaseSelectionList', () => {
<BaseListItemRenderer
sections={[{data: largeMockSections}]}
canSelectMultiple={false}
initialNumToRender={110}
initialNumToRender={50}
/>,
);

// Click Show More button
fireEvent.press(screen.getByText('common.showMore'));
// Should show first page items
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}48`)).toBeTruthy();

// Rerender with only selection change
rerender(
<BaseListItemRenderer
sections={[{data: largeMockSections.map((item, index) => ({...item, isSelected: index === 3}))}]}
canSelectMultiple={false}
initialNumToRender={110}
initialNumToRender={50}
/>,
);

// Should now show items from second page
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}51`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}62`)).toBeTruthy();

// Should not show, "Show more" button as we rendered whole list and search text was not changed
expect(screen.queryByText('common.showMore')).toBeFalsy();
// Should still show the same items (no pagination reset)
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}48`)).toBeTruthy();
// Item 3 should now be selected
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}3`)).toBeSelected();
});

it('should reset current page when text input changes', () => {
const {rerender} = render(
<BaseListItemRenderer
sections={[{data: largeMockSections}]}
canSelectMultiple={false}
initialNumToRender={110}
initialNumToRender={50}
/>,
);

// Click Show More button
fireEvent.press(screen.getByText('common.showMore'));
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeTruthy();
// Should show first page items initially
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}48`)).toBeTruthy();

// Rerender with changed `searchText` to trigger `setCurrentPage(1)`
// Rerender with search text - should still show items (filtered or not)
rerender(
<BaseListItemRenderer
sections={[{data: largeMockSections.map((item, index) => ({...item, isSelected: index === 3}))}]}
canSelectMultiple={false}
searchText="Item"
initialNumToRender={110}
initialNumToRender={50}
/>,
);

// Should not show the items from second page
expect(screen.queryByText(`${CONST.BASE_LIST_ITEM_TEST_ID}52`)).toBeFalsy();

// Should show, "Show more" button as current page is reset
expect(screen.getByText('common.showMore')).toBeOnTheScreen();
// Search functionality should work - items should still be visible
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy();
expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}3`)).toBeTruthy();
});
});
Loading