-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Search v2] Add displaying advanced filter values and type/status #46022
Changes from 7 commits
346a55c
22693d4
9927d4a
422d75c
9aa2ffb
e373c47
c08eec0
173b619
b7387c0
5efe081
1496f73
2c95ef2
153102f
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,54 +1,102 @@ | ||||||||
import {Str} from 'expensify-common'; | ||||||||
import React, {useMemo} from 'react'; | ||||||||
import {View} from 'react-native'; | ||||||||
import {useOnyx} from 'react-native-onyx'; | ||||||||
import type {ValueOf} from 'type-fest'; | ||||||||
import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton'; | ||||||||
import type {LocaleContextProps} from '@components/LocaleContextProvider'; | ||||||||
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription'; | ||||||||
import useLocalize from '@hooks/useLocalize'; | ||||||||
import useSingleExecution from '@hooks/useSingleExecution'; | ||||||||
import useThemeStyles from '@hooks/useThemeStyles'; | ||||||||
import useWaitForNavigation from '@hooks/useWaitForNavigation'; | ||||||||
import Navigation from '@libs/Navigation/Navigation'; | ||||||||
import * as SearchActions from '@userActions/Search'; | ||||||||
import ONYXKEYS from '@src/ONYXKEYS'; | ||||||||
import ROUTES from '@src/ROUTES'; | ||||||||
import type {SearchAdvancedFiltersForm} from '@src/types/form'; | ||||||||
import type INPUT_IDS from '@src/types/form/SearchAdvancedFiltersForm'; | ||||||||
|
||||||||
function getFilterDisplayTitle(filters: Record<string, string>, fieldName: string) { | ||||||||
// This is temporary because the full parsing of search query is not yet done | ||||||||
// TODO once we have values from query, this value should be `filters[fieldName].value` | ||||||||
return fieldName; | ||||||||
// the values of dateBefore+dateAfter map to just a single 'date' field on advanced filters | ||||||||
type AvailableFilters = ValueOf<typeof INPUT_IDS> | 'date'; | ||||||||
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 think it's better if we define the available filter names as constants in the CONST file, like this: in
Then, use 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. So you suggest I have both the Btw this is most likely the only case where there are 2 inputs that map to 1 filter, because the actual filter will be 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. Maybe we can define it as follows: const FIELDS_NAMES = Object.entries(INPUT_IDS).reduce<Record<string, string>>((fields, [key, value]) => {
if (key === INPUT_IDS.DATE_AFTER || key === INPUT_IDS.DATE_BEFORE) {
fields.DATE = 'date';
} else {
fields[key] = value;
}
return fields;
}, {});
type AvailableFilters = ValueOf<typeof FIELDS_NAMES>; My concern here is to avoid using hardcoded values when referencing 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. @rayane-djouah after we merged the other PR with new query/syntax - CONSTs were modified, and now I can use better more fitting consts specific to query here. |
||||||||
|
||||||||
function getFilterDisplayTitle(filters: Partial<SearchAdvancedFiltersForm>, fieldName: AvailableFilters, translate: LocaleContextProps['translate']) { | ||||||||
if (fieldName === 'date') { | ||||||||
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.
Suggested change
|
||||||||
const {dateAfter, dateBefore} = filters; | ||||||||
let dateValue = ''; | ||||||||
if (dateBefore) { | ||||||||
dateValue = translate('search.filters.date.before', dateBefore); | ||||||||
} | ||||||||
if (dateBefore && dateAfter) { | ||||||||
dateValue += ', '; | ||||||||
} | ||||||||
if (dateAfter) { | ||||||||
dateValue += translate('search.filters.date.after', dateAfter); | ||||||||
} | ||||||||
|
||||||||
return dateValue; | ||||||||
} | ||||||||
|
||||||||
const filterValue = filters[fieldName]; | ||||||||
return filterValue ? Str.recapitalize(filterValue) : undefined; | ||||||||
} | ||||||||
|
||||||||
function AdvancedSearchFilters() { | ||||||||
const {translate} = useLocalize(); | ||||||||
const styles = useThemeStyles(); | ||||||||
const {singleExecution} = useSingleExecution(); | ||||||||
const waitForNavigate = useWaitForNavigation(); | ||||||||
|
||||||||
const [searchAdvancedFilters = {}] = useOnyx(ONYXKEYS.FORMS.SEARCH_ADVANCED_FILTERS_FORM); | ||||||||
|
||||||||
const advancedFilters = useMemo( | ||||||||
() => [ | ||||||||
{ | ||||||||
title: getFilterDisplayTitle({}, 'title'), | ||||||||
title: getFilterDisplayTitle(searchAdvancedFilters, 'type', translate), | ||||||||
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. For example, here we can use |
||||||||
description: 'common.type' as const, | ||||||||
route: ROUTES.SEARCH_ADVANCED_FILTERS_TYPE, | ||||||||
}, | ||||||||
{ | ||||||||
title: getFilterDisplayTitle({}, 'date'), | ||||||||
title: getFilterDisplayTitle(searchAdvancedFilters, 'date', translate), | ||||||||
description: 'common.date' as const, | ||||||||
route: ROUTES.SEARCH_ADVANCED_FILTERS_DATE, | ||||||||
}, | ||||||||
{ | ||||||||
title: getFilterDisplayTitle(searchAdvancedFilters, 'status', translate), | ||||||||
description: 'search.filters.status' as const, | ||||||||
route: ROUTES.SEARCH_ADVANCED_FILTERS_STATUS, | ||||||||
}, | ||||||||
], | ||||||||
[], | ||||||||
[searchAdvancedFilters, translate], | ||||||||
); | ||||||||
|
||||||||
return ( | ||||||||
<View> | ||||||||
{advancedFilters.map((item) => { | ||||||||
const onPress = singleExecution(waitForNavigate(() => Navigation.navigate(item.route))); | ||||||||
|
||||||||
return ( | ||||||||
<MenuItemWithTopDescription | ||||||||
key={item.description} | ||||||||
title={item.title} | ||||||||
description={translate(item.description)} | ||||||||
shouldShowRightIcon | ||||||||
onPress={onPress} | ||||||||
/> | ||||||||
); | ||||||||
})} | ||||||||
<View style={[styles.flex1, styles.justifyContentBetween]}> | ||||||||
<View> | ||||||||
{advancedFilters.map((item) => { | ||||||||
const onPress = singleExecution(waitForNavigate(() => Navigation.navigate(item.route))); | ||||||||
|
||||||||
return ( | ||||||||
<MenuItemWithTopDescription | ||||||||
key={item.description} | ||||||||
title={item.title} | ||||||||
description={translate(item.description)} | ||||||||
shouldShowRightIcon | ||||||||
onPress={onPress} | ||||||||
/> | ||||||||
); | ||||||||
})} | ||||||||
</View> | ||||||||
<FormAlertWithSubmitButton | ||||||||
buttonText={translate('search.viewResults')} | ||||||||
containerStyles={[styles.m4]} | ||||||||
onSubmit={() => { | ||||||||
// here set the selected filters as new query and redirect to SearchResults page | ||||||||
// waiting for: https://github.com/Expensify/App/issues/45028 and https://github.com/Expensify/App/issues/45027 | ||||||||
SearchActions.clearAdvancedFilters(); | ||||||||
Navigation.goBack(); | ||||||||
Kicu marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
}} | ||||||||
/> | ||||||||
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 think that the "View results" button should not be disabled offline. based on OfflineUX_Patterns_Flowchart, No offline pattern is needed here.
Suggested change
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 think we can keep it as is for now so that we don't block other issues, but I think we should disable it offline because the Search API command only works while the user is online. |
||||||||
</View> | ||||||||
); | ||||||||
} | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import React, {useMemo} from 'react'; | ||
import {View} from 'react-native'; | ||
import {useOnyx} from 'react-native-onyx'; | ||
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView'; | ||
import type {FormOnyxValues} from '@components/Form/types'; | ||
import HeaderWithBackButton from '@components/HeaderWithBackButton'; | ||
import ScreenWrapper from '@components/ScreenWrapper'; | ||
import SelectionList from '@components/SelectionList'; | ||
import RadioListItem from '@components/SelectionList/RadioListItem'; | ||
import useLocalize from '@hooks/useLocalize'; | ||
import useThemeStyles from '@hooks/useThemeStyles'; | ||
import Navigation from '@navigation/Navigation'; | ||
import * as SearchActions from '@userActions/Search'; | ||
import CONST from '@src/CONST'; | ||
import ONYXKEYS from '@src/ONYXKEYS'; | ||
|
||
function SearchFiltersStatusPage() { | ||
const styles = useThemeStyles(); | ||
const {translate} = useLocalize(); | ||
|
||
const [searchAdvancedFiltersForm] = useOnyx(ONYXKEYS.FORMS.SEARCH_ADVANCED_FILTERS_FORM); | ||
|
||
const activeItem = searchAdvancedFiltersForm?.status; | ||
|
||
const filterStatusItems = useMemo( | ||
() => [ | ||
{ | ||
text: translate('common.all'), | ||
value: CONST.SEARCH.TAB.ALL, | ||
keyForList: CONST.SEARCH.TAB.ALL, | ||
Kicu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
isSelected: activeItem === CONST.SEARCH.TAB.ALL, | ||
}, | ||
{ | ||
text: translate('common.shared'), | ||
value: CONST.SEARCH.TAB.SHARED, | ||
keyForList: CONST.SEARCH.TAB.SHARED, | ||
isSelected: activeItem === CONST.SEARCH.TAB.SHARED, | ||
}, | ||
{ | ||
text: translate('common.drafts'), | ||
value: CONST.SEARCH.TAB.DRAFTS, | ||
keyForList: CONST.SEARCH.TAB.DRAFTS, | ||
isSelected: activeItem === CONST.SEARCH.TAB.DRAFTS, | ||
}, | ||
{ | ||
text: translate('common.finished'), | ||
value: CONST.SEARCH.TAB.FINISHED, | ||
keyForList: CONST.SEARCH.TAB.FINISHED, | ||
isSelected: activeItem === CONST.SEARCH.TAB.FINISHED, | ||
}, | ||
], | ||
[translate, activeItem], | ||
); | ||
|
||
const updateStatus = (values: Partial<FormOnyxValues<typeof ONYXKEYS.FORMS.SEARCH_ADVANCED_FILTERS_FORM>>) => { | ||
SearchActions.updateAdvancedFilters(values); | ||
Navigation.goBack(); | ||
Kicu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
return ( | ||
<ScreenWrapper | ||
testID={SearchFiltersStatusPage.displayName} | ||
shouldShowOfflineIndicatorInWideScreen | ||
offlineIndicatorStyle={styles.mtAuto} | ||
> | ||
<FullPageNotFoundView shouldShow={false}> | ||
<HeaderWithBackButton title={translate('search.filters.status')} /> | ||
<View style={[styles.flex1]}> | ||
<SelectionList | ||
sections={[{data: filterStatusItems}]} | ||
onSelectRow={(item) => { | ||
updateStatus({ | ||
status: item.value, | ||
}); | ||
}} | ||
initiallyFocusedOptionKey={activeItem} | ||
shouldStopPropagation | ||
ListItem={RadioListItem} | ||
/> | ||
</View> | ||
</FullPageNotFoundView> | ||
</ScreenWrapper> | ||
); | ||
} | ||
|
||
SearchFiltersStatusPage.displayName = 'SearchFiltersStatusPage'; | ||
|
||
export default SearchFiltersStatusPage; |
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.
NAB we should eventually move this to
CONST.SEARCH.DATA_TYPES.EXPENSE
and get rid of transactions and reports. We'll do so as part of v2.2.