-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Implement accessibility changes for search suggestions #85296
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
base: main
Are you sure you want to change the base?
Changes from all commits
468e57c
5991c37
8307d98
f5a328a
3a804f3
7db6af5
e920326
6c1e503
e268d33
0a5b5ec
4333f8d
9443412
1046f4b
819ad95
9adc6e7
b41e509
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import type {ForwardedRef, RefObject} from 'react'; | ||
| import React, {useContext, useEffect, useRef, useState} from 'react'; | ||
| import React, {useContext, useEffect, useMemo, useRef, useState} from 'react'; | ||
| import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; | ||
| import {OptionsListStateContext, useOptionsList} from '@components/OptionListContextProvider'; | ||
| import OptionsListSkeletonView from '@components/OptionsListSkeletonView'; | ||
|
|
@@ -11,6 +11,7 @@ import type {Section, SelectionListWithSectionsHandle} from '@components/Selecti | |
| import useAutocompleteSuggestions from '@hooks/useAutocompleteSuggestions'; | ||
| import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; | ||
| import useDebounce from '@hooks/useDebounce'; | ||
| import useDebouncedAccessibilityAnnouncement from '@hooks/useDebouncedAccessibilityAnnouncement'; | ||
| import useFeedKeysWithAssignedCards from '@hooks/useFeedKeysWithAssignedCards'; | ||
| import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; | ||
| import useLocalize from '@hooks/useLocalize'; | ||
|
|
@@ -352,73 +353,86 @@ function SearchAutocompleteList({ | |
| }, [autocompleteQueryWithoutFilters, debounceHandleSearch]); | ||
|
|
||
| /* Sections generation */ | ||
| const sections: Array<Section<AutocompleteListItem>> = []; | ||
| let sectionIndex = 0; | ||
| const {sections, styledRecentReports, suggestionsCount} = useMemo(() => { | ||
| const nextSections: Array<Section<AutocompleteListItem>> = []; | ||
| let sectionIndex = 0; | ||
| let nextSuggestionsCount = 0; | ||
|
|
||
| const pushSection = (section: Section<AutocompleteListItem>) => { | ||
| nextSections.push(section); | ||
| nextSuggestionsCount += section.data.filter((item) => item.keyForList !== CONST.SEARCH.SEARCH_ROUTER_ITEM_TYPE.FIND_ITEM).length; | ||
| }; | ||
|
|
||
| if (searchQueryItem) { | ||
| sections.push({data: [searchQueryItem as AutocompleteListItem], sectionIndex: sectionIndex++}); | ||
| } | ||
| if (searchQueryItem) { | ||
| pushSection({data: [searchQueryItem as AutocompleteListItem], sectionIndex: sectionIndex++}); | ||
| } | ||
|
|
||
| const additionalSections = getAdditionalSections?.(searchOptions, sectionIndex); | ||
| const additionalSections = getAdditionalSections?.(searchOptions, sectionIndex); | ||
|
|
||
| if (additionalSections) { | ||
| for (const section of additionalSections) { | ||
| sections.push(section); | ||
| sectionIndex++; | ||
| if (additionalSections) { | ||
| for (const section of additionalSections) { | ||
| pushSection(section); | ||
| sectionIndex++; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (!autocompleteQueryValue && recentSearchesData && recentSearchesData.length > 0) { | ||
| sections.push({title: translate('search.recentSearches'), data: recentSearchesData as AutocompleteListItem[], sectionIndex: sectionIndex++}); | ||
| } | ||
| const styledRecentReports = recentReportsOptions.map((option) => { | ||
| const report = getReportOrDraftReport(option.reportID); | ||
| const reportAction = getReportAction(report?.parentReportID, report?.parentReportActionID); | ||
| const shouldParserToHTML = reportAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT; | ||
| const keyForList = option.keyForList ?? option.reportID ?? (option.accountID ? String(option.accountID) : undefined); | ||
| return { | ||
| ...option, | ||
| keyForList, | ||
| pressableStyle: styles.br2, | ||
| text: StringUtils.lineBreaksToSpaces(shouldParserToHTML ? Parser.htmlToText(option.text ?? '') : (option.text ?? '')), | ||
| wrapperStyle: [styles.pr3, styles.pl3], | ||
| } as AutocompleteListItem; | ||
| }); | ||
|
|
||
| sections.push({title: autocompleteQueryValue.trim() === '' ? translate('search.recentChats') : undefined, data: styledRecentReports, sectionIndex: sectionIndex++}); | ||
| if (!autocompleteQueryValue && recentSearchesData && recentSearchesData.length > 0) { | ||
| pushSection({title: translate('search.recentSearches'), data: recentSearchesData as AutocompleteListItem[], sectionIndex: sectionIndex++}); | ||
| } | ||
|
|
||
| if (autocompleteSuggestions.length > 0) { | ||
| const autocompleteData: AutocompleteListItem[] = autocompleteSuggestions.map(({filterKey, text, autocompleteID, mapKey}) => { | ||
| const nextStyledRecentReports = recentReportsOptions.map((option) => { | ||
| const report = getReportOrDraftReport(option.reportID); | ||
| const reportAction = getReportAction(report?.parentReportID, report?.parentReportActionID); | ||
| const shouldParserToHTML = reportAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT; | ||
| const keyForList = option.keyForList ?? option.reportID ?? (option.accountID ? String(option.accountID) : undefined); | ||
| return { | ||
| text: getAutocompleteDisplayText(filterKey, text), | ||
| mapKey: mapKey ? getSubstitutionMapKey(mapKey, text) : undefined, | ||
| singleIcon: expensifyIcons.MagnifyingGlass, | ||
| searchQuery: text, | ||
| autocompleteID, | ||
| keyForList: autocompleteID ?? text, // in case we have a unique identifier then use it because text might not be unique | ||
| searchItemType: CONST.SEARCH.SEARCH_ROUTER_ITEM_TYPE.AUTOCOMPLETE_SUGGESTION, | ||
| }; | ||
| ...option, | ||
| keyForList, | ||
| pressableStyle: styles.br2, | ||
| text: StringUtils.lineBreaksToSpaces(shouldParserToHTML ? Parser.htmlToText(option.text ?? '') : (option.text ?? '')), | ||
| wrapperStyle: [styles.pr3, styles.pl3], | ||
| } as AutocompleteListItem; | ||
| }); | ||
|
|
||
| sections.push({title: translate('search.suggestions'), data: autocompleteData, sectionIndex: sectionIndex++}); | ||
| } | ||
| pushSection({ | ||
| title: autocompleteQueryValue.trim() === '' ? translate('search.recentChats') : undefined, | ||
| data: nextStyledRecentReports, | ||
| sectionIndex: sectionIndex++, | ||
| }); | ||
|
|
||
| if (autocompleteSuggestions.length > 0) { | ||
| const autocompleteData: AutocompleteListItem[] = autocompleteSuggestions.map(({filterKey, text, autocompleteID, mapKey}) => { | ||
| return { | ||
| text: getAutocompleteDisplayText(filterKey, text), | ||
| mapKey: mapKey ? getSubstitutionMapKey(mapKey, text) : undefined, | ||
| singleIcon: expensifyIcons.MagnifyingGlass, | ||
| searchQuery: text, | ||
| autocompleteID, | ||
| keyForList: autocompleteID ?? text, // in case we have a unique identifier then use it because text might not be unique | ||
| searchItemType: CONST.SEARCH.SEARCH_ROUTER_ITEM_TYPE.AUTOCOMPLETE_SUGGESTION, | ||
| }; | ||
| }); | ||
|
|
||
| pushSection({title: translate('search.suggestions'), data: autocompleteData, sectionIndex: sectionIndex++}); | ||
| } | ||
|
|
||
| return {sections: nextSections, styledRecentReports: nextStyledRecentReports, suggestionsCount: nextSuggestionsCount}; | ||
| }, [autocompleteQueryValue, autocompleteSuggestions, expensifyIcons, getAdditionalSections, recentReportsOptions, recentSearchesData, searchOptions, searchQueryItem, styles, translate]); | ||
|
|
||
| const sectionItemText = sections?.at(1)?.data?.[0]?.text ?? ''; | ||
| const normalizedReferenceText = sectionItemText.toLowerCase(); | ||
| const trimmedAutocompleteQueryValue = autocompleteQueryValue.trim(); | ||
| const isLoading = !isRecentSearchesDataLoaded || !areOptionsInitialized; | ||
| const suggestionsAnnouncement = suggestionsCount > 0 ? translate('search.suggestionsAvailable', {count: suggestionsCount}, trimmedAutocompleteQueryValue) : ''; | ||
| useDebouncedAccessibilityAnnouncement(suggestionsAnnouncement, !!suggestionsAnnouncement, autocompleteQueryValue); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should 3rd param be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That third param is only used as the debounce signal in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no announcement when no results found. Should we also fix this or out of scope?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure we fixed this on main, let's not undo that
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Which PR?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will fix it btw, feel free to implement if you want
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checking
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated (with isLoading check as well since it shouldn't be announced when results are loading)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aimane-chnaif Can you verify?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified |
||
|
|
||
| const firstRecentReportKey = styledRecentReports.at(0)?.keyForList; | ||
| const noResultsFoundText = translate('common.noResultsFound'); | ||
| const shouldAnnounceNoResults = !isLoading && suggestionsCount === 0 && !!trimmedAutocompleteQueryValue; | ||
| useDebouncedAccessibilityAnnouncement(noResultsFoundText, shouldAnnounceNoResults, autocompleteQueryValue); | ||
|
|
||
| // When options initialize after the list is already mounted, initiallyFocusedItemKey has no effect | ||
| // because useState(initialFocusedIndex) in useArrowKeyFocusManager only reads the initial value. | ||
| // Imperatively focus the first recent report once options become available (desktop only). | ||
| useEffect(() => { | ||
| if (shouldUseNarrowLayout || !areOptionsInitialized || hasSetInitialFocusRef.current || !firstRecentReportKey) { | ||
| return; | ||
| } | ||
| hasSetInitialFocusRef.current = true; | ||
|
|
||
| // Compute the flat index of firstRecentReportKey by replicating the flattening logic | ||
| // from useFlattenedSections: each section may prepend a header row when it has a title/customHeader. | ||
| const firstRecentReportKey = styledRecentReports.at(0)?.keyForList; | ||
| let firstRecentReportFlatIndex = -1; | ||
| if (firstRecentReportKey) { | ||
| let flatIndex = 0; | ||
| for (const section of sections) { | ||
| const hasData = (section.data?.length ?? 0) > 0; | ||
|
|
@@ -428,13 +442,28 @@ function SearchAutocompleteList({ | |
| } | ||
| for (const item of section.data ?? []) { | ||
| if (item.keyForList === firstRecentReportKey) { | ||
| innerListRef.current?.updateAndScrollToFocusedIndex(flatIndex, false); | ||
| return; | ||
| firstRecentReportFlatIndex = flatIndex; | ||
| break; | ||
| } | ||
| flatIndex++; | ||
| } | ||
| if (firstRecentReportFlatIndex !== -1) { | ||
| break; | ||
| } | ||
| } | ||
| }, [areOptionsInitialized, firstRecentReportKey, sections, shouldUseNarrowLayout]); | ||
| } | ||
|
|
||
| // When options initialize after the list is already mounted, initiallyFocusedItemKey has no effect | ||
| // because useState(initialFocusedIndex) in useArrowKeyFocusManager only reads the initial value. | ||
| // Imperatively focus the first recent report once options become available (desktop only). | ||
| useEffect(() => { | ||
| if (shouldUseNarrowLayout || !areOptionsInitialized || hasSetInitialFocusRef.current || firstRecentReportFlatIndex === -1) { | ||
| return; | ||
| } | ||
| hasSetInitialFocusRef.current = true; | ||
|
|
||
| innerListRef.current?.updateAndScrollToFocusedIndex(firstRecentReportFlatIndex, false); | ||
| }, [areOptionsInitialized, firstRecentReportFlatIndex, shouldUseNarrowLayout]); | ||
|
|
||
| useEffect(() => { | ||
| const targetText = autocompleteQueryValue; | ||
|
|
@@ -444,8 +473,6 @@ function SearchAutocompleteList({ | |
| } | ||
| }, [autocompleteQueryValue, onHighlightFirstItem, normalizedReferenceText]); | ||
|
|
||
| const isLoading = !isRecentSearchesDataLoaded || !areOptionsInitialized; | ||
|
|
||
| const reasonAttributes: SkeletonSpanReasonAttributes = { | ||
| context: 'SearchAutocompleteList', | ||
| isRecentSearchesDataLoaded, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -293,7 +293,7 @@ function BaseSelectionListWithSections<TItem extends ListItem>({ | |
| accessibilityLabel={textInputOptions?.label} | ||
| options={textInputOptions} | ||
| onSubmit={selectFocusedItem} | ||
| dataLength={flattenedData.length} | ||
| dataLength={itemsCount} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's this change for?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is to keep the announced count aligned with actual suggestion rows. |
||
| isLoading={isLoadingNewOptions} | ||
| onFocusChange={(v: boolean) => (isTextInputFocusedRef.current = v)} | ||
| shouldShowLoadingPlaceholder={shouldShowLoadingPlaceholder} | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ CLEAN-REACT-PATTERNS-0 (docs)
React Compiler is enabled and will automatically memoize this pure computation.
firstRecentReportFlatIndexis derived fromfirstRecentReportKeyandsectionswith no side effects -- the compiler handles this without manualuseMemo.Remove the
useMemoand compute the value directly:Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.