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

Removing modal on page route change #143

Open
nik32 opened this issue Jan 10, 2024 · 10 comments
Open

Removing modal on page route change #143

nik32 opened this issue Jan 10, 2024 · 10 comments
Labels
enhancement New feature or request

Comments

@nik32
Copy link

nik32 commented Jan 10, 2024

Kudos for such a great library! It has really simplified working with modals

The modals don't get removed automatically on route change. Can this feature be added to NiceModal such that developers can configure (at the time of using the modal with useModal) to close the modal on route change.

@supnate supnate added the enhancement New feature or request label Jan 11, 2024
@supnate
Copy link
Collaborator

supnate commented Jan 11, 2024

Thanks @nik32 , we will consider this requirement.

@ArturKustyaev
Copy link

Thanks @nik32 , we will consider this requirement.

I would be very happy about this feature. Right now I have to remove all modals on a page via NiceModal.remove() in useEffect

@nik32
Copy link
Author

nik32 commented Feb 1, 2024

@supnate really sorry for troubling you with this... but any update regarding this? Actually, folks in our company wanted to go to production with this feature... that's why asking you.

If I can provide any assistance from my end, please feel free to let me know

@supnate
Copy link
Collaborator

supnate commented Feb 1, 2024

Hi @nik32 ,

I'm just curious why page route changes when a modal is still visible, is that a common pattern in your app or just edge cases?
If just in few places, you can try declarative way. If a common pattern, the problem equals to "when a component call NiceModal.show to show a modal, the modal should auto close when the component is unmounted.". For now I've not figured out a proper solution to this. And if this is implemented, it intends to be a break change. So I'm still thinking about a good solution.

@nik32
Copy link
Author

nik32 commented Feb 1, 2024

Thanks @supnate for the reply!

Basically the user can press the back button in the browser (while the modal is still open). He intended to go to the previous page but the modal doesn't belong to the previous page (thus our folks wanted that the modal should be closed on route change).

We are using NiceModal on a lot of our pages so this is a common pattern. And thus you are right... the problem does equal to "when a component call NiceModal.show to show a modal, the modal should auto close when the component is unmounted."

And if this is implemented, it intends to be a break change. So I'm still thinking about a good solution.

With regards to it being a breaking change... while calling useModal... can we allow an optional property (something along the lines of removeOnUnmount)... which will close the modal when the component (in which useModal is called) unmounts? Will this still be a breaking change?

@supnate
Copy link
Collaborator

supnate commented Feb 4, 2024

Hi @nik32 ,

Thanks for the explanation.

What do you think of the solution of closing all modals while route change?
You can get all modals' ids with NiceModalContext API and you can add a listener to route change where you can close all modals by NiceModal.hide(modalId).

@nik32
Copy link
Author

nik32 commented Feb 5, 2024

@supnate thanks for helping us come up with a solution!

This might work... but I need to give it a try first. In the solution... can you help me with a couple of doubts -

  1. Can you help me with how to use NiceModalContext to extract all the modal ids open on a page?

  2. Also I haven't provided any ids to modals (as I was using useModal() and thus never needed to explicitly provide any id to the modal)... so will I be needing to provide id to all my modals for this solution to work?

@kainstar
Copy link

kainstar commented Feb 6, 2024

@supnate thanks for helping us come up with a solution!

This might work... but I need to give it a try first. In the solution... can you help me with a couple of doubts -

  1. Can you help me with how to use NiceModalContext to extract all the modal ids open on a page?
  2. Also I haven't provided any ids to modals (as I was using useModal() and thus never needed to explicitly provide any id to the modal)... so will I be needing to provide id to all my modals for this solution to work?

@nik32

NiceModalContext's value is just a plain object record (id -> modal), you can use Object.keys or other API iterate them

Here is my code:

const location = useLocation(); // react-router-dom v5 API

const niceModalContext = useContext(NiceModal.NiceModalContext);

useEffect(() => {
  Object.keys(niceModalContext).forEach((key) => {
    NiceModal.hide(key);
  });
}, [location.pathname, location.search]);

I don't use modal.remove because the method would be called after modal's transition animation end in modal component

@supnate
Copy link
Collaborator

supnate commented Feb 6, 2024

Thanks @kainstar for the example!

Every modal always has an id, it auto generates one internally if not given.

@nik32
Copy link
Author

nik32 commented Feb 6, 2024

Thanks @supnate and @kainstar for helping us! This solution works!

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

No branches or pull requests

4 participants