Rework search input to be a keyword search only#93543
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 587eb7ac8e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const keywordQuery = buildFilterValuesString(CONST.SEARCH.SYNTAX_FILTER_KEYS.KEYWORD, keywordFilters).trim(); | ||
|
|
||
| function submitSearch(query: string) { | ||
| const queryWithContext = getKeywordQueryWithCurrentSearchContext(query, queryJSON); |
There was a problem hiding this comment.
Preserve keyword-only semantics before parsing input
When the new search-page input receives text that is valid search syntax, such as type:expense, merchant:Amazon, or amount>10, it is passed directly into getKeywordQueryWithCurrentSearchContext and then parsed by getQueryWithUpdatedValues, so the text becomes root fields or filters instead of the keyword filter this field is supposed to edit. In those cases the search can switch type/add filters or carry stale context rather than applying the entered text as a keyword; escape/wrap the user text as a keyword value (or otherwise keep explicit syntax out of this path) before composing the query.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Oh, interesting, but this is probably out of scope. This is happening on prod too.
@trjExpensify should we handle the keyword escaping here?
There was a problem hiding this comment.
Is it out of scope? 🤔 On production right now the search input above the table accepts syntax doesn't it? Whereas this change is it to move the query search input to the top right, and make the search input above the table keyword only.
So with that, I think we probably do need to do something about this here to accommodate the desired change.
CC: @JS00001 @Expensify/design
There was a problem hiding this comment.
On prod, we can replicate it by typing the keyword in the keyword filter.
web.mp4
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.
|
|
Videos look good to me 👍 Shall we run an adhoc? |
|
Let's run it 🚀 will build now |
|
🚧 @shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
I don't think I would expect needing to press enter to actually trigger this search, I would think we'd start searching pretty much as you type? CleanShot.2026-06-15.at.09.56.51.mp4Also, note that we get a little jumping of the table when the skeleton comes in - the skeleton is not located at the same exact point as the table is so we get the vertical shifting. |
Oh hm, isn't that going to result in us needing to call the Search API after every letter typed or something though? 🤔 |
|
I think we should search on enter rather than basing it on the input changing (debounced or not) |
@trjExpensify my question was that if "X clear" should also clear keyword, why No (bold) when has keyword? Looks like everyone agreed the behavior on the adhoc build. So ignore my concern. |
Fixed by escaping the input. It will wrap any keyword with (xx:xx) pattern with a quote (""). For example, if we input But it will also escape invalid syntax. For example, There is no harm in doing so, though, so I left it as is to avoid making the escaping code complex.
Fixed |
|
conflicts! |
|
Fixed |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Doesn't seem like it on the adhoc build. 2026-06-18_01-03-31.mp4 |
|
Same, no change. 2026-06-18_01-06-29.mp4 |
|
Hmm, that's weird, it works fine when I test it on the AdHoc build w.mp4 |
|
Okay, weird. I even tried to clear cache last night. I can see changes now though. So back to this. Can you see my video on how staging works, and follow that? On this adhoc build:
Compare to my video on staging of the router:
So we want to keep doing that for now. Can you do that? |
|
Ah I see. Let me try. |
|
@trjExpensify a few clarification questions:
Basically I need a clarification whether it should work similarly to the current search input we have in staging/prod or should it only keep the last input. |
Like it works today on staging/prod is where we want to start. 👍
2026-06-18_16-09-12.mp4 |
|
@trjExpensify done, please check again |
|
Running a fresh build. |
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
trjExpensify
left a comment
There was a problem hiding this comment.
That was fixed for me now. Thanks! 👍
Explanation of Change
Fixed Issues
$ #92924
PROPOSAL:
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
Android: mWeb Chrome
iOS: Native
ios.mp4
iOS: mWeb Safari
ios.mweb.mp4
MacOS: Chrome / Safari
web.mp4