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

chore: Chore/12628 create navigation provider #12649

Open
wants to merge 55 commits into
base: main
Choose a base branch
from

Conversation

Cal-L
Copy link
Contributor

@Cal-L Cal-L commented Dec 11, 2024

Description

This PR cleans up the NavigationService and moves the navigation provider further up in the app stack. This ensures that the provider's children has access to the navigation object. This should reduce the number of cases where navigation is accessed before the navigation provider is ready. In theory, this should eliminate the navigation issue WRT WalletConnect service altogether - #12628. The metric to measure this improvement is looking at the reduction of this event - https://metamask.sentry.io/issues/4909236583/?environment=production&project=2299799&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20%22navigation%22&referrer=issue-stream&sort=date&statsPeriod=90d&stream_index=0

Related issues

Fixes: #12628

Manual testing steps

  • App should function normally
  • Build the app as usual
  • Navigation works as soon as app starts
  • Deeplinking works as expected

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@Cal-L Cal-L requested review from a team as code owners December 11, 2024 19:49
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@Cal-L Cal-L added Run Smoke E2E Triggers smoke e2e on Bitrise needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Dec 11, 2024
Copy link
Contributor

github-actions bot commented Dec 11, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: e923754
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c7c6c977-bb4e-4b33-8766-ac71e9cd63e4

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@Cal-L Cal-L changed the title chore: Chore/12628 create navigation gate chore: Chore/12628 create navigation provider Dec 11, 2024
@Cal-L Cal-L added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Dec 13, 2024
Copy link
Contributor

github-actions bot commented Dec 13, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: b466761
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/75f42644-9d93-45ab-9b3c-1d50121290be

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@Cal-L Cal-L added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Dec 13, 2024
Copy link
Contributor

github-actions bot commented Dec 13, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 64c2bcb
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/bf386fc2-eb46-48a8-adb5-5e7b30ed34bc

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

Copy link

sonarcloud bot commented Dec 13, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) Run Smoke E2E Triggers smoke e2e on Bitrise team-mobile-platform
Projects
Status: Needs dev review
Development

Successfully merging this pull request may close these issues.

Move navigation gate earlier in the app stack to better control app start flow
2 participants