Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When navigating to the same report, don't reset the navigation state #3945

Merged
merged 3 commits into from
Jul 9, 2021
Merged

When navigating to the same report, don't reset the navigation state #3945

merged 3 commits into from
Jul 9, 2021

Conversation

rdjuric
Copy link
Contributor

@rdjuric rdjuric commented Jul 9, 2021

Details

Check if we're attempting to navigate to the active report. If we are, reveals the report and do not reset the navigation state.

Fixed Issues

$ #3893

Tests

  1. Opened a chat on the app
  2. Attempted to navigate to that chat again (via a notification or directly)
  3. The chat was revealed and the navigation state wasn't reseted

QA Steps

  1. Open any chat on the app
  2. Attempt to navigate to that chat again (via a notification or directly)
  3. The chat should be revealed and the navigation state shouldn't be reseted (the component isn't being remounted and the Pusher isn't being reconnected)

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web.mov

Mobile Web

Desktop

desktop.mov

iOS

Android

I'm having a hard time adding videos for mobile as it isn't receiving the notifications. I tested on mobile by emulating the behavior of a notification click somewhere in a modal.

@rdjuric rdjuric requested a review from a team as a code owner July 9, 2021 04:07
@MelvinBot MelvinBot requested review from joelbettner and removed request for a team July 9, 2021 04:08
const rootState = navigationRef.current.getRootState();
const activeReportID = rootState.routes[0].state.routes[0].params.reportID;
const activeReportID = lodashGet(rootState, 'routes[0].state.routes[0].params.reportID', '')
Copy link
Contributor Author

@rdjuric rdjuric Jul 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look very good but should be safe. Maybe a better idea would be making this a getActiveReport(rootState) function that searches for a route of type === 'drawer' and get's the reportID from it?

Not sure.

Copy link
Contributor

@joelbettner joelbettner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@joelbettner joelbettner merged commit 16ff439 into Expensify:main Jul 9, 2021
@OSBotify
Copy link
Contributor

OSBotify commented Jul 9, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.77-5🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants