Add workspace autocomplete list and highlighting#60424
Conversation
|
@Expensify/design @flaviadefaria @mountiny @WojtekBoman This pr is pretty much ready. Could you trigger an adhoc build and test it. |
|
There is a bug currently on main that makes a search router behave weirdly. I fixed it in the last commit so the router can be tested in adhoc builds. |
|
Will trigger a build now. |
|
🚧 @shawnborton has triggered a test app build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Highlight is looking pretty good to me! |
4fd0ba8 to
dd58318
Compare
|
@289Adam289 How is this one looking? |
Kicu
left a comment
There was a problem hiding this comment.
Looks pretty straightforward 👍
I only left minor ideas for code improvements, nothing big.
| case CONST.SEARCH.SYNTAX_FILTER_KEYS.FEED: | ||
| case CONST.SEARCH.SYNTAX_FILTER_KEYS.CARD_ID: | ||
| case CONST.SEARCH.SYNTAX_FILTER_KEYS.POLICY_ID: | ||
| return substitutionMap[`${range.key}:${range.value}`] !== undefined; |
There was a problem hiding this comment.
No bugs here 👍 I noticed a small improvement that we could do.
The way this key is built: ${range.key}:${range.value} should be the exact same way that we build it inside autocompleteMap.
There is a local getSubstitutionsKey() function in buildSubstitutionsMap (and duplicated in 1 other file as well).
We could extract it, and reuse here - just to be super clean 😎
|
|
||
| /** | ||
| * A copy of `getFilterDisplayValue` handling the policy ID, used if you have access to the leftHandBar beta. | ||
| * When this beta is no longer needed, this method will be renamed to `getFilterDisplayValue` and will replace the old method. |
There was a problem hiding this comment.
The cleanest thing we can do here is to mark this with @Todo and then link the correct issue for the cleanup (I think @WojtekBoman can probably help us find the proper one).
@todo comments stand out more than normal comments
There was a problem hiding this comment.
Would be good to tie it to some issue so we do not forget this
I think we are ready for review |
|
@dukenv0307 @dannymcclain ready for a review |
|
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
|
code looks good |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2025-04-23.at.15.55.10.movAndroid: mWeb ChromeScreen.Recording.2025-04-23.at.15.53.44.moviOS: NativeScreen.Recording.2025-04-23.at.15.54.43.moviOS: mWeb SafariScreen.Recording.2025-04-23.at.15.53.10.movMacOS: Chrome / SafariScreen.Recording.2025-04-23.at.15.47.54.movMacOS: DesktopScreen.Recording.2025-04-23.at.15.58.11.mov |
|
|
||
| /** | ||
| * A copy of `getFilterDisplayValue` handling the policy ID, used if you have access to the leftHandBar beta. | ||
| * When this beta is no longer needed, this method will be renamed to `getFilterDisplayValue` and will replace the old method. |
There was a problem hiding this comment.
Would be good to tie it to some issue so we do not forget this
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.32-0 🚀
|
|
@289Adam289 Any steps for QA team? |
|
Sorry I forgot about that
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.1.32-8 🚀
|
Explanation of Change
Fixed Issues
$ #59367
PROPOSAL:
Tests
workspace:Offline tests
QA Steps
workspace:// TODO: These must be filled out, or the issue title must include "[No QA]."
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))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
Screen.Recording.2025-04-22.at.12.30.10.mov
iOS: Native
iOS: mWeb Safari
Screen.Recording.2025-04-22.at.12.18.25.mov
MacOS: Chrome / Safari
Screen.Recording.2025-04-22.at.12.05.27.mov
MacOS: Desktop