From 6c876c4dee0c858009d156f2e56c27b00a0ac422 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Wed, 22 Nov 2023 19:59:04 +0100 Subject: [PATCH 1/6] Fix bug with Web - Profile - When selecting the last country or state extra space is displayed for a while --- .../SelectionList/BaseSelectionList.js | 29 ++++++++++++++----- .../SelectionList/selectionListPropTypes.js | 3 ++ .../StatePicker/StateSelectorModal.js | 1 + .../PersonalDetails/CountrySelectionPage.js | 1 + 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 90ed7c707fe9..29b3f1c4ed3e 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -59,6 +59,7 @@ function BaseSelectionList({ disableKeyboardShortcuts = false, children, shouldStopPropagation = false, + shouldUseDynamicMaxToRenderPerBatch = false, }) { const theme = useTheme(); const styles = useThemeStyles(); @@ -71,6 +72,8 @@ function BaseSelectionList({ const shouldShowSelectAll = Boolean(onSelectAll); const activeElement = useActiveElement(); const isFocused = useIsFocused(); + const [maxToRenderPerBatch, setMaxToRenderPerBatch] = useState(5); + /** * Iterates through the sections and items inside each section, and builds 3 arrays along the way: * - `allOptions`: Contains all the items in the list, flattened, regardless of section @@ -310,13 +313,23 @@ function BaseSelectionList({ ); }; - const scrollToFocusedIndexOnFirstRender = useCallback(() => { - if (!firstLayoutRef.current) { - return; - } - scrollToIndex(focusedIndex, false); - firstLayoutRef.current = false; - }, [focusedIndex, scrollToIndex]); + const scrollToFocusedIndexOnFirstRender = useCallback( + ({nativeEvent}) => { + if (shouldUseDynamicMaxToRenderPerBatch) { + const listHeight = lodashGet(nativeEvent, 'layout.height', 0); + const itemHeight = lodashGet(nativeEvent, 'layout.y', 0); + + setMaxToRenderPerBatch((Math.ceil(listHeight / itemHeight) || 0) + 15); + } + + if (!firstLayoutRef.current) { + return; + } + scrollToIndex(focusedIndex, false); + firstLayoutRef.current = false; + }, + [focusedIndex, scrollToIndex], + ); const updateAndScrollToFocusedIndex = useCallback( (newFocusedIndex) => { @@ -451,7 +464,7 @@ function BaseSelectionList({ keyboardShouldPersistTaps="always" showsVerticalScrollIndicator={showScrollIndicator} initialNumToRender={12} - maxToRenderPerBatch={5} + maxToRenderPerBatch={maxToRenderPerBatch} windowSize={5} viewabilityConfig={{viewAreaCoveragePercentThreshold: 95}} testID="selection-list" diff --git a/src/components/SelectionList/selectionListPropTypes.js b/src/components/SelectionList/selectionListPropTypes.js index 5b95f7dd0cbf..66eec1ed49a4 100644 --- a/src/components/SelectionList/selectionListPropTypes.js +++ b/src/components/SelectionList/selectionListPropTypes.js @@ -182,6 +182,9 @@ const propTypes = { /** Custom content to display in the footer */ footerContent: PropTypes.oneOfType([PropTypes.func, PropTypes.node]), + + /** Whether to use dynamic maxToRenderPerBatch depending on the visible number of elements */ + shouldUseDynamicMaxToRenderPerBatch: PropTypes.bool, }; export {propTypes, baseListItemPropTypes, radioListItemPropTypes, userListItemPropTypes}; diff --git a/src/components/StatePicker/StateSelectorModal.js b/src/components/StatePicker/StateSelectorModal.js index be899adec0a2..908bb5eb5b2a 100644 --- a/src/components/StatePicker/StateSelectorModal.js +++ b/src/components/StatePicker/StateSelectorModal.js @@ -101,6 +101,7 @@ function StateSelectorModal({currentState, isVisible, onClose, onStateSelected, onChangeText={setSearchValue} initiallyFocusedOptionKey={currentState} shouldStopPropagation + shouldUseDynamicMaxToRenderPerBatch /> diff --git a/src/pages/settings/Profile/PersonalDetails/CountrySelectionPage.js b/src/pages/settings/Profile/PersonalDetails/CountrySelectionPage.js index a0c2a02b2cb5..38c4d1eac449 100644 --- a/src/pages/settings/Profile/PersonalDetails/CountrySelectionPage.js +++ b/src/pages/settings/Profile/PersonalDetails/CountrySelectionPage.js @@ -96,6 +96,7 @@ function CountrySelectionPage({route, navigation}) { onSelectRow={selectCountry} onChangeText={setSearchValue} initiallyFocusedOptionKey={currentCountry} + shouldUseDynamicMaxToRenderPerBatch /> ); From a175205c50b46c4a8f3d443620fe10285fe48496 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Wed, 22 Nov 2023 20:10:53 +0100 Subject: [PATCH 2/6] Update value to increase --- src/components/SelectionList/BaseSelectionList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 29b3f1c4ed3e..70efb2af6089 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -319,7 +319,7 @@ function BaseSelectionList({ const listHeight = lodashGet(nativeEvent, 'layout.height', 0); const itemHeight = lodashGet(nativeEvent, 'layout.y', 0); - setMaxToRenderPerBatch((Math.ceil(listHeight / itemHeight) || 0) + 15); + setMaxToRenderPerBatch((Math.ceil(listHeight / itemHeight) || 0) + 5); } if (!firstLayoutRef.current) { From 2b22aa7a0904641d156167202fa1b5d954a79c7f Mon Sep 17 00:00:00 2001 From: Yauheni Date: Wed, 22 Nov 2023 20:15:41 +0100 Subject: [PATCH 3/6] Fix lint warning --- src/components/SelectionList/BaseSelectionList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 70efb2af6089..3b5c5f295e0c 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -328,7 +328,7 @@ function BaseSelectionList({ scrollToIndex(focusedIndex, false); firstLayoutRef.current = false; }, - [focusedIndex, scrollToIndex], + [focusedIndex, scrollToIndex, shouldUseDynamicMaxToRenderPerBatch], ); const updateAndScrollToFocusedIndex = useCallback( From 2749bfd57172328630dfdbec78287c655ef75d3f Mon Sep 17 00:00:00 2001 From: Yauheni Date: Wed, 22 Nov 2023 21:26:55 +0100 Subject: [PATCH 4/6] Fix bug with mobile --- src/components/SelectionList/BaseListItem.js | 2 ++ src/components/SelectionList/BaseSelectionList.js | 3 ++- src/components/SelectionList/selectionListPropTypes.js | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/SelectionList/BaseListItem.js b/src/components/SelectionList/BaseListItem.js index a37f2fe4ddc0..6d89ab258fda 100644 --- a/src/components/SelectionList/BaseListItem.js +++ b/src/components/SelectionList/BaseListItem.js @@ -19,6 +19,7 @@ function BaseListItem({ item, isFocused = false, isDisabled = false, + isHide = false, showTooltip, shouldPreventDefaultFocusOnSelectRow = false, canSelectMultiple = false, @@ -56,6 +57,7 @@ function BaseListItem({ styles.userSelectNone, isUserItem ? styles.peopleRow : styles.optionRow, isFocused && styles.sidebarLinkActive, + isHide && styles.opacity0, ]} > {canSelectMultiple && ( diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 3b5c5f295e0c..2ccfb56966cf 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -72,7 +72,7 @@ function BaseSelectionList({ const shouldShowSelectAll = Boolean(onSelectAll); const activeElement = useActiveElement(); const isFocused = useIsFocused(); - const [maxToRenderPerBatch, setMaxToRenderPerBatch] = useState(5); + const [maxToRenderPerBatch, setMaxToRenderPerBatch] = useState(shouldUseDynamicMaxToRenderPerBatch ? 0 : 5); /** * Iterates through the sections and items inside each section, and builds 3 arrays along the way: @@ -304,6 +304,7 @@ function BaseSelectionList({ item={item} isFocused={isItemFocused} isDisabled={isDisabled} + isHide={!maxToRenderPerBatch} showTooltip={showTooltip} canSelectMultiple={canSelectMultiple} onSelectRow={() => selectRow(item, true)} diff --git a/src/components/SelectionList/selectionListPropTypes.js b/src/components/SelectionList/selectionListPropTypes.js index 66eec1ed49a4..7810280c178a 100644 --- a/src/components/SelectionList/selectionListPropTypes.js +++ b/src/components/SelectionList/selectionListPropTypes.js @@ -91,6 +91,7 @@ const baseListItemPropTypes = { ...commonListItemPropTypes, item: PropTypes.oneOfType([PropTypes.shape(userListItemPropTypes.item), PropTypes.shape(radioListItemPropTypes.item)]), shouldPreventDefaultFocusOnSelectRow: PropTypes.bool, + isHide: PropTypes.bool, }; const propTypes = { @@ -183,7 +184,7 @@ const propTypes = { /** Custom content to display in the footer */ footerContent: PropTypes.oneOfType([PropTypes.func, PropTypes.node]), - /** Whether to use dynamic maxToRenderPerBatch depending on the visible number of elements */ + /** Whether to use dynamic maxToRenderPerBatch depending on the visible number of elements */ shouldUseDynamicMaxToRenderPerBatch: PropTypes.bool, }; From 5e1acdcc817e26b3d8cb43ba996609c52bac230e Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 25 Nov 2023 21:39:03 +0100 Subject: [PATCH 5/6] Optimize the list while transitioning between screens --- src/components/SelectionList/BaseListItem.js | 2 -- src/components/SelectionList/BaseSelectionList.js | 1 + src/components/SelectionList/selectionListPropTypes.js | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/SelectionList/BaseListItem.js b/src/components/SelectionList/BaseListItem.js index 6d89ab258fda..a37f2fe4ddc0 100644 --- a/src/components/SelectionList/BaseListItem.js +++ b/src/components/SelectionList/BaseListItem.js @@ -19,7 +19,6 @@ function BaseListItem({ item, isFocused = false, isDisabled = false, - isHide = false, showTooltip, shouldPreventDefaultFocusOnSelectRow = false, canSelectMultiple = false, @@ -57,7 +56,6 @@ function BaseListItem({ styles.userSelectNone, isUserItem ? styles.peopleRow : styles.optionRow, isFocused && styles.sidebarLinkActive, - isHide && styles.opacity0, ]} > {canSelectMultiple && ( diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 2ccfb56966cf..19c8c646103d 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -470,6 +470,7 @@ function BaseSelectionList({ viewabilityConfig={{viewAreaCoveragePercentThreshold: 95}} testID="selection-list" onLayout={scrollToFocusedIndexOnFirstRender} + style={!maxToRenderPerBatch && styles.opacity0} /> {children} diff --git a/src/components/SelectionList/selectionListPropTypes.js b/src/components/SelectionList/selectionListPropTypes.js index 7810280c178a..0c2fe83d025f 100644 --- a/src/components/SelectionList/selectionListPropTypes.js +++ b/src/components/SelectionList/selectionListPropTypes.js @@ -91,7 +91,6 @@ const baseListItemPropTypes = { ...commonListItemPropTypes, item: PropTypes.oneOfType([PropTypes.shape(userListItemPropTypes.item), PropTypes.shape(radioListItemPropTypes.item)]), shouldPreventDefaultFocusOnSelectRow: PropTypes.bool, - isHide: PropTypes.bool, }; const propTypes = { From 6cf91f61cb339c984b1a97ff79af8659ccef128f Mon Sep 17 00:00:00 2001 From: Yauheni Date: Mon, 27 Nov 2023 10:56:31 +0100 Subject: [PATCH 6/6] Add constants for maxToRenderPerBatch --- src/CONST.ts | 8 ++++++++ src/components/Attachments/AttachmentCarousel/index.js | 3 ++- src/components/LHNOptionsList/LHNOptionsList.js | 2 +- src/components/OptionsList/BaseOptionsList.js | 3 ++- src/components/SelectionList/BaseSelectionList.js | 4 ++-- 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index 47ab30589612..1693208f138a 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -2908,6 +2908,14 @@ const CONST = { PERFORMANCE_TESTS: { RUNS: 20, }, + + /** + * Constants for maxToRenderPerBatch parameter that is used for FlatList or SectionList. This controls the amount of items rendered per batch, which is the next chunk of items rendered on every scroll. + */ + MAX_TO_RENDER_PER_BATCH: { + DEFAULT: 5, + CAROUSEL: 3, + }, } as const; export default CONST; diff --git a/src/components/Attachments/AttachmentCarousel/index.js b/src/components/Attachments/AttachmentCarousel/index.js index fa4ff50512d0..141e619e489e 100644 --- a/src/components/Attachments/AttachmentCarousel/index.js +++ b/src/components/Attachments/AttachmentCarousel/index.js @@ -12,6 +12,7 @@ import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import Navigation from '@libs/Navigation/Navigation'; import useThemeStyles from '@styles/useThemeStyles'; import variables from '@styles/variables'; +import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import AttachmentCarouselCellRenderer from './AttachmentCarouselCellRenderer'; import {defaultProps, propTypes} from './attachmentCarouselPropTypes'; @@ -203,7 +204,7 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, initialScrollIndex={page} initialNumToRender={3} windowSize={5} - maxToRenderPerBatch={3} + maxToRenderPerBatch={CONST.MAX_TO_RENDER_PER_BATCH.CAROUSEL} data={attachments} CellRendererComponent={AttachmentCarouselCellRenderer} renderItem={renderItem} diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 5e77947187e9..07b08b410300 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -176,7 +176,7 @@ function LHNOptionsList({ renderItem={renderItem} getItemLayout={getItemLayout} initialNumToRender={20} - maxToRenderPerBatch={5} + maxToRenderPerBatch={CONST.MAX_TO_RENDER_PER_BATCH.DEFAULT} windowSize={5} /> diff --git a/src/components/OptionsList/BaseOptionsList.js b/src/components/OptionsList/BaseOptionsList.js index 2f81e6d80e7d..ac39e25411a2 100644 --- a/src/components/OptionsList/BaseOptionsList.js +++ b/src/components/OptionsList/BaseOptionsList.js @@ -9,6 +9,7 @@ import Text from '@components/Text'; import usePrevious from '@hooks/usePrevious'; import useThemeStyles from '@styles/useThemeStyles'; import variables from '@styles/variables'; +import CONST from '@src/CONST'; import {defaultProps as optionsListDefaultProps, propTypes as optionsListPropTypes} from './optionsListPropTypes'; const propTypes = { @@ -281,7 +282,7 @@ function BaseOptionsList({ renderSectionHeader={renderSectionHeader} extraData={focusedIndex} initialNumToRender={12} - maxToRenderPerBatch={5} + maxToRenderPerBatch={CONST.MAX_TO_RENDER_PER_BATCH.DEFAULT} windowSize={5} viewabilityConfig={{viewAreaCoveragePercentThreshold: 95}} onViewableItemsChanged={onViewableItemsChanged} diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 19c8c646103d..6f53679f28d3 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -72,7 +72,7 @@ function BaseSelectionList({ const shouldShowSelectAll = Boolean(onSelectAll); const activeElement = useActiveElement(); const isFocused = useIsFocused(); - const [maxToRenderPerBatch, setMaxToRenderPerBatch] = useState(shouldUseDynamicMaxToRenderPerBatch ? 0 : 5); + const [maxToRenderPerBatch, setMaxToRenderPerBatch] = useState(shouldUseDynamicMaxToRenderPerBatch ? 0 : CONST.MAX_TO_RENDER_PER_BATCH.DEFAULT); /** * Iterates through the sections and items inside each section, and builds 3 arrays along the way: @@ -320,7 +320,7 @@ function BaseSelectionList({ const listHeight = lodashGet(nativeEvent, 'layout.height', 0); const itemHeight = lodashGet(nativeEvent, 'layout.y', 0); - setMaxToRenderPerBatch((Math.ceil(listHeight / itemHeight) || 0) + 5); + setMaxToRenderPerBatch((Math.ceil(listHeight / itemHeight) || 0) + CONST.MAX_TO_RENDER_PER_BATCH.DEFAULT); } if (!firstLayoutRef.current) {