Add const / permissions function for help side panel beta#59221
Conversation
|
Oh nice, I guess I can close #58751 if we go ahead with this then? |
Yes please! |
|
@carlosmiceli 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] |
|
@carlosmiceli I think you can ignore this PR, @francoisl has context and will likely review it! |
|
@blazejkustra Using an account with beta permissions, when setting the object to null Screen.Recording.2025-03-31.at.11.04.56.mov |
| const {isExtraLargeScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout(); | ||
| const [sidePaneNVP] = useOnyx(ONYXKEYS.NVP_SIDE_PANE); | ||
| const [language] = useOnyx(ONYXKEYS.NVP_PREFERRED_LOCALE); | ||
| const [canUseHelpSidePanel = false] = useOnyx(ONYXKEYS.BETAS, {selector: (betas) => !!betas?.includes(CONST.BETAS.HELP_SIDE_PANEL)}); |
There was a problem hiding this comment.
I wonder why we don't use usePermissions() here? Thanks.
There was a problem hiding this comment.
I don't know why but usePermissions was breaking GH checks here
| const isSidePaneHiddenOrLargeScreen = !isSidePaneVisible || isLanguageUnsupported || isExtraLargeScreenWidth || !canUseHelpSidePanel; | ||
|
|
||
| // The help button is hidden when: | ||
| // - side pane nvp is not set |
There was a problem hiding this comment.
Also, do we need to update this comment like we did in the other PR.?
cc: @francoisl
No it wasn't.. Great find @brunovjk! Fixed in the latest commit |
|
Great @blazejkustra, I have to go now, I will complete the checklist by the end of the day. |
@brunovjk Any updates? |
|
Sorry, I was dealing with the deploy blocker and ran out of time. I'll review it now. |
|
No worries! Thank you 🚀 |
Reviewer Checklist
Screenshots/VideosAndroid: Native59221_android_native.movAndroid: mWeb ChromeUploading 59221_android_web.mov… iOS: Native59221_ios_native.moviOS: mWeb SafariUploading 59221_ios_web.mov… MacOS: Chrome / Safari59221_web_chrome.movMacOS: Desktop59221_web_desktop.mov |
|
All yours @francoisl :D |
|
Oh, lol, just seeing the comments, went straight to review! All yours @francoisl :D |
| function canUseHelpSidePanel(betas: OnyxEntry<Beta[]>): boolean { | ||
| return !!betas?.includes(CONST.BETAS.HELP_SIDE_PANEL) || canUseAllBetas(betas); | ||
| } |
There was a problem hiding this comment.
So this isn't used anywhere? If so let's remove it.
There was a problem hiding this comment.
We have a tracking issue to remove the beta when it's no longer needed anyway, not blocking on this.
There was a problem hiding this comment.
Ahh, I missed that one 🤦 I should've used this function as the selector of useOnyx. I'll refactor it for Stage 4
|
✋ 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/francoisl in version: 9.1.23-1 🚀
|
|
@blazejkustra @brunovjk QA team is not seeing the help button even after using an Expensifail account and running Screenrecorder-2025-04-03-22-09-25-632.mp4 |
|
@nlemma this is because we need to add accounts to a beta differently for this, it doesn't work on every expensifail account like other betas. I added applausetester+bp0104h@applause.expensifail.com, so it should work from that account now. |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.23-7 🚀
|
Explanation of Change
Add a function
canUseHelpSidePanel()for theusePermissions()hook, that we can use to check if a user should see the help panel button. There was a bug that caused users on production to see the button at some point, so we thought it might be safer to use a beta instead.Fixed Issues
N/A Part of #56988
Tests
newDotHelpSidePanel, or part of all betasOffline tests
N/A
QA Steps
*expensifail.comaccountPR 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
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop