[Desktop Navigation] Remove leftHandBar beta#61066
Conversation
|
Is it ready for review @WojtekBoman ? |
851f95c to
e17a312
Compare
It's ready :) |
| const [shouldShowComposeInput] = useOnyx(ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT, {initialValue: false}); | ||
| const [isAnonymousUser = false] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS}); | ||
| const [shouldShowComposeInput] = useOnyx(ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT, {initialValue: false, canBeMissing: true}); | ||
| const [isAnonymousUser = false] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS, canBeMissing: true}); |
There was a problem hiding this comment.
canBeMissing should be false here
src/pages/ConciergePage.tsx
Outdated
| const {canUseLeftHandBar} = usePermissions(); | ||
|
|
||
| const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED); | ||
| const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {canBeMissing: false}); |
There was a problem hiding this comment.
I think canBeMissing should be true, we did the same in other place
src/pages/ConciergePage.tsx
Outdated
| const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {canBeMissing: false}); | ||
| const viewTourTaskReportID = introSelected?.viewTour; | ||
| const [viewTourTaskReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${viewTourTaskReportID}`); | ||
| const [viewTourTaskReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${viewTourTaskReportID}`, {canBeMissing: false}); |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2025-04-30.at.10.27.11.movAndroid: mWeb ChromeScreen.Recording.2025-04-30.at.10.21.55.moviOS: NativeScreen.Recording.2025-04-30.at.10.26.57.moviOS: mWeb SafariScreen.Recording.2025-04-30.at.10.22.08.movMacOS: Chrome / Safariweb-resize.mp4MacOS: DesktopScreen.Recording.2025-04-30.at.10.29.50.mov |
|
looks good, I just left some minor comments |
|
Comments resolved |
|
@stitesExpensify 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] |
|
🎯 @dukenv0307, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #61155. |
|
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
mountiny
left a comment
There was a problem hiding this comment.
@WojtekBoman minor changes requested
This comment has been minimized.
This comment has been minimized.
|
@mountiny @Expensify/design The build problem should be solved now. Please run new builds and check if everything looks fine :) There might be a problem with |
|
🚧 @shawnborton 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! 🧪🧪 |
a6f1294 to
32fc713
Compare
Done! |
|
✋ 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.40-0 🚀
|
|
I am not 100% sure, but I think this PR might have caused this bug on Safari: #61460 |
|
Nvm, regardless of Applause, I was able to reproduce on production as well so not coming from this |
|
Interesting catch on the Safari bug! I wonder if that's a weird z-index thing or something? No idea! |
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.1.40-7 🚀
|
|
|
||
| // We need to shift the sidebar to not be covered by the StackNavigator so it can be clickable. | ||
| marginLeft: shouldUseNarrowLayout ? 0 : -sidebarWidth, | ||
| marginLeft: shouldUseNarrowLayout ? 0 : -variables.sideBarWithLHBWidth, |
| canBeMissing: true, | ||
| }); | ||
| const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID}`); | ||
| const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID}`, {canBeMissing: false}); |
There was a problem hiding this comment.
@mountiny not yet. I can take care of it if needed.



Explanation of Change
This PR removes the
leftHandBarbetaFixed Issues
$ #59371
$ #59372
$ #59373
$ #59369
$ #59366
PROPOSAL:
Tests
Offline tests
QA Steps
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))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
Screen.Recording.2025-04-30.at.08.48.25.mov
iOS: mWeb Safari
Screen.Recording.2025-04-30.at.09.01.44.mov
MacOS: Chrome / Safari
Screen.Recording.2025-04-29.at.20.19.52.mov
MacOS: Desktop
Screen.Recording.2025-04-29.at.20.21.07.mov