material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[material-ui][docs] Document Drawer onClose reason in v4

Open albertchae opened this issue 1 year ago • 2 comments

The type shows it is the same as Modal's onClose https://github.com/mui/material-ui/blob/v4.x/packages/material-ui/src/Drawer/Drawer.d.ts#L31-L36

So I copied the documentation line about reason from Modal.d.ts, ran yarn proptypes and yarn docs:api

This generates almost the same doc as modal.md, but there is an additional line in the modal documentation that wasn't present in Modal.d.ts 🤷 https://github.com/mui/material-ui/blob/v4.x/docs/pages/api-docs/modal.md?plain=1#L54

The reason parameter can optionally be used to control the response to onClose

This is already correctly documented in v5

One caveat is that I am targeting the v4.x branch since this documentation needs to be backported there

albertchae avatar Jul 22 '24 12:07 albertchae

Hey @albertchae, thanks for working on this, and sorry for the late review.

I agree with the change. This .md content is generated from Drawer.d.ts, so may I ask you to modify that instead (You can copy it from modal as well) and run yarn proptypes and then yarn docs:api? That should modify the docs content accordingly.

DiegoAndai avatar Aug 26 '24 20:08 DiegoAndai

Thanks @DiegoAndai , I followed those steps and pushed my changes with updated PR description. One slightly weird thing I noticed compared to when I manually copied from modal.md is this:

This generates almost the same doc as modal.md, but there is an additional line in the modal documentation that wasn't present in Modal.d.ts 🤷 https://github.com/mui/material-ui/blob/v4.x/docs/pages/api-docs/modal.md?plain=1#L54

The reason parameter can optionally be used to control the response to onClose

albertchae avatar Aug 27 '24 17:08 albertchae

Hey @albertchae, sorry for the delay. If you're still interested in merging this change, may I ask you to work on the failing CI tests?

DiegoAndai avatar Dec 27 '24 15:12 DiegoAndai