Implement flexible bottom tab bar

[adamgrzybowski]

Details

Motivation

This PR started with this feature request: Use persistent bottom tabs in Settings to improve Workspace Editor UX. We realized that the current structure of navigators isn't well-suited to handle features requiring conditional placement of the bottom tab bar. Additionally, after many iterations and discussions about navigation, we had an idea for a structure that meets current needs more effectively and is easier to develop and maintain.

Problem

Currently, the first route in the root navigator is always the BottomTabNavigator. This setup means all screens needing the bottom tab bar are nested inside it. All other screens and navigators are stacked on top. This configuration also makes the root navigator responsible for the split view, causing two major issues:

• a) Limited flexibility for deciding which screens should have a bottom tab bar.
• b) The root navigator is too coupled with the split view functionality, which can slow down development and cause bugs.

Solution

a) Flexible Bottom Tab Bar

We decided to detach the bottom tab bar from the bottom tab navigator. The bottom tab navigator no longer exists. Now, we can render the bottom tab bar on any screen that requires it, even conditionally like on the workspace list page where it should be visible only for the narrow layout.

b) Separating Root Navigator and Split Functionality

We have extracted the logic responsible for managing the split view into a separate custom SplitNavigator. Better separation makes it easier to develop new screens and adapt different navigation patterns that don’t follow the usual rules.

Other Changes

We've used this opportunity to address other pain points and bugs in the current navigation implementation. In addition to cleaning up old helpers and reorganizing the directory structure, there are a few notable changes:

getAdaptedState

Thanks to the new split navigators managing their own initial state, we were able to simplify the function that generates the initial global state from a link.

linkTo

This function has grown complex over multiple iterations. Small changes could cause hard to detect bugs. We've simplified it significantly and moved much of the logic to the routers of the custom navigators to improve readability and separation of responsibility.

goBack

With the new structure, we've taken the opportunity to rethink the implementation of this function, making it more bulletproof and eliminating some old hard to fix bugs.

tests

To ensure that resolved bugs remain fixed, we've created manual test cases in NAVIGATION.md and automated tests in tests/navigation/. These tests also help communicate how the navigation should behave, especially in cases that are exemptions from standard rules.

Fixed Issues

$ #44242
PROPOSAL:

Tests

• Verify that no errors appear in the JS console

Offline tests

QA Steps

• Verify that no errors appear in the JS console

There should be a proper report under attachment screen after reload

1. Open any report with image attachment on narrow layout.
2. Open attachment.
3. Reload the page.
4. Verify that after pressing back arrow in the header you are on the report where you sent the attachment.

There is a proper split navigator under RHP with a sidebar screen only for screens that can be opened from the sidebar

1. Open the browser on narrow layout with url /settings/profile/status.
2. Reload the page.
3. Verify that after pressing back arrow in the header you are on the settings root page.

There is a proper split navigator under the overlay after refreshing page with RHP/LHP on wide screen

1. Open the browser on wide screen with url /settings/profile/display-name.
2. Verify that you can see settings profile page under the overlay of RHP.

There is a proper split navigator under the overlay after deeplinking to page with RHP/LHP on wide screen

1. Open the browser on wide screen.
2. Open any report.
3. Send message with url /settings/profile/display-name.
4. Press the sent link
5. Verify that the settings profile screen is now visible under the overlay

The Workspace list page is displayed (SCREENS.SETTINGS.WORKSPACES) after clicking the Settings tab from the Workspace settings screen

1. Open any workspace settings (Settings → Workspaces → Select any workspace)
2. Click the Settings button on the bottom tab.
3. Verify that the Workspace list is displayed (/settings/workspaces)
4. Select any workspace again.
5. Reload the page.
6. Click the Settings button on the bottom tab.
7. Verify that the Workspace list is displayed (/settings/workspaces)

The last visited screen in the settings tab is saved when switching between tabs

1. Open the app.
2. Go to the settings tab.
3. Open the workspace list.
4. Select any workspace.
5. Switch between tabs and open the settings tabs again.
6. Verify that the last visited page in this tab is displayed.

The Workspace selected in the application is reset when you select a chat that does not belong to the current policy

1. Open the home page.
2. Click on the Expensify icon in the upper left corner.
3. Select any workspace.
4. Click on the magnifying glass above the list of available chats.
5. Select a chat that does not belong to the workspace selected in the third step.
6. Verify if the chat is opened and the global workspace is selected.

The selected workspace is saved between Search and Inbox tabs

1. Open the Inbox tab.
2. Change the workspace using the workspace switcher.
3. Switch to the Search tab and verify if the workspace selected in the second step is also selected in the Search.
4. Change the workspace once again.
5. Go back to the Inbox.
6. Verify if the workspace selected in the fourth step is also selected in the Inbox tab.

Going up to the workspace list page after refreshing on the workspace settings and pressing the up button

1. Open the workspace settings from the deep link (use a link in format: /settings/workspaces/:policyID:/profile)
2. Click the app’s back button.
3. Verify if the workspace list is displayed.

Going up to the RHP screen provided in the backTo parameter in the url

1. Open the settings tab.
2. Go to the Profile page.
3. Click the Address button.
4. Click the Country button.
5. Reload the page.
6. Click the app’s back button.
7. Verify if the Profile address page is displayed (/settings/profile/address)

There is proper split navigator under the overlay after refreshing page in RHP that includes valid reportID in params

wide layout :

1. Open any report.
2. Open report details (press the chat header).
3. Reload the app.
4. Verify that the report under the overlay is the same as the one opened in report details.

narrow layout :

1. Open any report
2. Open report details (press the chat header).
3. Reload the app.
4. Verify that after pressing back arrow in the header you are on the report previously seen in the details page.

Navigating back to the Workspace Switcher from the created workspace

1. Open the app and go to the Inbox tab.
2. Open the workspace switcher (Click on the button in the upper left corner).
3. Create a new workspace by clicking on the + button.
4. Navigate back using the back button in the app.
5. Verify if the workspace switcher is displayed with the report screen below it

Going up to the sidebar screen

Linked issue: #44138

1. Go to Subscription page in the settings tab.
2. Click on Request refund button
3. Verify that modal shown
4. Next click Downgrade...
5. Verify that modal got closed, your account is downgraded and the Home page is opened.

Navigating back from the Search page with invalid query parameters

1. Open the search page with invalid query parameters (e.g /search?q=from%3a)
2. Press the app's back button on the not found page.
3. Verify that the Search page with default query parameters is displayed.

Navigating to the chat from the link in the thread

1. Open any chat.
2. If there are no messages in the chat, send a message.
3. Press reply in thread.
4. Press the "From" link in the displayed header.
5. Verify if the link correctly redirects to the chat opened in the first step.

Expense - App does not open destination report after submitting expense

Linked issue: #49539 (comment)

1. Launch the app.
2. Open FAB > Submit expense > Manual.
3. Submit a manual expense to any user (as long as the user is not the currrently opened report and the receiver is not workspace chat).
4. Verify if the destination report is opened after submitting expense.

QBO - Preferred exporter/Export date tab do not auto-close after value selected

Linked issue: #49539 (comment)

Precondition: Workspace with QBO integration connected.

1. Go to Workspace > Accounting.
2. Click on Export > Preferred exporter (or Export date).
3. Click on value.
4. Verify if the value chosen in the third step is selected and the app redirects to the Export page.

Web - Hold - App flickers after entering reason and saving it when holding expense

Linked issue: #49539 (comment)

1. Launch the app.
2. Open DM with any user.
3. Submit two expenses to them.
4. Click on the expense preview to go to expense report.
5. Click on any preview to go to transaction thread.
6. Go back to expense report.
7. Right click on the expense preview in Step 5 > Hold.
8. Enter a reason and save it.
9. Verify if the app does not flicker after entering reason and saving it.

Group - App returns to group settings page after saving group name

Linked issue: #49539 (comment)

1. Launch the app.
2. Create a group chat.
3. Go to group chat.
4. Click on the group chat header.
5. Click Group name field.
6. Click Save.
7. Verify if the app returs to group details RHP after saving group name.

Going up to a screen with any params

Linked issue: #49539 (comment)

1. Press the FAB.
2. Select "Book travel".
3. Press "Book travel" in the new RHP pane.
4. Press "Country".
5. Select any country.
6. Verify that the country you selected is actually visible in the form.

Change params of existing attachments screens instead of pushing new screen on the stack

Linked issue: #49539 (comment)

1. Open any chat.
2. Send at least two images.
3. Open attachment by pressing on image.
4. Press arrow on the side of attachment modal to navigate to the second image.
5. Close the modal with X in the corner.
6. Verify that the modal is now fully closed.

Navigate instead of push for reports with same reportID

Linked issue: #49539 (comment)

1. Open app on wide layout web.
2. Go to report A (any report).
3. Go to report B (any report with message).
4. Press reply in thread.
5. Press on header subtitle.
6. Press on the report B in the sidebar.
7. Verify that the message you replied to is no longer highlighted.
8. Press the browsers back button.
9. Verify that you are on the A report.

Don't push the default full screen route if not necessary.

1. Open app on wide layout web.
2. Open search tab.
3. Press track expense.
4. Verify that the split navigator hasn't changed under the overlay.

BA - Back button on connect bank account modal opens incorporation state modal

Linked issue: #49539 (comment)

Precondition: Use staging server (it can be set in Settings >> Troubleshoot)

1. Launch the app.
2. Navigate to Settings >> Workspaces >> Workspace >> Workflows.
3. Select Connect with Plaid option.
4. Go through the Plaid flow (Added Wells Fargo details).
5. Complete the Personal info, Company info & agreements section.
6. Note user redirected to page with the header Connect bank account and the option to disconnect your now set up bank account.
7. Tap back button on connect bank account modal.
8. Verify if the connect bank account modal is closed and the Workflows page is opened with the bank account added.

App opens room details page when tapping RHP back button after saving Private notes in DM

Linked issue: #49539 (comment)

1. Launch the app.
2. Open DM with any user that does not have content in Private notes.
3. Click on the chat header.
4. Click Private notes.
5. Enter anything and click Save.
6. Click on the RHP back button.
7. Verify if the Profile RHP Page is opened (URL in the format /a/:accountID).

Opening particular onboarding pages from a link and going back

Linked issue: #50177

1. Sign in as a new user.
2. Select Something else from the onboarding flow.
3. Reopen/refresh the app.
4. Verify the Personal detail step is shown.
5. Go back.
6. Verify you are navigated back to the Purpose step.
7. Select Manage my team.
8. Choose the employee size.
9. Reopen/refresh the app.
10. Verify the connection integration step is shown.
11. Go back.
12. Verify you are navigated back to the employee size step.
13. Go back.
14. Verify you are navigated back to the Purpose step.

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

androidWeb.mp4

iOS: Native

ios.mp4

iOS: mWeb Safari

iosWeb.mp4

MacOS: Chrome / Safari

web.mp4

MacOS: Desktop

desktop.mp4

[mountiny]

trim.EDC2126F-EA24-40D4-87A4-E252B86DA87A.MOV

There are no page animations in the settings stack

[JmillsExpensify]

I took this for a quick spin on Chrome/web and it worked fairly well, including properly accounting for the browser back case. One thing I noticed, though I'm not sure if it's related to this PR or not: I occasionally experienced performance issues, where the responsiveness of the page would slow down and almost freeze up for a second or two. I saw this on the Settings pages. I can do a profile trace if helpful, just let me know.

[mountiny]

Definitely performance is something we need to keep in mind

[adamgrzybowski]

@mountiny If I understand correctly this is something that we want. Those screens have the bottom tab bar. Other bottom tab screens don't have animations from the beginning.

@JmillsExpensify we are still working on adjusting performance tricks for these changes. Please point us to flows that feels slower than before so we can investigate it 🙇

[mountiny]

If I understand correctly this is something that we want. Those screens have the bottom tab bar. Other bottom tab screens don't have animations from the beginning.

Hmm, I dont think we want that, going deeper in the settings should have animations, same as going to a report from LHN has animation @Expensify/design @trjExpensify @JmillsExpensify

[mountiny]

@adamgrzybowski Wanted to report here as well that I think the performance on web for me is gradually getting worse and this morning it's noticeably slow, you can see it here as well which I have recorded for a different bug

Screen.Recording.2024-09-25.at.10.12.04.mp4

[adamgrzybowski]

Hmm, I dont think we want that, going deeper in the settings should have animations, same as going to a report from LHN has animation

Screen.Recording.2024-09-25.at.11.52.59.mov

But doesn't that look weird? You can see how one bottom tab bar covers the previous one with animation.

cc: @mountiny

[trjExpensify]

Yeah, I think that does look weird. The bottom tab bar feels like it's part of page being animated in, rather than persisting across the different pages.

[dubielzyk-expensify]

Yeah that looks a bit off. The view should go over the bottom navigation instead. Though there's another issue to put the bottom navigation on top in workspaces.

Which should give us this:

CleanShot.2024-09-26.at.11.06.21.mp4

[mountiny]

I agree that the bottom tab should not animate if it's staying on another page, too. I would defer to @dubielzyk-expensify and the design team to see how we want to handle this.

[dannymcclain]

The video that Jon shared is what I would expect and prefer. But if that's not technically feasible at the moment, I would rather have the bottom tabs than the animation.

[adamgrzybowski]

#49539 (comment) is what I would expect and prefer. But if that's not technically feasible at the moment, I would rather have the bottom tabs than the animation.

That's the problem. It may be tough to implement and if that's okay I would investigate this as follow-up. But let me explain why this is problematic at all.

Previously, we had a navigator that displayed a bottom tab bar over every screen within itself. This approach did not allow us to place the bottom tab bar on every screen in the app. Only on screens in this particular navigator. And it definitely did not allow us to display the bottom tab bar conditionally, as in the "Workspaces screen" case.

We decided to render the bottom on every screen that requires it instead. This approach gives us a lot of flexibility.

We did know that we couldn't create an animation of sliding screen with the bottom tab onto another screen with the bottom tab. But we assumed that as in current use cases, we won't animate screens with the bottom tab so that's not a problem.

Let's assume that we want to have this animation and we won't display it on every page but we will figure out a way to do it smarter.

#49539 (comment)

All screens that you can open from the menu visible in the video above are on the same level. Screen containing this menu is also on the same level. If that doesn't make sense, please consider the fact that we also need to handle the wide layout with this structure.

That means they are rendered with one <StackView />. In the case of workspaces screen and settings root we would need to render the bottom tab bar on top of this stack view. For other screens, we would need to render it somewhere between these cards to have proper animation. But we can't really put anything between these cards. The task will be even harder given the incoming which leaves us with even less wiggle room.

I am not saying it is impossible. I just want to give you context on how complex is adding this little animation. I have a few tricks in my mind but I would rather focus on this after migrating to the new structure.

[adamgrzybowski]

When it comes to performance. I started digging and couldn't reproduce the laggy RHP that you filmed @mountiny. I might have to try the high traffic account or something like that.

However, I discovered a different issue when switching to the "Reports" tab.

What you can notice in our POC is that you can't see the skeleton loading. The first render of the report screen is delayed in comparison to the main.

before.mp4

It doesn't look like a big delay but it feels laggy when you click it yourself.

I investigate that and it looks like this doesn't happen on the main only by coincidence. I figured out a solution for that and tomorrow I will work on PR that we could merge to the main. It should help with the initial render of heavy screens in general and improve the user experience.

Below you can see the test result

after.mp4

[mountiny]

I am fine exploring the animation later

@adamgrzybowski, you should definitely test with a high-traffic account at least; you could even try the heavy performance testing account we provided to SWM. Testing with a fresh account is not representative of the users we care about the most.

[adamgrzybowski]

FYI: I played around with a solution I found yesterday regarding performance, and it may need more work and investigation. For now, I'll focus on other work left for this PR and get back to perf closer to review.

[github-actions]

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/49539/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/49539/index.html
Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/49539/NewExpensify.dmg	https://49539.pr-testing.expensify.com

---

👀 View the workflow run that generated this build 👀

[melvin-bot]

@ 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]

[adamgrzybowski]

@mountiny tooltip issues on the bottom tab bar are finally resolved and tests should be fixed. I removed the draft label.

BTW Thanks @WoLewicki for the internal review 🙇

[shubham1206agra]

@adamgrzybowski Is it possible to fix ESLint here.

[github-actions]

🚧 @mountiny has triggered a test build. You can view the workflow run here.

[mountiny]

No need to fix the default IDs and the import issues, lets not add more changes that are not relevant to this PR

I have kicked off the build @shubham1206agra @dukenv0307 can you please start with testing and reviewing and make this your top priority? Thanks!

[github-actions]

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/49539/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/49539/index.html
Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/49539/NewExpensify.dmg	https://49539.pr-testing.expensify.com

---

👀 View the workflow run that generated this build 👀

[mountiny]

@adamgrzybowski @WojtekBoman getting crash when signing into my account on the build

[dukenv0307]

Reviewing...

[dukenv0307]

We don't need to add setActiveWorkspaceID to dependency.

[dukenv0307]

Bug:

1. Logout if needed
2. Login with new account

[WojtekBoman]

Bug:

1. Logout if needed
2. Login with new account

Hey, I've already fixed that, it should work now :)

[github-actions]

🚧 @mountiny has triggered a test build. You can view the workflow run here.

[github-actions]

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/49539/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/49539/index.html
Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/49539/NewExpensify.dmg	https://49539.pr-testing.expensify.com

---

👀 View the workflow run that generated this build 👀

[mountiny]

Asked QA for testing!