eBay / nice-modal-react

A modal state manager for React.
https://ebay.github.io/nice-modal-react
MIT License
1.96k stars 109 forks source link

Removing modal on page route change #143

Open nik32 opened 5 months ago

nik32 commented 5 months ago

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 commented 5 months ago

Thanks @nik32 , we will consider this requirement.

ArturKustyaev commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

@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 commented 4 months ago

Thanks @kainstar for the example!

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

nik32 commented 4 months ago

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