feat/81501: Add Range filter for all date type filters#81897
Open
btkcodedev wants to merge 68 commits intoExpensify:mainfrom
Open
feat/81501: Add Range filter for all date type filters#81897btkcodedev wants to merge 68 commits intoExpensify:mainfrom
btkcodedev wants to merge 68 commits intoExpensify:mainfrom
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [isSearchAdvancedFiltersFormLoading]); | ||
|
|
||
| useEffect(() => { |
Contributor
There was a problem hiding this comment.
❌ PERF-10
Reasoning: Parent-child communication should not use useEffect. Instead, lift the state up to the parent component and pass it down as props. This follows React unidirectional data flow pattern, eliminates synchronization issues, reduces unnecessary renders, and makes the data flow clearer.
Suggested fix: Instead of calling onDateValuesChange in useEffect, call it directly when setDateValues is called in setDateValue:
const setDateValue = useCallback((dateModifier: SearchDateModifier, value: string | undefined) => {
setDateValues((prevDateValues) => {
const newDateValues = {...prevDateValues, [dateModifier]: value};
// Update parent immediately with new values
onDateValuesChange?.(newDateValues);
return newDateValues;
});
}, [onDateValuesChange]);Then remove the useEffect entirely.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
trjExpensify
previously approved these changes
Feb 10, 2026
Contributor
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
Looks pretty good to me. CC: @Expensify/design @JmillsExpensify
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Contributor
|
Good to run a test build to check it out? |
Contributor
Author
|
Approval for Spanish translation commented on Slack: |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
Contributor
Author
|
Comments addressed, please continue review 🙇 |
Contributor
|
@Pujan92 can you complete your review today, please? Thanks! |
Pujan92
reviewed
Feb 25, 2026
Comment on lines
201
to
265
| {useRangeLayout ? ( | ||
| <View style={[styles.flexRow, styles.mh5, styles.alignItemsCenter, styles.pt1]}> | ||
| {shouldShowInlineRangeText ? ( | ||
| <> | ||
| <View style={[styles.flex1, styles.mr2]}> | ||
| {!!rangeText && ( | ||
| <Text style={[styles.textLabelSupporting]}> | ||
| {`${translate('common.range')}: `} | ||
| <Text style={[styles.textLabel]}>{rangeText}</Text> | ||
| </Text> | ||
| )} | ||
| </View> | ||
| <View style={[styles.flex1, styles.ml2]}> | ||
| <View style={[styles.flexRow, styles.gap2]}> | ||
| <Button | ||
| medium | ||
| style={[styles.flex1]} | ||
| text={translate('common.reset')} | ||
| onPress={resetChanges} | ||
| /> | ||
| <Button | ||
| success | ||
| medium | ||
| style={[styles.flex1]} | ||
| text={translate('common.apply')} | ||
| onPress={applyChanges} | ||
| /> | ||
| </View> | ||
| </View> | ||
| </> | ||
| ) : ( | ||
| <View style={[styles.flexRow, styles.gap2, styles.flex1]}> | ||
| <Button | ||
| medium | ||
| style={[styles.flex1]} | ||
| text={translate('common.reset')} | ||
| onPress={resetChanges} | ||
| /> | ||
| <Button | ||
| success | ||
| medium | ||
| style={[styles.flex1]} | ||
| text={translate('common.apply')} | ||
| onPress={applyChanges} | ||
| /> | ||
| </View> | ||
| )} | ||
| </View> | ||
| ) : ( | ||
| <View style={[styles.flexRow, styles.gap2, styles.ph5]}> | ||
| <Button | ||
| medium | ||
| style={[styles.flex1]} | ||
| text={translate('common.reset')} | ||
| onPress={resetChanges} | ||
| /> | ||
| <Button | ||
| success | ||
| medium | ||
| style={[styles.flex1]} | ||
| text={translate('common.apply')} | ||
| onPress={applyChanges} | ||
| /> | ||
| </View> | ||
| )} |
Contributor
There was a problem hiding this comment.
Suggested change
| {useRangeLayout ? ( | |
| <View style={[styles.flexRow, styles.mh5, styles.alignItemsCenter, styles.pt1]}> | |
| {shouldShowInlineRangeText ? ( | |
| <> | |
| <View style={[styles.flex1, styles.mr2]}> | |
| {!!rangeText && ( | |
| <Text style={[styles.textLabelSupporting]}> | |
| {`${translate('common.range')}: `} | |
| <Text style={[styles.textLabel]}>{rangeText}</Text> | |
| </Text> | |
| )} | |
| </View> | |
| <View style={[styles.flex1, styles.ml2]}> | |
| <View style={[styles.flexRow, styles.gap2]}> | |
| <Button | |
| medium | |
| style={[styles.flex1]} | |
| text={translate('common.reset')} | |
| onPress={resetChanges} | |
| /> | |
| <Button | |
| success | |
| medium | |
| style={[styles.flex1]} | |
| text={translate('common.apply')} | |
| onPress={applyChanges} | |
| /> | |
| </View> | |
| </View> | |
| </> | |
| ) : ( | |
| <View style={[styles.flexRow, styles.gap2, styles.flex1]}> | |
| <Button | |
| medium | |
| style={[styles.flex1]} | |
| text={translate('common.reset')} | |
| onPress={resetChanges} | |
| /> | |
| <Button | |
| success | |
| medium | |
| style={[styles.flex1]} | |
| text={translate('common.apply')} | |
| onPress={applyChanges} | |
| /> | |
| </View> | |
| )} | |
| </View> | |
| ) : ( | |
| <View style={[styles.flexRow, styles.gap2, styles.ph5]}> | |
| <Button | |
| medium | |
| style={[styles.flex1]} | |
| text={translate('common.reset')} | |
| onPress={resetChanges} | |
| /> | |
| <Button | |
| success | |
| medium | |
| style={[styles.flex1]} | |
| text={translate('common.apply')} | |
| onPress={applyChanges} | |
| /> | |
| </View> | |
| )} | |
| <View style={[styles.flexRow, styles.gap2, styles.ph5, styles.alignItemsCenter, styles.pt1]}> | |
| {useRangeLayout && <View style={[styles.flex1]}> | |
| {!!rangeText && ( | |
| <Text style={[styles.textLabelSupporting]}> | |
| {`${translate('common.range')}: `} | |
| <Text style={[styles.textLabel]}>{rangeText}</Text> | |
| </Text> | |
| )} | |
| </View>} | |
| <View style={[styles.flex1]}> | |
| <View style={[styles.flexRow, styles.gap2]}> | |
| <Button | |
| medium | |
| style={[styles.flex1]} | |
| text={translate('common.reset')} | |
| onPress={resetChanges} | |
| /> | |
| <Button | |
| success | |
| medium | |
| style={[styles.flex1]} | |
| text={translate('common.apply')} | |
| onPress={applyChanges} | |
| /> | |
| </View> | |
| </View> | |
| </View> |
We can minimize this block here
Pujan92
reviewed
Feb 25, 2026
Contributor
Author
|
All comments address |
Pujan92
suggested changes
Feb 26, 2026
Contributor
There was a problem hiding this comment.
Thanks @btkcodedev. I just suggested changes of not maintaining non-mandatory trackedDateValues state in the DateSelectPopup and DateFilterBase.
I tried in my local by applying the suggested changes and it looks good to me. Let me know if you think the other way
Screen.Recording.2026-02-26.at.22.46.06.mov
Comment on lines
61
to
65
| const [trackedDateValues, setTrackedDateValues] = useState<SearchDateValues>(value); | ||
|
|
||
| const updateTrackedDateValues = useCallback((dateValues: SearchDateValues) => { | ||
| setTrackedDateValues(dateValues); | ||
| }, []); |
Comment on lines
64
to
68
| // Sync trackedDateValues when actual date values change from parent | ||
| useEffect(() => { | ||
| // eslint-disable-next-line react-hooks/set-state-in-effect | ||
| setTrackedDateValues(value); | ||
| }, [value]); |
Contributor
There was a problem hiding this comment.
Suggested change
| // Sync trackedDateValues when actual date values change from parent | |
| useEffect(() => { | |
| // eslint-disable-next-line react-hooks/set-state-in-effect | |
| setTrackedDateValues(value); | |
| }, [value]); |
Comment on lines
105
to
106
| const dateValues = searchDatePresetFilterBaseRef.current.setDateValueOfSelectedDateModifier(); | ||
| updateTrackedDateValues(dateValues); |
Contributor
There was a problem hiding this comment.
Suggested change
| const dateValues = searchDatePresetFilterBaseRef.current.setDateValueOfSelectedDateModifier(); | |
| updateTrackedDateValues(dateValues); | |
| searchDatePresetFilterBaseRef.current.setDateValueOfSelectedDateModifier(); |
|
|
||
| const dateValues = searchDatePresetFilterBaseRef.current.getDateValues(); | ||
| onChange(dateValues); | ||
| onChange(trackedDateValues); |
Contributor
There was a problem hiding this comment.
Suggested change
| onChange(trackedDateValues); | |
| const dateValues = searchDatePresetFilterBaseRef.current.getDateValues(); | |
| onChange(dateValues); |
Comment on lines
140
to
144
| const rangeText = getDateRangeDisplayValueFromFormValue( | ||
| trackedDateValues[CONST.SEARCH.DATE_MODIFIERS.RANGE], | ||
| trackedDateValues[CONST.SEARCH.DATE_MODIFIERS.AFTER], | ||
| trackedDateValues[CONST.SEARCH.DATE_MODIFIERS.BEFORE], | ||
| ); |
Contributor
There was a problem hiding this comment.
Suggested change
| const rangeText = getDateRangeDisplayValueFromFormValue( | |
| trackedDateValues[CONST.SEARCH.DATE_MODIFIERS.RANGE], | |
| trackedDateValues[CONST.SEARCH.DATE_MODIFIERS.AFTER], | |
| trackedDateValues[CONST.SEARCH.DATE_MODIFIERS.BEFORE], | |
| ); |
| onSelectDateModifier={setSelectedDateModifier} | ||
| presets={presets} | ||
| shouldShowRangeError={shouldShowRangeError} | ||
| onDateValuesChange={updateTrackedDateValues} |
Contributor
There was a problem hiding this comment.
Suggested change
| onDateValuesChange={updateTrackedDateValues} | |
| onDateValuesChange={() => setRangeText(searchDatePresetFilterBaseRef.current?.getRangeDisplayText() ?? '')} |
| const isSearchAdvancedFiltersFormLoading = isLoadingOnyxValue(searchAdvancedFiltersFormMetadata); | ||
| const [selectedDateModifier, setSelectedDateModifier] = useState<SearchDateModifier | null>(null); | ||
| const [shouldShowRangeError, setShouldShowRangeError] = useState(false); | ||
| const [trackedDateValues, setTrackedDateValues] = useState<SearchDateValues>(emptyDateValues); |
Contributor
There was a problem hiding this comment.
Suggested change
| const [trackedDateValues, setTrackedDateValues] = useState<SearchDateValues>(emptyDateValues); |
Similar to the suggested changes in DateFilterPopup
Comment on lines
28
to
29
| /** Sets the date value of the selected date modifier to the ephemeral date value (the selected date in calendar) and returns the updated values */ | ||
| setDateValueOfSelectedDateModifier: () => SearchDateValues; |
Contributor
There was a problem hiding this comment.
Ok, as we update the pattern and won't maintain the trackedDateValues, we can revert this change(void as a return type) now.
| /** Clears the date value of the selected date modifier */ | ||
| clearDateValueOfSelectedDateModifier: () => void; | ||
| /** Clears the date value of the selected date modifier and returns the updated values */ | ||
| clearDateValueOfSelectedDateModifier: () => SearchDateValues; |
Contributor
|
Also tagging @neil-marcellini in case they can do the parallel review here. |
Co-authored-by: Pujan Shah <shahpujan1992@gmail.com>
Contributor
Author
|
All comments cleared |
Contributor
Author
|
All checks have passed. I believe all reported issues have been addressed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Added a new Range date filter option with dual side-by-side calendars for easier date range selection across all search date filters (Date, Approved, Paid, Submitted, Withdrawn, Posted, Exported).
Key Improvements
New Range Mode: Users can now select date ranges using two calendars displayed side-by-side (desktop) or stacked (mobile/sidebar)
Two-Date Validation: Enforces that both From and To dates must be selected when using Range mode, preventing incomplete date ranges
Consistent Display: Date ranges are now displayed as "Range: Jan 1, 2024 - Jan 31, 2024" entries in filter lists and titles
Auto-Detection: When both After and Before dates exist on page load, it automatically switches to Range mode for better UX
Responsive Layouts:
Preserved Workflows: Individual After/Before editing still works when selected directly from menu items
Technical Changes
CONST.SEARCH.DATE_MODIFIERS.RANGERangeDatePickercomponent with dual calendarsDateFilterBase,DatePresetFilterBase, andDateSelectPopupto support Range modeAdvancedSearchFiltersandSearchFiltersReportFieldPageDropdownButtonFixed Issues
$ #81501
PROPOSAL: #81501 (comment)
Tests
Test Steps
Range Mode - Desktop Popup
Range Mode - Sidebar Filters
Two-Date Validation
Auto-Detection on Reload
Individual After/Before Editing
Mobile/Narrow Layout
Offline tests
Same as above
QA Steps
Design verification
Verify that the designs are as given in the issue section
Range Mode - Desktop Popup
Range Mode - Sidebar Filters
Two-Date Validation
Auto-Detection on Reload
Individual After/Before Editing
Mobile/Narrow Layout
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen_Recording_20260209_234831_New.Expensify.Dev.mp4
Android: mWeb Chrome
Screen_Recording_20260209_234141_Brave.mp4
iOS: Native
Screen.Recording.2026-02-09.at.11.30.23.PM.mov
iOS: mWeb Safari
Screen.Recording.2026-02-09.at.11.34.04.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-02-10.at.12.04.47.AM.mov