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

Make sure the first result in most recents is highlighted when user uses CMD+K #52298

Merged
merged 19 commits into from
Nov 28, 2024
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
60 changes: 35 additions & 25 deletions src/components/Search/SearchRouter/SearchRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type Report from '@src/types/onyx/Report';
import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import {getQueryWithSubstitutions} from './getQueryWithSubstitutions';
import type {SubstitutionMap} from './getQueryWithSubstitutions';
import {getUpdatedSubstitutionsMap} from './getUpdatedSubstitutionsMap';
Expand Down Expand Up @@ -75,15 +76,15 @@ function SearchRouter({onRouterClose, shouldHideInputCaret}: SearchRouterProps)
const styles = useThemeStyles();
const {translate} = useLocalize();
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [recentSearches] = useOnyx(ONYXKEYS.RECENT_SEARCHES);
const [recentSearches, recentSearchesMetadata] = useOnyx(ONYXKEYS.RECENT_SEARCHES);
const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});
const [autocompleteSubstitutions, setAutocompleteSubstitutions] = useState<SubstitutionMap>({});

const personalDetails = usePersonalDetails();
const [reports = {}] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const taxRates = getAllTaxRates();

const {shouldUseNarrowLayout} = useResponsiveLayout();
const {shouldUseNarrowLayout, isLargeScreenWidth} = useResponsiveLayout();
const listRef = useRef<SelectionListHandle>(null);

// The actual input text that the user sees
Expand Down Expand Up @@ -310,6 +311,10 @@ function SearchRouter({onRouterClose, shouldHideInputCaret}: SearchRouterProps)

const modalWidth = shouldUseNarrowLayout ? styles.w100 : {width: variables.searchRouterPopoverWidth};

const isDataLoaded = useMemo(() => {
return (!contextualReportID || contextualReportID !== undefined) && !isLoadingOnyxValue(recentSearchesMetadata) && recentReports.length > 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: I think we don't need contextualReportID condition here bcoz currently also that condition truthy for any value.

Suggested change
return (!contextualReportID || contextualReportID !== undefined) && !isLoadingOnyxValue(recentSearchesMetadata) && recentReports.length > 0;
return !isLoadingOnyxValue(recentSearchesMetadata) && recentReports.length > 0;

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a NAB? We should never merge code like this @Pujan92 😒

Copy link
Contributor

Choose a reason for hiding this comment

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

It shouldn't be NAB, my bad 😢

}, [contextualReportID, recentSearchesMetadata, recentReports]);

return (
<View
style={[styles.flex1, modalWidth, styles.h100, !shouldUseNarrowLayout && styles.mh85vh]}
Expand All @@ -321,29 +326,34 @@ function SearchRouter({onRouterClose, shouldHideInputCaret}: SearchRouterProps)
onBackButtonPress={() => onRouterClose()}
/>
)}
<SearchRouterInput
value={textInputValue}
isFullWidth={shouldUseNarrowLayout}
onSearchQueryChange={onSearchQueryChange}
onSubmit={() => {
submitSearch(textInputValue);
}}
caretHidden={shouldHideInputCaret}
routerListRef={listRef}
shouldShowOfflineMessage
wrapperStyle={[styles.border, styles.alignItemsCenter]}
outerWrapperStyle={[shouldUseNarrowLayout ? styles.mv3 : styles.mv2, shouldUseNarrowLayout ? styles.mh5 : styles.mh2]}
wrapperFocusedStyle={[styles.borderColorFocus]}
isSearchingForReports={isSearchingForReports}
/>
<SearchRouterList
autocompleteQueryValue={autocompleteQueryValue}
searchQueryItem={searchQueryItem}
additionalSections={sections}
onListItemPress={onListItemPress}
onListItemFocus={onListItemFocus}
ref={listRef}
/>
{(isDataLoaded || !!debouncedInputValue) && (
nyomanjyotisa marked this conversation as resolved.
Show resolved Hide resolved
<>
<SearchRouterInput
value={textInputValue}
isFullWidth={shouldUseNarrowLayout}
onSearchQueryChange={onSearchQueryChange}
onSubmit={() => {
submitSearch(textInputValue);
}}
caretHidden={shouldHideInputCaret}
routerListRef={listRef}
shouldShowOfflineMessage
wrapperStyle={[styles.border, styles.alignItemsCenter]}
outerWrapperStyle={[shouldUseNarrowLayout ? styles.mv3 : styles.mv2, shouldUseNarrowLayout ? styles.mh5 : styles.mh2]}
wrapperFocusedStyle={[styles.borderColorFocus]}
isSearchingForReports={isSearchingForReports}
/>
<SearchRouterList
autocompleteQueryValue={autocompleteQueryValue}
searchQueryItem={searchQueryItem}
additionalSections={sections}
onListItemPress={onListItemPress}
onListItemFocus={onListItemFocus}
initiallyFocusedOptionKey={isLargeScreenWidth ? styledRecentReports.at(0)?.keyForList : undefined}
ref={listRef}
/>
</>
)}
</View>
);
}
Expand Down
16 changes: 13 additions & 3 deletions src/components/Search/SearchRouter/SearchRouterList.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Str} from 'expensify-common';
import React, {forwardRef, useMemo} from 'react';
import React, {forwardRef, useMemo, useState} from 'react';
import type {ForwardedRef} from 'react';
import {useOnyx} from 'react-native-onyx';
import * as Expensicons from '@components/Icon/Expensicons';
Expand Down Expand Up @@ -58,6 +58,9 @@ type SearchRouterListProps = {

/** Callback to call when an item is focused via arrow buttons */
onListItemFocus: (item: SearchQueryItem) => void;

/** Item `keyForList` to focus initially */
initiallyFocusedOptionKey?: string | null;
};

const defaultListOptions = {
Expand Down Expand Up @@ -106,7 +109,7 @@ function SearchRouterItem(props: UserListItemProps<OptionData> | SearchQueryList

// Todo rename to SearchAutocompleteList once it's used in both Router and SearchPage
function SearchRouterList(
{autocompleteQueryValue, searchQueryItem, additionalSections, shouldPreventDefault = true, onListItemFocus, onListItemPress}: SearchRouterListProps,
{autocompleteQueryValue, searchQueryItem, additionalSections, shouldPreventDefault = true, onListItemFocus, onListItemPress, initiallyFocusedOptionKey}: SearchRouterListProps,
ref: ForwardedRef<SelectionListHandle>,
) {
const styles = useThemeStyles();
Expand Down Expand Up @@ -345,6 +348,8 @@ function SearchRouterList(

const sections: Array<SectionListDataType<OptionData | SearchQueryItem>> = [];

const [isInitialRender, setIsInitialRender] = useState(true);

if (searchQueryItem) {
sections.push({data: [searchQueryItem]});
}
Expand Down Expand Up @@ -385,13 +390,18 @@ function SearchRouterList(
sectionListStyle={[shouldUseNarrowLayout ? styles.ph5 : styles.ph2, styles.pb2]}
listItemWrapperStyle={[styles.pr0, styles.pl0]}
getItemHeight={getItemHeight}
onLayout={setPerformanceTimersEnd}
onLayout={() => {
setPerformanceTimersEnd();
setIsInitialRender(false);
}}
showScrollIndicator={!shouldUseNarrowLayout}
sectionTitleStyles={styles.mhn2}
shouldSingleExecuteRowSelect
onArrowFocus={onArrowFocus}
shouldPreventDefault={shouldPreventDefault}
ref={ref}
initiallyFocusedOptionKey={initiallyFocusedOptionKey}
shouldScrollToFocusedIndex={!isInitialRender}
/>
);
}
Expand Down
11 changes: 8 additions & 3 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ function BaseSelectionList<TItem extends ListItem>(
shouldKeepFocusedItemAtTopOfViewableArea = false,
shouldDebounceScrolling = false,
shouldPreventActiveCellVirtualization = false,
shouldScrollToFocusedIndex = true,
}: BaseSelectionListProps<TItem>,
ref: ForwardedRef<SelectionListHandle>,
) {
Expand Down Expand Up @@ -322,7 +323,9 @@ function BaseSelectionList<TItem extends ListItem>(
if (focusedItem) {
onArrowFocus(focusedItem);
}
(shouldDebounceScrolling ? debouncedScrollToIndex : scrollToIndex)(index, true);
if (shouldScrollToFocusedIndex) {
(shouldDebounceScrolling ? debouncedScrollToIndex : scrollToIndex)(index, true);
}
},
isFocused,
});
Expand Down Expand Up @@ -591,10 +594,12 @@ function BaseSelectionList<TItem extends ListItem>(
if (!isInitialSectionListRender) {
return;
}
scrollToIndex(focusedIndex, false);
if (shouldScrollToFocusedIndex) {
scrollToIndex(focusedIndex, false);
}
setIsInitialSectionListRender(false);
},
[focusedIndex, isInitialSectionListRender, scrollToIndex, shouldUseDynamicMaxToRenderPerBatch],
[focusedIndex, isInitialSectionListRender, scrollToIndex, shouldUseDynamicMaxToRenderPerBatch, shouldScrollToFocusedIndex],
);

const onSectionListLayout = useCallback(
Expand Down
3 changes: 3 additions & 0 deletions src/components/SelectionList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,9 @@ type BaseSelectionListProps<TItem extends ListItem> = Partial<ChildrenProps> & {

/** Whether to prevent the active cell from being virtualized and losing focus in browsers */
shouldPreventActiveCellVirtualization?: boolean;

/** Whether to scroll to the focused index */
shouldScrollToFocusedIndex?: boolean;
} & TRightHandSideComponent<TItem>;

type SelectionListHandle = {
Expand Down