Update Workspaces tab URLs to just workspaces to clearly differentiate them from Account-related URLs#65018
Conversation
|
Assigning Gandalf for this one due to context and follow up, I hope that is ok |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
@sumo-slonik please update the following docs with the correct link for workspace navigation: ./contributingGuides/NAVIGATION_TESTS.md
./contributingGuides/NAVIGATION.md
./contributingGuides/STYLE.md |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
| @@ -1,33 +0,0 @@ | |||
| --- | |||
There was a problem hiding this comment.
why are we deleting this file, shouldn’t we just rename it?
There was a problem hiding this comment.
Yes, you're right. For now, I'm working on your previous comments. I'll let you know when it's ready for the next round of review.
e447620 to
3af0a69
Compare
|
@allgandalf I've fixed everything, you can review it now. The removal of one index.md file in the help directory is justified because we previously had two directories: :policyID and policyId. While updating the structure to the new paths, I decided to standardize it. |
|
@allgandalf ready for you |
|
On it today! |
allgandalf
left a comment
There was a problem hiding this comment.
Code LGTM! , completing checklsit now
| @@ -0,0 +1,25 @@ | |||
| import getMatchingNewRoute from '@navigation/helpers/getMatchingNewRoute'; | |||
There was a problem hiding this comment.
nice job with the tests!
Okay cool!! |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppBuild errorAndroid: mWeb ChromeScreen.Recording.2025-07-01.at.9.44.44.AM.moviOS: HybridAppScreen.Recording.2025-07-01.at.9.28.02.AM.moviOS: mWeb SafariScreen.Recording.2025-07-01.at.9.30.12.AM.movMacOS: Chrome / SafariScreen.Recording.2025-07-01.at.8.55.54.AM.movMacOS: DesktopScreen.Recording.2025-07-01.at.9.03.26.AM.mov |
|
All yours @mountiny |
mountiny
left a comment
There was a problem hiding this comment.
In general changes look good to me but left couple questions
| @@ -0,0 +1,8 @@ | |||
| const oldRoutes: Record<string, string> = { | |||
There was a problem hiding this comment.
I feel like we could add this to cloudfront to redirect, but also do we need to add /workspaces to the AndroidManifest or apple-site json thing for deeplinks?
App/android/app/src/main/AndroidManifest.xml
Lines 61 to 106 in 3caba34
There was a problem hiding this comment.
I'll finish the changes in the PR related to the empty state in the report, and then I'll get started on this.
There was a problem hiding this comment.
@sumo-slonik ok there are some conflicts now, can you please check the deeplink configs for ios and android?
| }; | ||
|
|
||
| const helpContentMap: HelpContent = { | ||
| content: () => null, |
There was a problem hiding this comment.
@sumo-slonik These changes feel to a big part unrelated, why is it here?
There was a problem hiding this comment.
This file is automatically generated. I regenerated it to include the changes I made, using the following command:
cd help && bundle exec jekyll build && cd .. && cp help/_src/helpContentMap.tsx src/components/SidePanel/HelpContent/helpContentMap.tsx
It's possible that some changes appeared on main that weren’t rebuilt by others.
There was a problem hiding this comment.
yeahhh, we need this help content, for example, the members page content is not present on staging/ prod, but with our PR, we add the content and now we can see the same when we visit the members page, Now i also feel like @blazejkustra should review this PR once, since they were the implementor of the help content PR.
It's possible that some changes appeared on main that weren’t rebuilt by others.
Makes sense!
Staging:
Our PR:
There was a problem hiding this comment.
It's possible that some changes appeared on main that weren’t rebuilt by others.
That's exactly what happened, Ted added some content here but didn't regenerate helpContentMap. I’ve opened a PR with the regenerated content, currently pending final review from an internal engineer.
Now i also feel like @blazejkustra should review this PR once, since they were the implementor of the help content PR.
I was overseeing this PR in terms of the help panel, and everything was implemented according to the current standards @allgandalf @mountiny
There was a problem hiding this comment.
Now that I think about it I believe it would be safer to wait for this to be merged first, then regenerate content one more time here and then merge this PR ![]()
# Conflicts: # src/components/SidePanel/HelpContent/helpContentMap.tsx
|
@allgandalf @mountiny I merged main and we've already gone through the PR that @blazejkustra mentioned, so I think we can go ahead and merge this. |
|
I'll just check the Deep Link configs on iOS and Android. |
|
@allgandalf @mountiny I've checked, and this change shouldn't impact the deep links because we don't have a deep link leading to the workspaces with |
|
🚀Deployed to NewHelp production! 🚀 |
|
✋ 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.77-1 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.1.77-2 🚀
|
| "policyID": "1" | ||
| }, | ||
| "path": "/settings/workspaces/1/overview", | ||
| "path": "workspaces/1/overview", |
There was a problem hiding this comment.
We should also add this new path to apple-app-site-association
file, so our smart app banner can display on mSafari.
More details:
#66743 (comment)


Explanation of Change
Fixed Issues
$ #64968
PROPOSAL:
Tests
On web (Mac, Android, and iOS):
Open all tabs.
Check if navigation works correctly.
Go to the "Workspace" tab
Change the link from /workspaces to /settings/workspaces.
Ensure that the app automatically redirects from /settings/workspaces to /workspaces.
Enter a subpage within the workspace section
Ensure that the app automatically redirects from /settings/workspaces to /workspaces.
On native apps (Android and iOS):
Offline tests
Unnecessary
QA Steps
Same as test
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Screen.Recording.2025-06-26.at.12.58.16.mp4
Android: mWeb Chrome
android.web.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
ios.web.mp4
MacOS: Chrome / Safari
mac.web.mp4
MacOS: Desktop
desktop.mp4