nice-modal-react icon indicating copy to clipboard operation
nice-modal-react copied to clipboard

muiDialog Function is Outdated

Open af185178 opened this issue 2 years ago • 3 comments

Hello!

All of the modals in our app are based on MUI Dialogs, but we are unable to use the muiDialog function because it's passing the deprecated onExited prop. This results in a console warning that we are passing deprecated props and should be using TransitionProps instead.

See the note here in the MUI v4 documentation, also the corresponding MUI v5 documentation here.

I think the function should be using TransitionProps like so:

export const muiDialog = (modal: NiceModalHandler): { open: boolean; onClose: () => void; TransitionProps: { onExited: () => void } } => {
  return {
    open: modal.visible,
    onClose: () => modal.hide(),
    TransitionProps: {
      onExited: () => {
        modal.resolveHide();
        !modal.keepMounted && modal.remove();
      },
    },
  };
};

Overall great package though guys thanks for your work!

af185178 avatar Jun 14 '22 14:06 af185178

Hi @af185178 , thanks for the report!

I will update docs about it. For v5, we suggest not using the helper method because there may be additional props necessary on TransitionProps by user. If we use <Dialog ...muiDialog(modal) /> then it's hard for user to understand what will happen if they put TransitionProps either before or after ...muiDialog(modal).

Or, you can use:

const muiProps = muiDialog(modal);
//...
<Dialog {...muiProps} TransitionProps={{onExited: muiProps.onExited}} />

supnate avatar Jun 17 '22 02:06 supnate

Hey @supnate thanks for getting back to me

That makes sense, I can see why you wouldn't want to set TransitionProps in the helper function.

If that's the case though, should onExited be removed from muiDialog? In the example you gave, I think we would still get the console warning since we're passing onExited in muiProps, which seems to be deprecated in all uses. Is there ever a case where onExited should be passed outside of TransitionProps?

af185178 avatar Jun 17 '22 13:06 af185178

hmm, maybe I can added a new helper named museDialogV5 as you mentioned, or if you can add a PR is welcome🙂

export const muiDialogV5 = (modal: NiceModalHandler): { open: boolean; onClose: () => void; TransitionProps: { onExited: () => void } } => {
  return {
    open: modal.visible,
    onClose: () => modal.hide(),
    TransitionProps: {
      onExited: () => {
        modal.resolveHide();
        !modal.keepMounted && modal.remove();
      },
    },
  };
};

The only thing for user to understand is they need construct full TransitionProps after the helper method if necessary. It seems to be convenient since I think most cases of MuiDialog usage don't customize TransitionProps.

supnate avatar Jun 18 '22 04:06 supnate

Resolved by: https://github.com/eBay/nice-modal-react/commit/5b3dbae4c1f884c2d653bc89416062a514b29132

supnate avatar Sep 22 '22 12:09 supnate