Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
2.99k stars 2.5k forks source link

[Wave 8] Ideal nav #33280

Closed adamgrzybowski closed 3 months ago

adamgrzybowski commented 5 months ago

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

$ https://github.com/Expensify/App/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

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
MaciejSWM commented 4 months ago

image

github-actions[bot] commented 4 months ago
:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube: Android :robot: iOS :apple:
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/33280/index.html https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/33280/index.html
Android iOS
Desktop :computer: Web :spider_web:
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/33280/NewExpensify.dmg https://33280.pr-testing.expensify.com
Desktop Web
staszekscp commented 4 months ago

Hey @mountiny! When would be possible to start testing the changes? 😄

s77rt commented 4 months ago

@adamgrzybowski Can you please merge main / resolve conflicts? I see some changes related the LHP that should not be a part of this PR

WojtekBoman commented 4 months ago

Conflicts have been resolved :)

s77rt commented 4 months ago

How is this supposed to look on Web? Not seeing much currently

Screenshot 2024-01-04 at 11 37 54 AM
s77rt commented 4 months ago
  1. Missing icon on the bottom tabs
  2. We should not keep navigating to a page that are we are currently in
  3. Navigation direction should be different when going to Chats

https://github.com/Expensify/App/assets/16493223/3b19eea1-2131-42c4-bc8e-c88f0bda7e11

s77rt commented 4 months ago

Question: What is the role of the Expensify top-left logo button?

https://github.com/Expensify/App/assets/16493223/e7e50849-4df4-49ed-adc0-f54bd2eb865c

github-actions[bot] commented 4 months ago
:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube: Android :robot: iOS :apple:
❌ FAILED ❌ https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/33280/index.html
The QR code can't be generated, because the android build failed iOS
Desktop :computer: Web :spider_web:
❌ FAILED ❌ https://33280.pr-testing.expensify.com
The QR code can't be generated, because the Desktop build failed Web

:eyes: View the workflow run that generated this build :eyes:

s77rt commented 4 months ago

Help and Go Expensify Classing missing the external link icon

Screenshot 2024-01-04 at 12 25 13 PM
kosmydel commented 4 months ago

Help and Go Expensify Classing missing the external link icon

The designs don't include this icon. Do you think the icons should be visible here? cc @shawnborton

Screenshot 2024-01-04 at 12 27 36

kosmydel commented 4 months ago

How is this supposed to look on Web? Not seeing much currently <img alt="Screenshot 2024-01-04 at 11 37 54 AM"

Oh, we forgot to mention. To got everything working you have to do the following:

rm -rf node_modules
npm i

We made some changes in a patch file and simple npm i doesn't re-apply patches.

kosmydel commented 4 months ago

Question: What is the role of the Expensify top-left logo button?

We haven't implemented this part yet - the workspace switcher. We are currently working on it.

mountiny commented 4 months ago

Question: What is the role of the Expensify top-left logo button?

This will be workspace switcher, where you can filter the app functionality on workspace basis

s77rt commented 4 months ago

I love the new web design <3

s77rt commented 4 months ago
  1. Going to Profile does not have the Profile highlighted
  2. Going to AllSettings does not have the Workspace highlighted (and it can't be highlighted on a second click)
  3. There is a back button in the Workspace that should not be there (at least for Web)

https://github.com/Expensify/App/assets/16493223/34ae90c0-e61e-417e-832c-da0b053fd8d1

s77rt commented 4 months ago

Delete popover is misplaced

Screenshot 2024-01-04 at 12 53 05 PM

Same with workspace avatar change button

Screenshot 2024-01-04 at 12 55 39 PM
s77rt commented 4 months ago

Bug / By design? The settings in the workspace do not take full width

Screenshot 2024-01-04 at 1 03 21 PM
s77rt commented 4 months ago

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

https://github.com/Expensify/App/assets/16493223/5ab90a13-1f2a-4556-aa6b-6829ac5b1f72

s77rt commented 4 months ago

Not a bug: Now that the workspace name field is pushed on it's own page we should auto focus the input

Screenshot 2024-01-04 at 1 18 10 PM
s77rt commented 4 months ago

Crash when trying to view a workspace avatar

https://github.com/Expensify/App/assets/16493223/59b55792-3174-414f-835a-5f8500b41386

s77rt commented 4 months ago

(similar to https://github.com/Expensify/App/pull/33280#issuecomment-1876967788) If you refresh the page while RHP is open the highlighted item in Settings list is lost

https://github.com/Expensify/App/assets/16493223/695b289f-408b-4f0b-9296-1a44bec5a776

s77rt commented 4 months ago

Workspace links all redirects to the overview and you can't refresh a page that you are currently in

https://github.com/Expensify/App/assets/16493223/bb6c8baa-8dbc-437a-986d-593ca286f89a

s77rt commented 4 months ago

URL naming inconsistency:

shawnborton commented 4 months ago

Bug / By design? The settings in the workspace do not take full width

That is by design, but we need some modifications here. @adamgrzybowski can you please do the following:

cc @dannymcclain @dubielzyk-expensify for extra eyes on this PR too.

shawnborton commented 4 months ago

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

Yeah, ideally this flow should start on the same page (in this case the travel page) but just launched in the RHP.

s77rt commented 4 months ago
  1. Workspace name overflows
  2. There is extra spacing in the Workspace initial settings page at the top and no back button
  3. Inner pages are broken e.g. Invoices
  4. All pages have a top padding except the Members page, the Invite button is too close to the top (not necessary a bug)

https://github.com/Expensify/App/assets/16493223/1947b7c6-4396-4a72-80e7-b3a432656ab5

shawnborton commented 4 months ago

Also I wanted to clarify, the Workspace name and page name should not be in a scrollable view. I think just the content below it (the subnav items) would be scrollable. I saw that from the video above, here is a screenshot: CleanShot 2024-01-04 at 14 46 23@2x

So how it should behave: basically everything in this top outlined rectangle is fixed, but the content in the rectangle below (the sub nav options) can scroll vertically: CleanShot 2024-01-04 at 14 47 06@2x

shubham1206agra commented 4 months ago

@shawnborton Can you provide figma link here if possible to check the styling layout in detail (spacing, color, etc.)?

shawnborton commented 4 months ago

@shubham1206agra I put a couple references for you here: https://www.figma.com/file/Xv3yHJooZcxRGkXbAcTZD5/%23ideal-nav-(Ideal-Nav)?type=design&node-id=2684%3A72517&mode=design&t=larByJRbckmqvl1c-1

shubham1206agra commented 4 months ago

Requested access

s77rt commented 4 months ago

As an anonymous user:

  1. Sign in button overlaps
  2. The search opens in RHP (not a blocker - it may not be related to this PR, didn't check)
  3. Can create workspace

https://github.com/Expensify/App/assets/16493223/5537ae48-1fea-4e63-9457-f240ae9043c9

s77rt commented 4 months ago

Animation inconsistency iOS / Android

https://github.com/Expensify/App/assets/16493223/f419ed05-8f1e-4f75-afc0-a5bcdae46f9f

s77rt commented 4 months ago
  1. Can't open https://dev.new.expensify.com:8082/settings/wallet from chat it always redirects to /profile
  2. The invite page https://dev.new.expensify.com:8082/workspace/0680EF3C70411F57/invite opens in RHP but keeps the central screen unchanged. Then when you press the assistance button and go back the central pane changes to /members and the bottom tab navigator stays on Chats

https://github.com/Expensify/App/assets/16493223/1cac26f8-dd68-43dd-ace4-db3ca4b046d9

s77rt commented 4 months ago
  1. Going to Workspace -> any workspace causes some a flicker (probably related to https://github.com/Expensify/App/pull/33280#issuecomment-1877362108)
  2. Going back takes you to the workspace initial page instead of the workspaces list

https://github.com/Expensify/App/assets/16493223/123f03fc-6a4b-433b-a582-7d78e807a07e

s77rt commented 4 months ago

(Similar to https://github.com/Expensify/App/pull/33280#issuecomment-1877440041 Clicking the get assistance button changes the central pan screen

https://github.com/Expensify/App/assets/16493223/af397fcd-a2fe-4694-bf32-13e1415b0467

shawnborton commented 4 months ago

One thing I want to mention here, the bottom FAB button should be at our large button size of 52x52, with the icon inside of it at 20x20: CleanShot 2024-01-05 at 09 27 38@2x

I'll update those two mocks I shared in Figma as well.

mountiny commented 4 months ago

@MaciejSWM @WojtekBoman @kosmydel Whenever you think this is ready for a new build once some bugs are fixed feel free to let me know I will kick it off

s77rt commented 4 months ago

Low priority bug: Go to Settings -> Preferences -> Go back to Settings and increase screen width. The central pan is empty

https://github.com/Expensify/App/assets/16493223/bf8299af-43d9-474f-89d3-469e8ee09f6a

s77rt commented 4 months ago

Avatars misalignment

Screenshot 2024-01-05 at 7 04 34 PM
s77rt commented 4 months ago
  1. Double offline indicator in small screens
  2. Missing offline indicator in wide screens
Screenshot 2024-01-05 at 7 01 14 PM Screenshot 2024-01-05 at 7 01 59 PM Screenshot 2024-01-05 at 7 02 10 PM
s77rt commented 4 months ago

On App open no bottom tab item is highlighted (native only)

Screenshot 2024-01-05 at 9 11 58 PM
s77rt commented 4 months ago

Missing border bottom in some pages e.g. Profile, Workspaces, etc.

Screenshot 2024-01-07 at 10 01 05 PM Screenshot 2024-01-07 at 10 01 12 PM
s77rt commented 4 months ago

The back behavior in "App download links" is wrong

https://github.com/Expensify/App/assets/16493223/a9f8c5d5-18ab-452d-bd05-e0e119ca5703

s77rt commented 4 months ago

(Could be related to https://github.com/Expensify/App/pull/33280#issuecomment-1876993340) Once you start VBBA process you get stuck and can't go back

https://github.com/Expensify/App/assets/16493223/074f019d-5389-4d74-b587-230eabf489c0

s77rt commented 4 months ago

Go to Workspace Settings and go though the settings (3+) in wide layout, then resize to small window. Pressing go back should take you directly to WorkspaceInitialPage. Instead you'd be going to the previous pages and weirdly end up passing by the Report screen. (reduceReportRoutes probably have some relation here)

https://github.com/Expensify/App/assets/16493223/fba078e1-fdbc-492a-b31b-075e5b30ffce

s77rt commented 4 months ago

Brick road indicator not implemented correctly (still in progress?)

  1. RBR is shown on Profile but it should not (going through the Profile route won't take you to where the error is)
  2. Most the pages are missing RBR

https://github.com/Expensify/App/assets/16493223/e01fb75e-5df6-41f6-bcff-58faea0b6cc7

kosmydel commented 4 months ago

Missing border bottom in some pages e.g. Profile, Workspaces, etc.

It was discussed here.

Attaching screenshot, as the channel probably is private:

Screenshot 2024-01-08 at 09 48 57

It will be done as a follow-up.

shubham1206agra commented 4 months ago

@MaciejSWM Can you fix merge conflicts and start an adhoc build?

kosmydel commented 4 months ago

@MaciejSWM Can you fix merge conflicts and start an adhoc build?

It's not ready yet for another build. We will have more information tomorrow as @adamgrzybowski will be back from OOO.