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.

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

Item in report list is not highlighted and list cannot be navigated with keyboard #37081

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
79f02bc
Fix bug with Item in report list is not highlighted and list cannot b…
ZhenjaHorbach Feb 22, 2024
172c807
Fix TS issue and update tests
ZhenjaHorbach Feb 22, 2024
f953b79
Merge branch 'main' into item-in-report-list-is-not-highlighted-and-l…
ZhenjaHorbach Feb 22, 2024
41f1507
Update branch
ZhenjaHorbach Feb 22, 2024
049756b
Update tests
ZhenjaHorbach Feb 22, 2024
150c90b
Update branch
ZhenjaHorbach Feb 22, 2024
2317ae5
Refactore code and add new utilit for sections
ZhenjaHorbach Feb 27, 2024
4ad85e6
Update branch
ZhenjaHorbach Feb 27, 2024
279cf4a
Update getSectionsWithIndexOffset
ZhenjaHorbach Feb 27, 2024
7eb76d6
Remove unnecessary code
ZhenjaHorbach Feb 27, 2024
6794d02
Update branch
ZhenjaHorbach Feb 28, 2024
fc784e6
Remove unnecessary code
ZhenjaHorbach Feb 28, 2024
8154f93
Update branch
ZhenjaHorbach Feb 29, 2024
5d440d7
Update branch
ZhenjaHorbach Feb 29, 2024
86ec95b
Update branch
ZhenjaHorbach Mar 4, 2024
0db97cf
Remove unnecessary code
ZhenjaHorbach Mar 4, 2024
1acf6b2
Fix comments
ZhenjaHorbach Mar 5, 2024
27576a0
Merge branch 'main' into item-in-report-list-is-not-highlighted-and-l…
ZhenjaHorbach Mar 5, 2024
7d67f31
Remove unnecessary code
ZhenjaHorbach Mar 5, 2024
fadf22a
Update branch
ZhenjaHorbach Mar 11, 2024
131435c
Update branch
ZhenjaHorbach Mar 15, 2024
826fbec
Update stories
ZhenjaHorbach Mar 15, 2024
672e271
Update branch
ZhenjaHorbach Mar 18, 2024
fba4343
Update branch and update perf-test
ZhenjaHorbach Mar 18, 2024
ff625d2
Fix lint issues
ZhenjaHorbach Mar 18, 2024
10fcd80
Update branch
ZhenjaHorbach Mar 19, 2024
713eee6
Update branch
ZhenjaHorbach Mar 20, 2024
90e3f46
Update branch and fix conflicts
ZhenjaHorbach Mar 25, 2024
83c4c31
Fix comments
ZhenjaHorbach Mar 25, 2024
e508f98
Update bransh and fix conflicts
ZhenjaHorbach Mar 28, 2024
98edd68
Update branch and fix conflicts
ZhenjaHorbach Mar 29, 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
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function YearPickerModal({isVisible, years, currentYear = new Date().getFullYear
const yearsList = searchText === '' ? years : years.filter((year) => year.text?.includes(searchText));
return {
headerMessage: !yearsList.length ? translate('common.noResultsFound') : '',
sections: [{data: yearsList.sort((a, b) => b.value - a.value), indexOffset: 0}],
sections: [{data: yearsList.sort((a, b) => b.value - a.value)}],
};
}, [years, searchText, translate]);

Expand Down
3 changes: 0 additions & 3 deletions src/components/MoneyRequestConfirmationList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -388,14 +388,12 @@ function MoneyRequestConfirmationList({
title: translate('moneyRequestConfirmationList.paidBy'),
data: [formattedPayeeOption],
shouldShow: true,
indexOffset: 0,
isDisabled: shouldDisablePaidBySection,
},
{
title: translate('moneyRequestConfirmationList.splitWith'),
data: formattedParticipantsList,
shouldShow: true,
indexOffset: 1,
},
);
} else {
Expand All @@ -407,7 +405,6 @@ function MoneyRequestConfirmationList({
title: translate('common.to'),
data: formattedSelectedParticipants,
shouldShow: true,
indexOffset: 0,
});
}
return sections;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,14 +436,12 @@ function MoneyTemporaryForRefactorRequestConfirmationList({
title: translate('moneyRequestConfirmationList.paidBy'),
data: [formattedPayeeOption],
shouldShow: true,
indexOffset: 0,
isDisabled: shouldDisablePaidBySection,
},
{
title: translate('moneyRequestConfirmationList.splitWith'),
data: formattedParticipantsList,
shouldShow: true,
indexOffset: 1,
},
);
} else {
Expand All @@ -455,7 +453,6 @@ function MoneyTemporaryForRefactorRequestConfirmationList({
title: translate('common.to'),
data: formattedSelectedParticipants,
shouldShow: true,
indexOffset: 0,
});
}
return sections;
Expand Down
15 changes: 9 additions & 6 deletions src/components/OptionsList/BaseOptionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import SectionList from '@components/SectionList';
import Text from '@components/Text';
import usePrevious from '@hooks/usePrevious';
import useThemeStyles from '@hooks/useThemeStyles';
import getSectionsWithIndexOffset from '@libs/getSectionsWithIndexOffset';
import type {OptionData} from '@libs/ReportUtils';
import StringUtils from '@libs/StringUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import type {BaseOptionListProps, OptionsList, OptionsListData, Section} from './types';
import type {BaseOptionListProps, OptionsList, OptionsListData, OptionsListDataWithIndexOffset, SectionWithIndexOffset} from './types';

function BaseOptionsList(
{
Expand Down Expand Up @@ -67,6 +68,7 @@ function BaseOptionsList(

const listContainerStyles = useMemo(() => listContainerStylesProp ?? [styles.flex1], [listContainerStylesProp, styles.flex1]);
const contentContainerStyles = useMemo(() => [safeAreaPaddingBottomStyle, contentContainerStylesProp], [contentContainerStylesProp, safeAreaPaddingBottomStyle]);
const sectionsWithIndexOffset = getSectionsWithIndexOffset(sections);

/**
* This helper function is used to memoize the computation needed for getItemLayout. It is run whenever section data changes.
Expand Down Expand Up @@ -133,7 +135,8 @@ function BaseOptionsList(
*
* [{header}, {sectionHeader}, {item}, {item}, {sectionHeader}, {item}, {item}, {footer}]
*/
const getItemLayout = (_data: OptionsListData[] | null, flatDataArrayIndex: number) => {
// eslint-disable-next-line @typescript-eslint/naming-convention
const getItemLayout = (_data: OptionsListDataWithIndexOffset[] | null, flatDataArrayIndex: number) => {
Comment on lines +138 to +139
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we need to violation this lint rule, or can we do something like this?

Suggested change
// eslint-disable-next-line @typescript-eslint/naming-convention
const getItemLayout = (_data: OptionsListDataWithIndexOffset[] | null, flatDataArrayIndex: number) => {
const getItemLayout = (data: OptionsListDataWithIndexOffset[] | null, flatDataArrayIndex: number) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-03-25 at 22 31 31

In this case we get ts error

if (!flattenedData.current[flatDataArrayIndex]) {
flattenedData.current = buildFlatSectionArray();
}
Expand Down Expand Up @@ -161,7 +164,7 @@ function BaseOptionsList(
* @return {Component}
*/

const renderItem: SectionListRenderItem<OptionData, Section> = ({item, index, section}) => {
const renderItem: SectionListRenderItem<OptionData, SectionWithIndexOffset> = ({item, index, section}) => {
const isItemDisabled = isDisabled || !!section.isDisabled || !!item.isDisabled;
const isSelected = selectedOptions?.some((option) => {
if (option.keyForList && option.keyForList === item.keyForList) {
Expand Down Expand Up @@ -202,7 +205,7 @@ function BaseOptionsList(
/**
* Function which renders a section header component
*/
const renderSectionHeader = ({section: {title, shouldShow}}: {section: OptionsListData}) => {
const renderSectionHeader = ({section: {title, shouldShow}}: {section: OptionsListDataWithIndexOffset}) => {
if (!title && shouldShow && !hideSectionHeaders && sectionHeaderStyle) {
return <View style={sectionHeaderStyle} />;
}
Expand Down Expand Up @@ -235,7 +238,7 @@ function BaseOptionsList(
<Text style={[styles.textLabel, styles.colorMuted]}>{headerMessage}</Text>
</View>
) : null}
<SectionList<OptionData, Section>
<SectionList<OptionData, SectionWithIndexOffset>
ref={ref}
style={listStyles}
indicatorStyle="white"
Expand All @@ -247,7 +250,7 @@ function BaseOptionsList(
onScroll={onScroll}
contentContainerStyle={contentContainerStyles}
showsVerticalScrollIndicator={showScrollIndicator}
sections={sections}
sections={sectionsWithIndexOffset}
keyExtractor={extractKey}
stickySectionHeadersEnabled={false}
renderItem={renderItem}
Expand Down
13 changes: 8 additions & 5 deletions src/components/OptionsList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@ import type {RefObject} from 'react';
import type {SectionList, SectionListData, StyleProp, View, ViewStyle} from 'react-native';
import type {OptionData} from '@libs/ReportUtils';

type OptionsList = SectionList<OptionData, Section>;
type OptionsListData = SectionListData<OptionData, Section>;
type OptionsListDataWithIndexOffset = SectionListData<OptionData, SectionWithIndexOffset>;
type OptionsList = SectionList<OptionData, SectionWithIndexOffset>;

type Section = {
/** Title of the section */
title: string;

/** The initial index of this section given the total number of options in each section's data array */
indexOffset: number;

/** Array of options */
data: OptionData[];

Expand All @@ -22,6 +20,11 @@ type Section = {
isDisabled?: boolean;
};

type SectionWithIndexOffset = Section & {
/** The initial index of this section given the total number of options in each section's data array */
indexOffset: number;
};

type OptionsListProps = {
/** option flexStyle for the options list container */
listContainerStyles?: StyleProp<ViewStyle>;
Expand Down Expand Up @@ -134,4 +137,4 @@ type BaseOptionListProps = OptionsListProps & {
listStyles?: StyleProp<ViewStyle>;
};

export type {OptionsListProps, BaseOptionListProps, Section, OptionsList, OptionsListData};
export type {OptionsListProps, BaseOptionListProps, Section, OptionsList, OptionsListData, SectionWithIndexOffset, OptionsListDataWithIndexOffset};
3 changes: 0 additions & 3 deletions src/components/OptionsSelector/optionsSelectorPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ const propTypes = {
/** Title of the section */
title: PropTypes.string,

/** The initial index of this section given the total number of options in each section's data array */
indexOffset: PropTypes.number,

/** Array of options */
data: PropTypes.arrayOf(optionPropTypes),

Expand Down
30 changes: 16 additions & 14 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useLocalize from '@hooks/useLocalize';
import usePrevious from '@hooks/usePrevious';
import useThemeStyles from '@hooks/useThemeStyles';
import getSectionsWithIndexOffset from '@libs/getSectionsWithIndexOffset';
import Log from '@libs/Log';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import type {BaseSelectionListProps, ButtonOrCheckBoxRoles, FlattenedSectionsReturn, ListItem, Section, SectionListDataType, SelectionListHandle} from './types';
import type {BaseSelectionListProps, ButtonOrCheckBoxRoles, FlattenedSectionsReturn, ListItem, SectionListDataType, SectionWithIndexOffset, SelectionListHandle} from './types';

function BaseSelectionList<TItem extends ListItem>(
{
Expand Down Expand Up @@ -74,7 +75,7 @@ function BaseSelectionList<TItem extends ListItem>(
) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const listRef = useRef<RNSectionList<TItem, Section<TItem>>>(null);
const listRef = useRef<RNSectionList<TItem, SectionWithIndexOffset<TItem>>>(null);
const innerTextInputRef = useRef<RNTextInput | null>(null);
const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const shouldShowTextInput = !!textInputLabel;
Expand Down Expand Up @@ -166,15 +167,17 @@ function BaseSelectionList<TItem extends ListItem>(

const [slicedSections, ShowMoreButtonInstance] = useMemo(() => {
let remainingOptionsLimit = CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * currentPage;
const processedSections = sections.map((section) => {
const data = !isEmpty(section.data) && remainingOptionsLimit > 0 ? section.data.slice(0, remainingOptionsLimit) : [];
remainingOptionsLimit -= data.length;

return {
...section,
data,
};
});
const processedSections = getSectionsWithIndexOffset(
sections.map((section) => {
const data = !isEmpty(section.data) && remainingOptionsLimit > 0 ? section.data.slice(0, remainingOptionsLimit) : [];
remainingOptionsLimit -= data.length;

return {
...section,
data,
};
}),
);

const shouldShowMoreButton = flattenedSections.allOptions.length > CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * currentPage;
const showMoreButton = shouldShowMoreButton ? (
Expand Down Expand Up @@ -312,9 +315,8 @@ function BaseSelectionList<TItem extends ListItem>(
);
};

const renderItem = ({item, index, section}: SectionListRenderItemInfo<TItem, Section<TItem>>) => {
const indexOffset = section.indexOffset ? section.indexOffset : 0;
const normalizedIndex = index + indexOffset;
const renderItem = ({item, index, section}: SectionListRenderItemInfo<TItem, SectionWithIndexOffset<TItem>>) => {
const normalizedIndex = index + section.indexOffset;
const isDisabled = !!section.isDisabled || item.isDisabled;
const isItemFocused = !isDisabled && (focusedIndex === normalizedIndex || itemsToHighlight?.has(item.keyForList ?? ''));
// We only create tooltips for the first 10 users or so since some reports have hundreds of users, causing performance to degrade.
Expand Down
3 changes: 0 additions & 3 deletions src/components/SelectionList/selectionListPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,6 @@ const propTypes = {
/** Title of the section */
title: PropTypes.string,

/** The initial index of this section given the total number of options in each section's data array */
indexOffset: PropTypes.number,

/** Array of options */
data: PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.shape(userListItemPropTypes.item), PropTypes.shape(radioListItemPropTypes.item)])),

Expand Down
11 changes: 7 additions & 4 deletions src/components/SelectionList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,6 @@ type Section<TItem extends ListItem> = {
/** Title of the section */
title?: string;

/** The initial index of this section given the total number of options in each section's data array */
indexOffset?: number;

/** Array of options */
data?: TItem[];

Expand All @@ -174,6 +171,11 @@ type Section<TItem extends ListItem> = {
shouldShow?: boolean;
};

type SectionWithIndexOffset<TItem extends ListItem> = Section<TItem> & {
/** The initial index of this section given the total number of options in each section's data array */
indexOffset: number;
};

type BaseSelectionListProps<TItem extends ListItem> = Partial<ChildrenProps> & {
/** Sections for the section list */
sections: Array<SectionListData<TItem, Section<TItem>>> | typeof CONST.EMPTY_ARRAY;
Expand Down Expand Up @@ -324,12 +326,13 @@ type FlattenedSectionsReturn<TItem extends ListItem> = {

type ButtonOrCheckBoxRoles = 'button' | 'checkbox';

type SectionListDataType<TItem extends ListItem> = SectionListData<TItem, Section<TItem>>;
type SectionListDataType<TItem extends ListItem> = SectionListData<TItem, SectionWithIndexOffset<TItem>>;

export type {
BaseSelectionListProps,
CommonListItemProps,
Section,
SectionWithIndexOffset,
BaseListItemProps,
UserListItemProps,
RadioListItemProps,
Expand Down
2 changes: 1 addition & 1 deletion src/components/StatePicker/StateSelectorModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function StateSelectorModal({currentState, isVisible, onClose = () => {}, onStat
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
textInputLabel={label || translate('common.state')}
textInputValue={searchValue}
sections={[{data: searchResults, indexOffset: 0}]}
sections={[{data: searchResults}]}
onSelectRow={onStateSelected}
onChangeText={setSearchValue}
initiallyFocusedOptionKey={currentState}
Expand Down
Loading
Loading