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

Add recent serches and recent chats in Search router #49457

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0acddec
add SingleIconListItem
SzymczakJ Sep 17, 2024
682a8ec
add SearchRouterList
SzymczakJ Sep 17, 2024
386a8f7
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Sep 18, 2024
5ed32cd
add find section to SearchRouterList
SzymczakJ Sep 18, 2024
0b3fe4b
add onSelectRow action
SzymczakJ Sep 19, 2024
3799068
clean up code
SzymczakJ Sep 19, 2024
2f3b0a5
add contextual search
SzymczakJ Sep 20, 2024
80370ca
fix big screen styling
SzymczakJ Sep 20, 2024
50d4e78
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Sep 20, 2024
17cda40
fix pr comments
SzymczakJ Sep 24, 2024
7df4c32
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Sep 24, 2024
48a4296
change contextual search logic
SzymczakJ Sep 25, 2024
96e309a
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Sep 25, 2024
445a031
fix linter
SzymczakJ Sep 25, 2024
6e4d2b2
fix Enter shortcut logic
SzymczakJ Sep 26, 2024
7e509d5
fix SearchRouterList types
SzymczakJ Sep 26, 2024
4e439a2
fix SearchQueryListItem
SzymczakJ Sep 26, 2024
38ba637
fix typescript errors
SzymczakJ Sep 26, 2024
e9cba4e
fix styling
SzymczakJ Sep 26, 2024
cf57d6e
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Sep 26, 2024
ab3109a
fix iOS
SzymczakJ Sep 27, 2024
48510b5
limit recentSearches amountto 5
SzymczakJ Sep 27, 2024
fb0fc7f
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Sep 27, 2024
26e9026
fix PR comments
SzymczakJ Sep 27, 2024
e5ae868
add recent chat filtering logic
SzymczakJ Sep 30, 2024
b5f33da
fix PR coments
SzymczakJ Oct 1, 2024
7b111e8
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Oct 1, 2024
66fa714
fix linter
SzymczakJ Oct 1, 2024
11525fa
fix PR comments
SzymczakJ Oct 1, 2024
994e17f
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Oct 2, 2024
1b7f6ac
fix styles
SzymczakJ Oct 2, 2024
a986ab6
fix searchRouterLoading bug
SzymczakJ Oct 2, 2024
3b9e946
Merge branch 'main' into @szymczak/serach-router-list
SzymczakJ Oct 2, 2024
524eb8b
focus list item if it exists
SzymczakJ Oct 2, 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
4 changes: 4 additions & 0 deletions src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,9 @@ const ONYXKEYS = {
/** Stores the information about the saved searches */
SAVED_SEARCHES: 'nvp_savedSearches',

/** Stores the information about the recent searches */
RECENT_SEARCHES: 'nvp_recentSearches',

/** Stores recently used currencies */
RECENTLY_USED_CURRENCIES: 'nvp_recentlyUsedCurrencies',

Expand Down Expand Up @@ -850,6 +853,7 @@ type OnyxValuesMapping = {

// ONYXKEYS.NVP_TRYNEWDOT is HybridApp onboarding data
[ONYXKEYS.NVP_TRYNEWDOT]: OnyxTypes.TryNewDot;
[ONYXKEYS.RECENT_SEARCHES]: Record<string, OnyxTypes.RecentSearchItem>;
[ONYXKEYS.SAVED_SEARCHES]: OnyxTypes.SaveSearch;
[ONYXKEYS.RECENTLY_USED_CURRENCIES]: string[];
[ONYXKEYS.ACTIVE_CLIENTS]: string[];
Expand Down
5 changes: 4 additions & 1 deletion src/components/Search/SearchPageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,15 @@ function HeaderWrapper({icon, children, text, isCannedQuery}: HeaderWrapperProps
) : (
<View style={styles.pr5}>
<SearchRouterInput
value={text}
setValue={() => {}}
updateSearch={() => {}}
disabled
isFullWidth
wrapperStyle={[styles.searchRouterInputResults, styles.br2]}
wrapperFocusedStyle={styles.searchRouterInputResultsFocused}
defaultValue={text}
rightComponent={children}
routerListRef={undefined}
/>
</View>
)}
Expand Down
177 changes: 147 additions & 30 deletions src/components/Search/SearchRouter/SearchRouter.tsx
Original file line number Diff line number Diff line change
@@ -1,71 +1,167 @@
import {useNavigationState} from '@react-navigation/native';
import debounce from 'lodash/debounce';
import React, {useCallback, useState} from 'react';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {View} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import FocusTrapForModal from '@components/FocusTrap/FocusTrapForModal';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Modal from '@components/Modal';
import {useOptionsList} from '@components/OptionListContextProvider';
import type {SearchQueryJSON} from '@components/Search/types';
import type {SelectionListHandle} from '@components/SelectionList/types';
import useDebouncedState from '@hooks/useDebouncedState';
import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useThemeStyles from '@hooks/useThemeStyles';
import Log from '@libs/Log';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import type {OptionData} from '@libs/ReportUtils';
import * as SearchUtils from '@libs/SearchUtils';
import Navigation from '@navigation/Navigation';
import variables from '@styles/variables';
import * as Report from '@userActions/Report';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import {useSearchRouterContext} from './SearchRouterContext';
import SearchRouterInput from './SearchRouterInput';
import SearchRouterList from './SearchRouterList';

const SEARCH_DEBOUNCE_DELAY = 150;

function SearchRouter() {
const styles = useThemeStyles();
const {translate} = useLocalize();
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [recentSearches] = useOnyx(ONYXKEYS.RECENT_SEARCHES);
const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});

const {isSmallScreenWidth} = useResponsiveLayout();
const {isSearchRouterDisplayed, closeSearchRouter} = useSearchRouterContext();
const listRef = useRef<SelectionListHandle>(null);

const [textInputValue, debouncedInputValue, setTextInputValue] = useDebouncedState('', 500);
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB Left a comment about the debounce, but I think we should only debounce the search in the server and nothing else. That'll make the App feel snappier and result in a better experience.

const [userSearchQuery, setUserSearchQuery] = useState<SearchQueryJSON | undefined>(undefined);

const clearUserQuery = () => {
setUserSearchQuery(undefined);
};

const onSearchChange = debounce((userQuery: string) => {
if (!userQuery) {
clearUserQuery();
return;
const contextualReportID = useNavigationState<Record<string, {reportID: string}>, string | undefined>((state) => {
return state?.routes.at(-1)?.params?.reportID;
});
SzymczakJ marked this conversation as resolved.
Show resolved Hide resolved
const sortedRecentSearches = useMemo(() => {
return Object.values(recentSearches ?? {}).sort((a, b) => b.timestamp.localeCompare(a.timestamp));
}, [recentSearches]);

const {options, areOptionsInitialized} = useOptionsList();
const searchOptions = useMemo(() => {
if (!areOptionsInitialized) {
return {recentReports: [], personalDetails: [], userToInvite: null, currentUserOption: null, categoryOptions: [], tagOptions: [], taxRatesOptions: []};
}
return OptionsListUtils.getSearchOptions(options, '', betas ?? []);
SzymczakJ marked this conversation as resolved.
Show resolved Hide resolved
}, [areOptionsInitialized, betas, options]);

const filteredOptions = useMemo(() => {
if (debouncedInputValue.trim() === '') {
return {
recentReports: [],
personalDetails: [],
userToInvite: null,
};
}

const queryJSON = SearchUtils.buildSearchQueryJSON(userQuery);
const newOptions = OptionsListUtils.filterOptions(searchOptions, debouncedInputValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true});

if (queryJSON) {
// eslint-disable-next-line
console.log('parsedQuery', queryJSON);
return {
recentReports: newOptions.recentReports,
personalDetails: newOptions.personalDetails,
userToInvite: newOptions.userToInvite,
};
}, [debouncedInputValue, searchOptions]);

setUserSearchQuery(queryJSON);
} else {
// Handle query parsing error
const recentReports: OptionData[] = useMemo(() => {
const currentSearchOptions = debouncedInputValue === '' ? searchOptions : filteredOptions;
const reports: OptionData[] = [...currentSearchOptions.recentReports, ...currentSearchOptions.personalDetails];
if (currentSearchOptions.userToInvite) {
reports.push(currentSearchOptions.userToInvite);
}
}, SEARCH_DEBOUNCE_DELAY);
return reports.slice(0, 10);
}, [debouncedInputValue, filteredOptions, searchOptions]);

const onSearchSubmit = useCallback(() => {
if (!userSearchQuery) {
useEffect(() => {
Report.searchInServer(debouncedInputValue.trim());
}, [debouncedInputValue]);

useEffect(() => {
if (!textInputValue && isSearchRouterDisplayed) {
return;
}
listRef.current?.updateAndScrollToFocusedIndex(0);
// eslint-disable-next-line react-compiler/react-compiler
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isSearchRouterDisplayed]);

closeSearchRouter();
const contextualReportData = contextualReportID ? searchOptions.recentReports?.find((option) => option.reportID === contextualReportID) : undefined;

const query = SearchUtils.buildSearchQueryString(userSearchQuery);
Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query}));
const clearUserQuery = () => {
setTextInputValue('');
setUserSearchQuery(undefined);
};

const onSearchChange = useMemo(
// eslint-disable-next-line react-compiler/react-compiler
() =>
debounce((userQuery: string) => {
if (!userQuery) {
clearUserQuery();
listRef.current?.updateAndScrollToFocusedIndex(-1);
return;
}
listRef.current?.updateAndScrollToFocusedIndex(0);
const queryJSON = SearchUtils.buildSearchQueryJSON(userQuery);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we try to parse the query on change? I feel like this will often fail because we'll try to parse an incomplete query. Wouldn't the navigation even in onSearchSubmit trigger the parsing/API request of the new search query?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was initially implemented like this by Mateusz. Right now it may not feel like the best idea, but when we will add autocomplete we will have to parse onChange to provide autocomplete suggestions.
I can refactor it to not be parsed on every change but we will have to roll it back when we implement 2.5, so should I do it then? WDYT @luacmartins

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, I think we should refactor it right now. I just started working on the autocomplete doc and I think we might want a different parser for it that only captures the filter key and checks for matches, without building the AST

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already started refactoring it, but now I have second thoughts.
Parser with or without AST will have to parse input on text change to provide suggestions, so after deleting this we will have to add this once more. Also this code is already working on main and is not that heavy performance-wise.
I could delete Log.alert and console.logs so we don't log things on every keystroke and don't clutter the terminal, but I'm highly in favour of leaving this code as it is.

Copy link
Contributor

@Kicu Kicu Sep 30, 2024

Choose a reason for hiding this comment

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

Hey it was me that originally introduce the code that parses after every keypress.
Please note that there is 150ms debounce on this function, so that we do not run it too often.

In general I was feeling that whatever user types in should go through parser as it feels safer and gives us more information about the query.
Yes, its true that right now we have no autocomplete of any kind yet, but it's just easier to work on parsed object.
If you worry about speed and computation - I wouldn't at this point, the parser is very fast and it will run only several times.

Also like @SzymczakJ said - in the long there will be a parser in this place, so that we can display the autocompletes to the user.
Does it really make sense to remove the parsing now only to add it in two weeks?

And the constant alert logs in console are the problem, then we could do either:

  • pass the flag buildSearchQueryJSON to fail silently and not log, false in every other case but this one
  • make buildSearchQueryJSON throw and manually handle parser errors in every place; in here we would simply do nothing

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll end up using a different parser for this that handles just autocomplete. We only really need the query AST when submitting the search to the server right?


if (queryJSON) {
setUserSearchQuery(queryJSON);
} else {
Log.alert(`${CONST.ERROR.ENSURE_BUGBOT} user query failed to parse`, userQuery, false);
}
}, SEARCH_DEBOUNCE_DELAY),
// eslint-disable-next-line react-compiler/react-compiler
// eslint-disable-next-line react-hooks/exhaustive-deps
[],
);

const updateUserSearchQuery = (newSearchQuery: string) => {
setTextInputValue(newSearchQuery);
onSearchChange(newSearchQuery);
};

const closeAndClearRouter = useCallback(() => {
closeSearchRouter();
clearUserQuery();
}, [closeSearchRouter, userSearchQuery]);
// eslint-disable-next-line react-compiler/react-compiler
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [closeSearchRouter]);

const onSearchSubmit = useCallback(
(query: SearchQueryJSON | undefined) => {
if (!query) {
return;
}
closeSearchRouter();
const queryString = SearchUtils.buildSearchQueryString(query);
Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query: queryString}));
clearUserQuery();
},
// eslint-disable-next-line react-compiler/react-compiler
// eslint-disable-next-line react-hooks/exhaustive-deps
[closeSearchRouter],
);

useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ESCAPE, () => {
closeSearchRouter();
clearUserQuery();
});

const modalType = isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.CENTERED : CONST.MODAL.MODAL_TYPE.POPOVER;
const isFullWidth = isSmallScreenWidth;
const modalType = isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.CENTERED_UNSWIPEABLE : CONST.MODAL.MODAL_TYPE.POPOVER;
const modalWidth = isSmallScreenWidth ? styles.w100 : {width: variables.popoverWidth};

return (
<Modal
Expand All @@ -76,11 +172,32 @@ function SearchRouter() {
onClose={closeSearchRouter}
>
<FocusTrapForModal active={isSearchRouterDisplayed}>
<View style={[styles.flex1, styles.p3]}>
<View style={[styles.flex1, modalWidth, styles.h100, !isSmallScreenWidth && styles.mh85vh]}>
{isSmallScreenWidth && (
<HeaderWithBackButton
title={translate('common.search')}
onBackButtonPress={() => closeSearchRouter()}
/>
)}
<SearchRouterInput
isFullWidth={isFullWidth}
onChange={onSearchChange}
onSubmit={onSearchSubmit}
value={textInputValue}
setValue={setTextInputValue}
isFullWidth={isSmallScreenWidth}
updateSearch={onSearchChange}
routerListRef={listRef}
wrapperStyle={[isSmallScreenWidth ? styles.mv3 : styles.mv2, isSmallScreenWidth ? styles.mh5 : styles.mh2, styles.border]}
wrapperFocusedStyle={[styles.borderColorFocus]}
isSearchingForReports={isSearchingForReports}
/>
<SearchRouterList
SzymczakJ marked this conversation as resolved.
Show resolved Hide resolved
currentQuery={userSearchQuery}
reportForContextualSearch={contextualReportData}
recentSearches={sortedRecentSearches?.slice(0, 5)}
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB the length of the object is capped on the backend, so we don't need to slice it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I initially thought, but I got bigger object for like 3 or 4 times 😄, so I did it for safety reasons. Maybe some Onyx methods are merging Objects in a wrong way or something.
After relog the objects were always capped by 5.

SzymczakJ marked this conversation as resolved.
Show resolved Hide resolved
recentReports={recentReports}
onSearchSubmit={onSearchSubmit}
updateUserSearchQuery={updateUserSearchQuery}
closeAndClearRouter={closeAndClearRouter}
ref={listRef}
/>
</View>
</FocusTrapForModal>
Expand Down
60 changes: 43 additions & 17 deletions src/components/Search/SearchRouter/SearchRouterInput.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
import React, {useState} from 'react';
import type {ReactNode} from 'react';
import type {ReactNode, RefObject} from 'react';
import {View} from 'react-native';
import type {StyleProp, ViewStyle} from 'react-native';
import type {SelectionListHandle} from '@components/SelectionList/types';
import TextInput from '@components/TextInput';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import variables from '@styles/variables';
import CONST from '@src/CONST';

type SearchRouterInputProps = {
/** Callback triggered when the input text changes */
onChange?: (searchTerm: string) => void;
/** Value of TextInput */
value: string;

/** Callback invoked when the user submits the input */
onSubmit?: () => void;
/** Setter to TextInput value */
setValue: (searchTerm: string) => void;

/** Callback to update search in SearchRouter */
updateSearch: (searchTerm: string) => void;

/** SearchRouterList ref for managing TextInput and SearchRouterList focus */
routerListRef?: RefObject<SelectionListHandle>;

/** Whether the input is full width */
isFullWidth: boolean;

/** Default value for text input */
defaultValue?: string;

/** Whether the input is disabled */
disabled?: boolean;

Expand All @@ -31,37 +36,58 @@ type SearchRouterInputProps = {

/** Component to be displayed on the right */
rightComponent?: ReactNode;

/** Whether the search reports API call is running */
isSearchingForReports?: boolean;
};

function SearchRouterInput({isFullWidth, onChange, onSubmit, defaultValue = '', disabled = false, wrapperStyle, wrapperFocusedStyle, rightComponent}: SearchRouterInputProps) {
function SearchRouterInput({
value,
setValue,
updateSearch,
routerListRef,
isFullWidth,
disabled = false,
wrapperStyle,
wrapperFocusedStyle,
rightComponent,
isSearchingForReports,
}: SearchRouterInputProps) {
const styles = useThemeStyles();

const [value, setValue] = useState(defaultValue);
const {translate} = useLocalize();
const [isFocused, setIsFocused] = useState<boolean>(false);

const onChangeText = (text: string) => {
setValue(text);
onChange?.(text);
updateSearch(text);
};

const inputWidth = isFullWidth ? styles.w100 : {width: variables.popoverWidth};

return (
<View style={[styles.flexRow, styles.alignItemsCenter, wrapperStyle ?? styles.searchRouterInput, isFocused && wrapperFocusedStyle]}>
<View style={[styles.flexRow, styles.alignItemsCenter, wrapperStyle ?? styles.searchRouterTextInputContainer, isFocused && wrapperFocusedStyle]}>
<View style={styles.flex1}>
<TextInput
Copy link
Contributor

@rayane-djouah rayane-djouah Oct 2, 2024

Choose a reason for hiding this comment

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

I think we might need to pass more properties to TextInput usage here like spellCheck={false}, enterKeyHint="search", and accessibilityLabel={translate('search.searchPlaceholder')}

onFocus={() => (isTextInputFocusedRef.current = true)}
onBlur={() => (isTextInputFocusedRef.current = false)}
label={textInputLabel}
accessibilityLabel={textInputLabel}
hint={textInputHint}
role={CONST.ROLE.PRESENTATION}
value={textInputValue}
placeholder={textInputPlaceholder}
maxLength={textInputMaxLength}
onChangeText={onChangeText}
inputMode={inputMode}
selectTextOnFocus
spellCheck={false}
iconLeft={textInputIconLeft}
onSubmitEditing={selectFocusedOption}
blurOnSubmit={!!flattenedSections.allOptions.length}
isLoading={isLoadingNewOptions}
testID="selection-list-text-input"
shouldInterceptSwipe={shouldTextInputInterceptSwipe}
/>
</View>

autoFocus
value={value}
onChangeText={onChangeText}
onSubmitEditing={onSubmit}
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Unable to submit the search input using the mobile keyboard enter key. We should use onSubmitEditing property to choose the focused option

android_native.mp4

autoFocus
loadingSpinnerStyle={[styles.mt0, styles.mr2]}
role={CONST.ROLE.PRESENTATION}
placeholder={translate('search.searchPlaceholder')}
autoCapitalize="none"
disabled={disabled}
shouldUseDisabledStyles={false}
textInputContainerStyles={styles.borderNone}
inputStyle={[styles.searchInputStyle, inputWidth, styles.pl3, styles.pr3]}
onFocus={() => setIsFocused(true)}
onBlur={() => setIsFocused(false)}
onFocus={() => {
setIsFocused(true);
routerListRef?.current?.updateExternalTextInputFocus(true);
}}
onBlur={() => {
setIsFocused(false);
routerListRef?.current?.updateExternalTextInputFocus(false);
}}
isLoading={!!isSearchingForReports}
/>
</View>
{rightComponent && <View style={styles.pr3}>{rightComponent}</View>}
Expand Down
Loading
Loading