fix: closing modal through ide url panel #316
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
kewinzaq1
changed the title
kmagiera
requested changes
May 29, 2024
There was a problem hiding this comment.
Thanks for sending this. It looks ok, however I think it'd be easier to just add the same code in both v2 and v3 implementation rather than extracting it out. There is already some code duplication and we aim to have as few runtime files as possible. On top of that we will likely phase out v2 implementation so it wouldn't make any sense once we only have one in place.
kewinzaq1
force-pushed
the
fix--closing-modal-when-navigating-through-ide-panel
branch
from
44f7255
to
786cc06
Compare
kmagiera
approved these changes
Jun 3, 2024
|
Thanks for this, it will go out in the next update |
kewinzaq1
deleted the
fix--closing-modal-when-navigating-through-ide-panel
branch
kmagiera
added a commit
that referenced
this pull request
Jun 12, 2024
kmagiera
added a commit
that referenced
this pull request
Jun 12, 2024
kmagiera
added a commit
that referenced
this pull request
Jun 12, 2024
kmagiera
added a commit
that referenced
this pull request
Jun 12, 2024
In #316 we introduced a regression with expo-router v2 integration. The used `router.navigate` method is only available in v3 and hence we revert that modification change for expo-router v2 plugin. This PR effectively reverts changes from #316 made to packages/vscode-extension/lib/expo_router_v2_plugin.js file
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem:
Currently, navigating to another page is impossible if a modal is opened. The root cause of the problem is using
router.pushthat does not dismiss modals. You can read more about it.expo/expo#26922 (comment) .
However approaches that they proposed aren't good because they introduce other bugs such as unnecessarily closing modals, because of this - thanks to @kmagiera catch (#204 (comment)) I eventually bet on
navigatecombined withsetParams. This approach works smoothly without introducing additional problems.Before:
Screen_Recording_2024-05-03_at_11.54.50.mov
By replacing the current implementation with
navigateandsetParamswe can eliminate the problem.After:
now.mov