Quernest / mui-modal-provider

🌞 Context API and Hooks based Modal Provider for react material-ui framework
MIT License
77 stars 10 forks source link

destroy is always called regardless of options. #78

Open michaeltford opened 1 year ago

michaeltford commented 1 year ago

disableAutoDestroy: false and destroyOnClose : false are not honored (at least not always).

https://github.com/Quernest/mui-modal-provider/blob/master/src/modal-provider.tsx#L165

The onExited should have an options (options.destroyOnClose || disableAutoDestroy) check before calling destroy?

Quernest commented 1 year ago

disableAutoDestroy: false and destroyOnClose : false are not honored (at least not always).

https://github.com/Quernest/mui-modal-provider/blob/master/src/modal-provider.tsx#L165

The onExited should have an options (options.destroyOnClose || disableAutoDestroy) check before calling destroy?

Hi @michaeltford, thank you for your comment :)

In the current implementation, the function show creates a unique key with a reference to the dialog component, its props, and options.

And the destroy function clears that key.

This is needed so that when opening/closing windows the state does not grow.

I don't see a problem with it. However, I have an interesting idea about reusing the key and optionally deleting it, for example, if the keepMounted prop is set to true

What do you think?

michaeltford commented 1 year ago

@Quernest, thank you for the response and sorry for the late response. I didn't realize that you had responded to me :-(.

I agree with your point about not wanting to grow and a keepMounted would be an alternative. I thought that is what disabledAutoDestory effective did. My use case is that I have a find dialog (that I don't want to unmount because it has a bunch of transient state) on hide.