Side Pane Stage 2: Add baseline content for each major section #58004
Conversation
… on the current route.
|
|
|
|
|
Screen.Recording.2025-03-07.at.11.43.45.movUpdate: fixed! |
|
|
|
|
|
|
|
|
|
|
|
🚧 @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! 🧪🧪
|
|
On small screens, the icon position is a bit inconsistent in my opinion, do you think we can fix it? Thanks. 58004_ios_web_bug.movcc: @shawnborton |
|
@blazejkustra In "production" and on a small screen, the Help button appears in the header normally, but the content is showing the missing route, not the "Production content": Screen.Recording.2025-03-07.at.12.43.54.mov |
What do you mean exactly? |
|
Though I could see where maybe those two icons could be slightly closer together. We should match whatever we had in the mocks, which looks like 20px of gap between them: |
|
@blazejkustra Android: Native - Staging58004_android_native_stag.movAndroid: Native - Prod58004_android_native_prod.movAndroid: mWeb Chrome - Staging58004_android_web_stag.movAndroid: mWeb Chrome - Prod58004_android_web_prod.mov |
|
On other platforms it looks good to me: iOS: Native58004_ios_native_prod.moviOS: mWeb Safari58004_ios_web_prod.mov58004_ios_web_stag.movMacOS: Chrome / Safari58004_web_chrome_prod.movMacOS: Desktop58004_ios_web_stag.mov58004_web_chrome_stag.mov |
Exactly this:
Thank you @shawnborton 😃 |
Yes, that is correct. There is no content available for /home, and there is no parent route to inherit the content from. When we populate help with content the aim is to have all top level routes covered as per this message form David. Does that make sense @brunovjk? |
I think mobile platforms (at least physical devices) don't have an ESC key at all. Do we still need to handle this behavior? 🤔 |
Let's not block on this, @brunovjk. We can fix it in a follow-up. Maybe another contributor could take it on, as this requires a lot of manual work, and I don’t have the time for it right now. 😅 |
|
Great!!! Thank you for clarifying!! My bad on the Esc thing hahahaha |
Reviewer Checklist
Screenshots/VideosAndroid: Native58004_android_native_stag.movAndroid: mWeb Chrome58004_android_web_stag.mov58004_android_native_prod.moviOS: Native58004_android_web_prod.moviOS: mWeb Safari58004_ios_web_prod.mov58004_ios_web_stag.movMacOS: Chrome / Safari58004_web_chrome_prod.movMacOS: Desktop58004_web_chrome_stag.mov |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #57860 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
On a fresh sign in, the help button doesn't show up on the Screen.Recording.2025-03-07.at.1.47.32.PM.mov |
|
My understanding is that App/src/pages/home/HeaderView.tsx Line 358 in 9063a67 |
|
I got it to show regardless of whether the NVP exists in Onyx with this change, but @blazejkustra I'll let you confirm and address in a follow-up PR if needed, in case there's a better solution :) diff --git a/src/pages/home/HeaderView.tsx b/src/pages/home/HeaderView.tsx
index f6874093ebd..b0c744c5932 100644
--- a/src/pages/home/HeaderView.tsx
+++ b/src/pages/home/HeaderView.tsx
@@ -72,6 +72,7 @@ import SCREENS from '@src/SCREENS';
import type {Report, ReportAction} from '@src/types/onyx';
import type {Icon as IconType} from '@src/types/onyx/OnyxCommon';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
+import useSidePane from '@hooks/useSidePane';
type HeaderViewProps = {
/** Toggles the navigationMenu open and closed */
@@ -206,6 +207,7 @@ function HeaderView({report, parentReportAction, onNavigationMenuButtonClicked,
const isReportInRHP = route.name === SCREENS.SEARCH.REPORT_RHP;
const shouldDisplaySidePane = !!sidePane;
+ const {shouldHideHelpButton} = useSidePane();
const shouldDisplaySearchRouter = !isReportInRHP || isSmallScreenWidth;
const [onboardingPurposeSelected] = useOnyx(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED);
const isChatUsedForOnboarding = isChatUsedForOnboardingReportUtils(report, onboardingPurposeSelected);
@@ -355,7 +357,7 @@ function HeaderView({report, parentReportAction, onNavigationMenuButtonClicked,
{!shouldUseNarrowLayout && isOpenTaskReport(report, parentReportAction) && <TaskHeaderActionButton report={report} />}
{!isParentReportLoading && canJoin && !shouldUseNarrowLayout && joinButton}
</View>
- {shouldDisplaySidePane && <HelpButton style={styles.ml2} />}
+ {!shouldHideHelpButton && <HelpButton style={styles.ml2} />}
{shouldDisplaySearchRouter && <SearchButton style={!shouldDisplaySidePane && styles.ml2} />}
</View>
<ConfirmModal |
| * route: /workspaces/123/rules/456 | ||
| * params: {workspaceID: '123', ruleID: '456'} | ||
| * result: /workspaces/:workspaceID/rules/:ruleID | ||
| */ |
There was a problem hiding this comment.
NAB - in a follow-up PR let's add a unit test for this function
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Explanation of Change
Design doc: https://docs.google.com/document/d/1prFcvgUlEGRuL3zE-JH-lhv_BWX2lL2YNw61w4YVnyY/edit?tab=t.0
This PR expands the Help Pane by adding initial content for key sections and ensuring users always see relevant information.
Key Changes:
/r,/search,/settings,/settings/workspaces, and/settings/workspaces/:policyID.Fixed Issues
$ #57860
PROPOSAL: N/A
Also fixes:
Tests
Staging & Development
/settings/workspaces/:policyIDcontent exists but child like/settings/workspaces/:policyID/membersdoes not and it should display its parent contentProduction (@brunovjk will be able to test it - hardcode
isProductiontotrueinuseSidePane.tsandSidePane/index.tsx)nvp_sidePaneis defined in the console:Offline tests
N/A
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
untitled.mp4
Android: mWeb Chrome
and-web.webm
☝️ for a brief moment there was no bottom tab on Reports screen, this got fixed in a recent commit
iOS: Native
ios.mp4
iOS: mWeb Safari
ios-web.mp4
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov