[Main Tracking] App Navigation Reboot

[puneetlath]

Design doc link - https://docs.google.com/document/d/1cQ87HcDA9RrmBM4ch7PmlNi9N9ZP8abQuPF0gKYrxDE/edit

Problem:

We initially implemented react-navigation in App without a proper design doc as part of the NewDot MVP. While this library came with some awesome benefits - we’ve realized over time that some of our early decisions in how we use the library have impacted performance and usability (i.e. bugs and lots of em). Our current design also deviates from our ideal design and needs to be reworked.

Solution:

Let’s establish a ruleset of navigational actions a user can take and define which gestures or interactions relate to those navigational actions. Then we’ll re-implement React Navigation with those rules in mind. We will kill the drawer navigator favoring a simpler stack navigator approach. And finally, unblock the bugs and make our app navigation competitive and bug free.

Timeline: WhatsApp Quality Level Urgency 🔥

Plan of Action:

• #whatsnext link
• Send Strategy@ email
• Predesign links 1, 2, 3
• HL Design Doc link
• Detailed Design Doc
• Implement New Navigation
• Unblock bugs by fixing or closing them

Implementation PRs

• [HOLD for payment 2023-06-15] [Navigation Reboot] Update the react-navigation version #15846
• [Navigation Reboot] Implement and use the createStackNavigator #15847
• [HOLD for payment 2023-06-15] [Navigation Reboot] Update RHP to be encapsulated in another navigator #15848
• [HOLD for payment 2023-06-15] [Navigation Reboot] Implement the navigation UP function  #15849
• [HOLD for payment 2023-06-15] [HOLD for payment 2023-05-26] [Navigation Reboot] Remove the X from modal screens #15850
• [HOLD for payment 2023-06-15] [Navigation Reboot] Create the ResponsiveNavigator #15851
• [HOLD for payment 2023-06-15] [HOLD for payment 2023-05-09] [Navigation Reboot] Remove the drawer navigator #15852
• [Navigation Refactor] Fix the Drop to upload feature  #18064
• [HOLD for payment 2023-06-15] [Navigation Refactor] Pronouns page has no animation when opening #18092
• [Navigation Reboot] Make the UP button more responsive #18603
• [HOLD for payment 2023-06-15] [Navigation Refactor] Implement the Stack navigation into NewDot #19665
• [HOLD for payment 2023-07-24] [Navigation Refactor] Follow-ups from the migration PR #20320
• [Polish] Make the RHP animations always working on web and they are smooth #20342
• Tests:
  • [Navigation Reboot] Add regression tests for new navigation reboot #15853
  • [HOLD for payment 2023-06-23] [Navigation Reboot] Add NAVIGATION.md file to the repository #15854

Overview

This GH will serve as the tracking issue for improving our React Navigation implementation. A bunch of issues with navigation have been found and are referenced in this project.

List of related issues

Currently fall into two major categories

Drawer Navigation / State

• Drawer state is unpredictable
  • [$500] Page goes to home screen if press chat and refresh the page - reported by @phivh #8126
  • Closing certain pages (drawers) navigates to the wrong URL #11111
  • IOS - Chat - Chat conversation is shifted and Fub menu partially visible after Take a picture and sent it #11961
  • [HOLD #11768] [$1000] Login - After creating a new account the user is not brought to Concierge conversation #11948
  • BUG: [mweb-safari] keyboard remains open while navigating back to LHN using fingers from right to left reported by @gadhiyamanan #11970
  • [$1000] Fix navigating to workspace chat when a new invited user logs in #12041
• Drawer state transition end can not be listened for - [HOLD - App Navigation Reboot] [$500] mWeb- Previous chat username is displayed when you navigate back from different chat @Yash Chauhan & @Puneet-here #9253
• Home screen / MainDrawerNavigator is not getting route params from Report route
• Switching from large to small width screens breaks search navigation (actually the drawer is just stuck closed)
  • [HOLD - #11768] [$500] Unable to open chat through search when the browser/app size is decreased - reported by @adeel0202 #9734
  • [HOLD] [$250] Web - Firefox- Search box - New chat selected through the search box does not open correct chat conversation #9342
• Navigating to search page using a link while logged out ends up in empty option list after signing in
  • [HOLD for payment 2023-05-23] [$1000] Search page shows blank list after login while trying to open that deeplink url reported by @0xmiroslav  #12717
  • [HOLD #11768][$1000] First name, last name not pre-filled on Profile page after login while trying to open that deeplink url reported by @0xmiroslav  #12719
  • [HOLD #11768] [$250] Report detail page is opened and closed immediately after login while trying to open that deeplink url reported by @0xmiroslav #12428
  • [$250] Workspace - That page is nowhere to be found error message appears for a brief moment reported by @thesahindia  #12616
• [HOLD #11768] [$1000] Hide create a workspace page when app is loading #12128
• [HOLD for payment 2023-06-13] [$1000] Infinite loading while opening Concierge deeplink #16319
• [HOLD #11768] mWeb - Chat - The page starts to shake when user went back to the conversation #17147

Browser History

• Back navigation / history stack for reports on web is hacked in
• mWeb navigating back exits the chat (report route not added to history
  • [HOLD #11768] Navigating Back takes the user to the home screen on the web - reported by @parasharrajat #10507
  • [HOLD #11768] Navigating Back takes the user to the home screen on the web - reported by @parasharrajat #10506
  • [HOLD #11768] mWeb-lands on wrong screen when pressing the back button from the attachment screen @gadhiyamanan #11372
• mWeb visual bug on Safari related to popstate event
  • [HOLD #11768] [$8000] mWeb - Swipe to go back on safari mobile app flickers - reported by @adeel0202 #7737
• Back/Forward shortcut keys do not work
  • [HOLD #11768] [$8000] The Back ⌘[ and Forward ⌘] shortcuts keys are not working as expected when navigating via RHN (Right Hand Navigation) #8657
  • [HOLD #11768] [$16000] Desktop - The Back ⌘[ and Forward ⌘] shortcuts keys are not working as expected in LHN #4612
  • [HOLD #11768] mWeb-lands on wrong screen when pressing the back button from the attachment screen @gadhiyamanan #11372
  • [HOLD #11768] Web - Pressing back after refresh dismisses the modal on many pages @Puneet-here #11420
  • [Hold #11768] mWeb- Connect Bank Account - Pressing back button at android in the middle of a step takes you back to the workspace page instead of going back to the previous step @Puneet-here #11429
• After deleting stuff, navigating back can lead to a 404
  • [$250] Android/IOS - Blank screen and an error displays while trying to delete a workspace right after creating it @adeel0202 #11524
• [HOLD #11768] - Pressing X closes the chat in mobile web reported by @Puneet-here #11453
• [HOLD #11768] Web- Chat - Transition using the back button does not work correctly #10409
• [HOLD #11768] Wrong URL when new chat is selected from new chat list - Reported by @aneequeahmad #11092
• [HOLD #11768] mWeb / Android - Popovers aren't closing on navigating back in native - reported by @thesahindia #9823
• [HOLD] [$250] mWeb- LHN - Forward arrow doesn't work when pressed again #12203
• [HOLD on #15212] Bug: Mark as unread state vanishes after closing the app in IOS app @chauchausoup #11515
• [HOLD #11768] [$1000] Android - Split Bill - Keyboard is not dismissed for a brief moment after changing currency #13449
• [HOLD #11768] Connect bank account manually page shows up on pressing "connect bank account" #13743
• [HOLD] When pressing back from Step 2 (Connect Bank Manually), it navigate to Connect Bank Account page. #14973
• [HOLD App Navigation Reboot #11768] [$1000] Web - Double click on message on user details page opens group with that user if group chat with that user is the latest chat #16557

Performance

• [HOLD App Navigation Reboot] React navigation StackNavigator causes screen content to load slowly #14036
• [HOLD #11768] Screens are unmounting unnecessarily when navigating between them #14173

Misc.

• [Tracking] Web/Desktop - Browser Resizing  #13971
• [$1000] Pressing space bar while details page is opened, scrolls up the reports #15631
• [HOLD #11768] [$8000] Chat room wobbles while opening up on mobile Chrome or Safari #15239
• [HOLD #11768] After opening the timezones deeplink once, pressing enter/return on any other page takes the user back to the timezone page #16512

[parasharrajat]

Get Control of React-Navigation

🔥 🔥

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[puneetlath]

Suggestion from @roryabraham: move src/libs/Navigation to src/Navigation to highlight its importance to someone coming to the repo for the first time.

[puneetlath]

Another interesting note from @roryabraham: we don't use React Navigation's link component.

[melvin-bot]

@puneetlath Eep! 4 days overdue now. Issues have feelings too...

[puneetlath]

Reached out to React Navigation library owner, but haven't heard back. Will follow up again this week as well as with perhaps some others.

[puneetlath]

Jason will be the BZ person tag teaming this with me, so assigning to him to the issue as well.

[JmillsExpensify]

This is on staging. Let the real-world testing begin!

[mountiny]

The main PR just got merged and stack navigation is available in Staging. I have compiled a list of some clean up and follow up tasks in this issue, working on adding a documentation to the main about the navigation.

[roryabraham]

Seeking guidance on whether this is now expected behavior: #20370

It seems like the answer is probably "yes" given the new stack navigator?

[mountiny]

Answered in the issue

[hayata-suenaga]

I have read and reviewed this Design Doc!

[mountiny]

The migration has been done at the end of the last week and it went surprisingly smooth. I am not tracking issues related to the new navigation in this public project https://github.com/orgs/Expensify/projects/37 and trying to iron them out with SWM.

With Jason, once we get a bit of time we will go through all the issues on hold and see which are fixed and which need to be addressed still

[mallenexpensify]

I put the below on hold, pending the navigation reboot. If anyone disagrees, please comment on the issue

• [HOLD #11768] App navigates back to main chat if user clicks on invalid link in reply in thread and refreshes on 'Hmm its not here' page #21229

[mountiny]

I will check it out, the refactor is deployed now and we are going through some follow up issues and we will go though all the issues on hold to test them and close issues we cannot repro

[mountiny]

Added all the relevant issues to the https://github.com/orgs/Expensify/projects/39 and to those we need to go through and confirm if they are fixed are added to the TODO column.

[mountiny]

I will try to close this out this week and send a wrap up!

[JmillsExpensify]

And I am still working on testing the linked issue. Should be able to finish it up this week.

[mountiny]

Going to send out project wrap up

[mountiny]

🗺️🧭 It’s time for the Navigation Reboot wrap-up 🎉

Its been almost 9 months since we embarked on the journey to update out navigation concepts in newDot to have more reliable and predictable navigation pattern across all our supported platforms. And today, this issue can be finally closed 🎯

This was one of the first projects where we have fully outsourced to an external agency, Software Mansion. I am also tagging @adamgrzybowski and @WoLewicki who have done amazing job navigating unknown codebase, learning our design doc process and delivering this solution with care and urgency. Thank you guys 👏

What went well

• We have closed over 28 bugs (more to be closed but we haven't got through them yet) which were on hold for this project. You can see them in this Github issue 💪

• We now have way more predictable navigation system which is easier to use and has less custom actions (which had steep learning curve for anyone new to the the codebase)

• This project has shown we can effectively expand our teams through the expert agencies if we can work using the “buddy” system and find a project that suits them. In this case, basically 100% of the implementation has been done in the App which was ideal.

• It has been a great learning experience on how to work with the expert agencies, I haven’t got to this yet, but I would love to start some SO where we can share best practices on how to work as a “buddy” and other engineers can share their experiences there too.

• The detailed section of the design doc has been very much written by the external agency and I have only helped with minor things and resolving the threads. The doc has got great feedback, hopefully we can do more docs like this with SWM 🎀. This quality has helped to save internal engineering time.

• The implementation has went quite smooth, it was a major change in the codebase, but there were no major regressions or blockers and we managed to address any issues after the “switch" in timely manner.

What we learned

• Lots about how to work with the external agencies. This is probably not surprising, but to allow them to work as efficiently as possible, the buddy should be proactive and be available as much as possible whenever they get stuck/ have questions. Given they usually have this single project as their main focus, if they are stuck on something, they will be wasting their time until we help them to get unstuck.

• It can take a lot of time to manage such project due to the above and being in similar timezone helps a lot to keep it going faster (move to Europe) but I certainly believe these projects can unlock so much productivity and high quality front-end code that we would hardly achieve in the same timeframe without the experts.

• Using Github Projects is quite handy for managing the open source projects. It would be even better if we could let the expert contributors move the card within the projects on their own, but we would most likely have to make them all part of our Organization.

What we could have done better

• This one could be in the previous category but: whenever the project is discussing a bit more abstract or ambivalent terms and concepts, its crucial to keep the conversation grounded in written down and well defined terminology. Always make sure to come back to the terminology section whenever the conversation deviates or you have a feeling someone is using some term with different meaning than was agreed on. We have had lots of discussions on what should the Up, Back, Push, etc actions do within the App and often people got confused about what they actually mean and wasted a lot of time as people were not on the same page with these terms important for the project.

  • Let’s make sure that whenever there are some non-trival terms in some discussions you are having in public rooms, assume people know nothing and take your time explaining the terms to get everyone on the same page to get the most out of the conversation.

• The project could have been completed faster especially if we would have repeatedly not run into confusing situations when discussing the design because of the above mentioned problem.

What’s next

We have ironed out most of the follow up bugs we have faced, but we are not ending here. I am tracking all of the follow up issues in this github project. There are 2 bigger issues addressed:

- Resizing the browser window from wide to narrow layout or viceversa make the components to loose their state - Adam already has [an active PR](https://github.com/Expensify/App/pull/22437) to address this by changing the layout to style based layout so we dont have to remount the navigators and state of components is not lost 🎉

- Trying to fix the browser back button behaviour. Browsers intentionally make it hard to mess with its internal history/ state. This makes it hard for us in many situations, but for example [here](https://github.com/Expensify/App/issues/22101), when user clicks to leave a room or a thread, they are navigated to some other page, but when they click browser back, this naturally takes them to the previous URL, which is the thread they have just left. Its not a huge problem but[ SWM is working](https://expensify.slack.com/archives/C04878MDF34/p1689872643785089?thread_ts=1689871481.652229&cid=C04878MDF34) on potential update to react-navigation which would allow us to handle this better.

Thanks everyone! Onwards and upwards! 🚀

Vit, Adam and Wojti from SWM, and also Marc and Vivek from the high level stages

[adeel0202]

Hi @mountiny, there was this issue that was initially held because of the navigation reboot but later it was closed without the reporting bonus. Can you please check?

[mallenexpensify]

Addressing @adeel0202's above comment here