feat(modal): prevent location change if modal is open #1334
Conversation
You are in a state that modal is open. User accidentally clicks back button on the browser. You use a state manager, for example ui-router, so page transition happens, without page refresh. Now, you have a completely different page, completely different state, but you have modal open. This PR prevents URL change if modal is open.
|
@ubenzer this is one possible approach. Alternatively we could simply dismiss all the modals on the location change. I really can't decide which one is better. In any case this change will need a passing unit test before it can be merged. |
|
I am not good at JavaScript unit testing and I have no idea about how to simulate browser url change. I thought about including ngRouter or uiRouter to testing environment but this is not something that makes sense for unit testing I guess... I am kinda confused. I'll look to uiRouter's tests once I have time, hoping there might be some samples that help me. |
|
@ubenzer you don't need ui-rooter to write a test, you just need to simulate a page change by triggering the locationStart event. But once again, my main problem here is the conceptual one - should we do what you are proposing or rather close modals on location change... |
|
@pkozlowski-opensource I understand, and agree with your problem. It is probably better to close modals on location change. I'll implement that too and see it in action in our app, a real app with real problems to see it fits better. I'll do that and update my PR when I have some time. P.S. I've never though about triggering event manually, thanks for the tip. :) |
You are in a state that modal is open. User clicks back button on the browser. You use a state manager, for example ui-router, so page transition happens, without page refresh. Now, you have a completely different page, completely different state, but you have modal open. To prevent this situation, modals are closed automatically on `$locationChangeStart` Developer may prevent this behaviour by passing `closeOnNavigation: false` as option.
|
@pkozlowski-opensource I've updated the PR. It closes modal, but people are able to prevent it by |
|
@ubenzer sorry, it took me a bit of time to come back to you regarding this one, but finally I've found some time to review your latest change and left some comments in the code. WDYT? Also, could you please rebase your branch on the latest master and squash all the commits into one? Thnx! |
|
@pkozlowski-opensource Your concerns are right, I'll update my PR when I find time. Thanks for feedbacks, happy new year. :) |
|
Has this been removed from the project? I cannot find this pull request merged in the project. May be I'm missing something haha |
|
Yea what happened to this feature? This doesn't work for me either with latest version. |
You are in a state that modal is open. User accidentally clicks back
button on the browser. You use a state manager, for example ui-router,
so page transition happens, without page refresh. Now, you have a
completely different page, completely different state, but you have
modal open.
This PR prevents URL change if modal is open.