Conversation
Kicu
left a comment
There was a problem hiding this comment.
Nice job, I dropped some small comments, but we also agreed to some changes over slack.
src/components/Search/SearchRouter/getQueryWithSubstitutions.ts
Outdated
Show resolved
Hide resolved
|
@hoangzinh Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Kicu
left a comment
There was a problem hiding this comment.
LGTM 👍 I left very small comments to improve the code quality
src/components/Search/SearchRouter/getQueryWithSubstitutions.ts
Outdated
Show resolved
Hide resolved
luacmartins
left a comment
There was a problem hiding this comment.
Looking great! I agree with @Kicu's comments
|
Hi @luacmartins, do you need a C+ on this PR? I assume we need in order to do some testing/recordings. |
|
@hoangzinh yes, please review it |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-12-13.at.15.45.10.android.movAndroid: mWeb ChromeScreen.Recording.2024-12-13.at.15.42.09.android.chrome.moviOS: NativeScreen.Recording.2024-12-13.at.16.57.26.ios.moviOS: mWeb SafariScreen.Recording.2024-12-13.at.16.55.02.ios.safari.movMacOS: Chrome / SafariScreen.Recording.2024-12-13.at.08.47.17.web.mp4MacOS: DesktopScreen.Recording.2024-12-13.at.15.25.28.desktop.mov |
|
@289Adam289 please add a recording for desktop platform. Thank you. |
|
✋ 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/luacmartins in version: 9.0.77-0 🚀
|
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.77-6 🚀
|
Explanation of Change
This PR changes names of filters only on UI level but allows also for old names to be used.
Rename:
cardIDtocardtaxRatetotax-rateexpenseTypetoexpense-typesortBytosort-bysortOrdertosort-orderreportIDtoreportidpolicyIDtoworkspaceFixed Issues
$#51966
PROPOSAL:
Tests
Search insuggestion generates query with new name and that it works.Offline tests
QA Steps
Same as tests
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
Screen.Recording.2024-12-09.at.17.33.16.mp4
Android: mWeb Chrome
Screen.Recording.2024-12-09.at.17.31.05.mp4
iOS: Native
Screen.Recording.2024-12-09.at.17.26.06.mp4
iOS: mWeb Safari
Screen.Recording.2024-12-09.at.17.29.07.mp4
MacOS: Chrome / Safari
Screen.Recording.2024-12-09.at.17.20.38.mp4
MacOS: Desktop
Screen.Recording.2024-12-12.at.16.12.41.mp4