migrate sidebarLinkData to useOnyx#52550
Conversation
|
@ikevin127 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] |
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid.webmAndroid: mWeb Chromeandroid-mweb.webmiOS: Nativeios.MP4iOS: mWeb Safariios-mweb.MP4MacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
| const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, {initialValue: CONST.PRIORITY_MODE.DEFAULT}); | ||
|
|
||
| const {orderedReportIDs, currentReportID, policyMemberAccountIDs} = useReportIDs(); | ||
|
|
There was a problem hiding this comment.
NAB: I noticed this constant assignment in this component which is redundant as it was leftover from another PRs commit 1b0738a back when we used to have 2 checks, but now that we only have 1 (isLoadingApp) there's no point in re-assigning it to a constant.
- const isLoading = isLoadingApp;
- isLoading={isLoadingApp ?? false}
+ isLoading={isLoadingApp ?? false}There was a problem hiding this comment.
Sorry, I missed this. I'll update it shortly.
|
@Nodebrute Hi, do you mind addressing the Requested changes from #52550 (comment) so I can move forward with the PR review process and hopefully merge this PR ? Thanks! |
|
@ikevin127 Thank you for your patience. The PR is now ready for review. |
ikevin127
left a comment
There was a problem hiding this comment.
Thanks for addressing the change request, the fix LGTM 🚀
|
cc @MariaHCD for visibility |
|
✋ 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/MariaHCD in version: 9.0.67-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.67-9 🚀
|
Explanation of Change
Fixed Issues
$ #52459
PROPOSAL: #52459 (comment)
Tests
Offline tests
Same as above
QA Steps
// 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
Screen.Recording.2024-11-14.at.7.42.50.PM.mov
Android: mWeb Chrome
Screen.Recording.2024-11-14.at.7.45.29.PM.mov
iOS: Native
Screen.Recording.2024-11-14.at.8.09.49.PM.mov
iOS: mWeb Safari
Screen.Recording.2024-11-14.at.7.54.32.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2024-11-14.at.7.27.30.PM.mov
MacOS: Desktop
Screen.Recording.2024-11-14.at.7.52.36.PM.mov