fix(lib/expo_router_plugin): closing modal when navigating through ide panel #204
Conversation
|
@kewinzaq1 is attempting to deploy a commit to the software-mansion Team on Vercel. To accomplish this, @kewinzaq1 needs to request access to the Team. Afterwards, an owner of the Team is required to accept their membership request. If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account. |
kewinzaq1
changed the title
kewinzaq1
force-pushed
the
hotfix/closing-modal-through-ide-url-panel
branch
from
9eb036d
to
687363b
Compare
kewinzaq1
changed the title
|
@kmagiera @filip131311 @kacperkapusciak Could you check this? |
| @@ -0,0 +1,11 @@ | |||
| function onRequestRouteChange({ router, pathname, params }) { | |||
| // in order to close a modal (https://github.com/expo/expo/issues/26922#issuecomment-1996862878) | |||
| router.dismissAll(); | |||
There was a problem hiding this comment.
wouldn't this always close all the modals? What if you just want to go one step back on the currently closed modal?
There was a problem hiding this comment.
@kmagiera Good catch, thank you, This behavior was obnoxious, but now it should work smoothly. I've replaced dismissAll and push with navigate and setParams; this approach allows resolving the very first problem without introducing additional one. Could you check this, please?
before.mov
now.mov
| router.dismissAll(); | ||
| setTimeout(() => { | ||
| router.push(pathname, params); | ||
| }, 0); |
There was a problem hiding this comment.
Suggestion: setImmediate may show intent more clearly.
There was a problem hiding this comment.
@jakub-gonet thx for the suggestion, I replaced the previous approach with a new one and probably it's no longer needed. Could you check this?
The fix solves the problem of closing too many modals in the stack. For example, if you had opened four modals in the stack and just wanted to navigate from the fourth modal to the second modal, and this modal was filled with some pieces of information, for instance, in a form, it was all gone. Now it moves to the second modal smoothly, keeping the state in place.
|
@patrycjakalinska Why is this issue closed? |
|
@kewinzaq1 Sorry for this inconvenience, this shouldn't have been closed - reopening it now :) |
|
Hi @kewinzaq1 – apparently we 're unable to re-open the PR. It is likely because your fork is private since it's been forked while the repo was still private. If you could make your fork public I believe we should be able to bring it back |
|
HI @kmagiera now it's public |
|
I'm sorry @kewinzaq1 but it seems like something is messed up on GitHub here. Every time I click re-open it immediately closes. Would you be able to open new PR with this? |
# The problem: Currently, navigating to another page is impossible if a modal is opened. The root cause of the problem is using `router.push` that 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 `navigate` combined with `setParams`. This approach works smoothly without introducing additional problems. # Before: https://github.com/software-mansion/react-native-ide/assets/77052270/5b26f4cb-24cf-4c13-a57d-f33f9a20d680 By replacing the current implementation with `navigate` and `setParams ` we can eliminate the problem. # After: https://github.com/software-mansion/react-native-ide/assets/77052270/ff425ade-8f77-4144-bf42-18368870ee86
The problem:
Screen_Recording_2024-05-03_at_11.54.50.mov
More info:
