[Wave 8] Ideal nav #33280
[Wave 8] Ideal nav #33280
Conversation
src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/index.tsx
Outdated
Show resolved
Hide resolved
src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/CustomFullScreenRouter.tsx
Outdated
Show resolved
Hide resolved
src/libs/Navigation/AppNavigator/createCustomStackNavigator/CustomRouter.ts
Outdated
Show resolved
Hide resolved
src/libs/Navigation/AppNavigator/createCustomStackNavigator/CustomRouter.ts
Outdated
Show resolved
Hide resolved
|
|
This comment has been minimized.
This comment has been minimized.
|
Hey @mountiny! When would be possible to start testing the changes? 😄 |
|
@adamgrzybowski Can you please merge main / resolve conflicts? I see some changes related the LHP that should not be a part of this PR |
|
Conflicts have been resolved :) |
|
How is this supposed to look on Web? Not seeing much currently |
Screen.Recording.2024-01-04.at.11.44.59.AM.mov |
|
Question: What is the role of the Expensify top-left logo button? Screen.Recording.2024-01-04.at.11.53.03.AM.mov |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
Help and Go Expensify Classing missing the external link icon 
|
The designs don't include this icon.
|
Oh, we forgot to mention. To got everything working you have to do the following: We made some changes in a patch file and simple |
We haven't implemented this part yet - the workspace switcher. We are currently working on it. |
This will be workspace switcher, where you can filter the app functionality on workspace basis |
|
I love the new web design <3 |
Screen.Recording.2024-01-04.at.12.45.47.PM.mov |
|
Delete popover is misplaced 
Same with workspace avatar change button 
|
|
Bug / By design? The settings in the workspace do not take full width 
|
|
Starting VBBA process messes with the flow: the central screen have a chat while the sidebar is on workspace settings. Also browser back button do not work Screen.Recording.2024-01-04.at.1.07.46.PM.mov |
|
Not a bug: Now that the workspace name field is pushed on it's own page we should auto focus the input 
|
|
Crash when trying to view a workspace avatar Screen.Recording.2024-01-04.at.1.26.02.PM.mov |
|
(similar to #33280 (comment)) If you refresh the page while RHP is open the highlighted item in Settings list is lost Screen.Recording.2024-01-04.at.2.19.37.PM.mov |
|
Workspace links all redirects to the overview and you can't refresh a page that you are currently in Screen.Recording.2024-01-04.at.2.23.26.PM.mov |
src/libs/Navigation/AppNavigator/Navigators/CentralPaneNavigator/BaseCentralPaneNavigator.tsx
Show resolved
Hide resolved
|
Coming from #37982. We missed updating a goBack here: App/src/libs/actions/PaymentMethods.ts Line 287 in 46d5d85 |
| SCREENS.SETTINGS.WALLET.CARDS_DIGITAL_DETAILS_UPDATE_ADDRESS, | ||
| ], | ||
| [SCREENS.SETTINGS.SECURITY]: [SCREENS.SETTINGS.TWO_FACTOR_AUTH, SCREENS.SETTINGS.CLOSE], | ||
| [SCREENS.SETTINGS.ABOUT]: [SCREENS.SETTINGS.APP_DOWNLOAD_LINKS, SCREENS.KEYBOARD_SHORTCUTS], |
There was a problem hiding this comment.
Coming from #35887, we shouldn’t add SCREENS.KEYBOARD_SHORTCUTS to this mapping. The page can be opened independently with a keyboard shortcut, so it shouldn't navigate from the current page.
| @@ -99,9 +186,54 @@ export default function linkTo(navigation: NavigationContainerRef<RootStackParam | |||
| // If this action is navigating to the ModalNavigator and the last route on the root navigator is not already opened ModalNavigator then push | |||
| } else if (isModalNavigator(action.payload.name) && !isTargetNavigatorOnTop) { | |||
There was a problem hiding this comment.
Opening new workspace from workspace list is not creating new workspace, ref: #38420
| } | ||
|
|
||
| return Object.values(policies) | ||
| .filter((policy) => PolicyUtils.shouldShowPolicy(policy, !!isOffline)) |
There was a problem hiding this comment.
We should filter unapproved workspaces as well. #40679
| shouldShowBackButton={isSmallScreenWidth} | ||
| onBackButtonPress={() => Navigation.goBack()} | ||
| > | ||
| <Button |
There was a problem hiding this comment.
This button was causing the header title to wrap in two lines. #44210
There was a problem hiding this comment.
thank you for letting us know 😄
| policy?.isPolicyExpenseChatEnabled && | ||
| policy?.role === CONST.POLICY.ROLE.ADMIN && | ||
| (isOffline || policy?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || Object.keys(policy.errors ?? {}).length > 0) | ||
| !!policy && policy?.isPolicyExpenseChatEnabled && (isOffline || policy?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || Object.keys(policy.errors ?? {}).length > 0) |
There was a problem hiding this comment.
👋 Hey folks, coming from #44811
This PR has deleted policy?.role === CONST.POLICY.ROLE.ADMIN check, which resulted in workspaces being displayed in workspace list even if user has no role in them
We've resolved this by adding a !!policy?.role check to filter out all of the workspaces in which the user has no role
There was a problem hiding this comment.
We didn't take into account the case where a user is removed from the selected workspace. In such case we should switch to "Expensify". (Coming from #43981)
| const policyID = isAnonymous ? undefined : extractPolicyIDFromPath(path); | ||
|
|
||
| const state = getStateFromPath(pathWithoutPolicyID, options) as PartialState<NavigationState<RootStackParamList>>; | ||
| replacePathInNestedState(state, path); |
There was a problem hiding this comment.
Coming from #49854.
We replaced path with normalizedPath to ensure it also have a prefix /, just like the path in useLinking of react-navigation, to prevent the URL from becoming incorrect when using the browser's back button. :)
| } | ||
|
|
||
| return Object.values(reports).reduce<ChatPolicyType>((result, report) => { | ||
| if (!report?.reportID || !report.policyID) { |
There was a problem hiding this comment.
Hey folks👋
This has caused a regression in #51820, user is navigated to thread message when going to #announce/admins room
Since all reports in announce room have chatType === policyAnnounce (even the thread reports), we were mistakenly identifying any report with chatType === policyAnnounce as the announce room in the switch statement below
This was resolved by returning early here is a report has parentReportID
| const selectPolicy = useCallback( | ||
| (option?: SimpleWorkspaceItem) => { | ||
| if (!option) { | ||
| return; | ||
| } | ||
|
|
||
| const {policyID, isPolicyAdmin} = option; | ||
|
|
||
| if (policyID) { | ||
| setSelectedOption(option); | ||
| } else { | ||
| setSelectedOption(undefined); | ||
| } | ||
| setActiveWorkspaceID(policyID); | ||
| Navigation.goBack(); | ||
| if (policyID !== activeWorkspaceID) { | ||
| Navigation.navigateWithSwitchPolicyID({policyID, isPolicyAdmin}); | ||
| } | ||
| }, | ||
| [activeWorkspaceID, setActiveWorkspaceID], | ||
| ); |
There was a problem hiding this comment.
On double click this callback is called twice. After the first click (and after callingsetActiveWorkspaceID), the order of the workspaces change making the second click hit a different workspace. Resulting in unexpected workspace selection (user lands on the previous selected workspace).
First click:
Second click:
Coming from #51402
Details
Introduces the new navigation style to the App, mainly the new bottom tabs, the workspace switcher and moves several pages to the main pane navigator. For more details about the navigation patterns and design, refer to the design doc.
Fixed Issues
$ #32941
Tests
There is obviously lots to cover in testing, we have let QA Applause team know about this big change and gave them a document with testing steps to follow.
Offline tests
Similar to the Test when it comes to the navigation patterns.
QA Steps
There is obviously lots to cover in testing, we have let QA Applause team know about this big change and gave them a document with testing steps to follow.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.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 so 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
We are not adding specific test screenshots or videos into this sections. The PR has been tested thoroughly over the course of weeks on various platforms. You can see that progress in the comment history here, rest assured we tried our best to catch any regressions.
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop