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

Improve transition documentation #17047

Closed
2 tasks done
cmeeren opened this issue Aug 18, 2019 · 9 comments · Fixed by #19201
Closed
2 tasks done

Improve transition documentation #17047

cmeeren opened this issue Aug 18, 2019 · 9 comments · Fixed by #19201
Labels
good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request
Milestone

Comments

@cmeeren
Copy link
Contributor

cmeeren commented Aug 18, 2019

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

The Dialog API docs do not seem to specify signatures for the handlers. I tried looking at the Dialog source code and eventually ended up at the react-transition-group API docs, which seems to call the enter functions with HtmlElement (node?) and bool, whereas the exit functions are only called with HtmlElement.

Is this behaviour that should be documented, or are these implementation details that should not be part of MUI's API surface?

@mbrookes mbrookes added component: dialog This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation labels Aug 18, 2019
@mbrookes
Copy link
Member

Since we document the react-transition-group props, we should document the signature. On the other hand, we don't explicitly document these props for the transitions themselves, but cover it under inheritance.

Meanwhile Popover and Snackbar also pass through and document these props (but not the signature), while ExpansionPanel, StepContent, Tooltip, SpeedDial and TreeItem don't.

I wonder if the best approach would be to have a transitionProps prop, the description for which references the react-transition-group props?

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 19, 2019

I wonder if the best approach would be to have a transitionProps prop, the description for which references the react-transition-group props?

Seems cleaner to me. 👍

@mbrookes mbrookes added this to the v5 milestone Aug 19, 2019
@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 20, 2019

Menu also documents these props (but not the signature).

@mbrookes

This comment has been minimized.

@cmeeren

This comment has been minimized.

@mbrookes

This comment has been minimized.

@cmeeren

This comment has been minimized.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 21, 2019

@mbrookes I'm not sure to understand the proposal. Here is a summary of the situation (as far as I can see):

Component onX props TransitionProps TransitionComponent
Dialog x x x
Snackbar x x x
Menu x x (inheritance) x (inheritance)
Popover x x x
ExpansionPanel x x
SpeedDial x x
StepContent x x
Tooltip x x
TreeItem x

From that, I conclude that

  • We should implement the TransitionProps prop to the tree item.
  • It's not obvious what props can be provided. I hope the diff solve this problem:
   /**
-   * Props applied to the `Transition` element.
+   * Props applied to the [`Transition`](http://reactcommunity.org/react-transition-group/transition#Transition-props) element.
    */
   TransitionProps: PropTypes.object,
 };
  • It's not obvious what interface the custom transition should respect. I would propose:
diff --git a/docs/src/pages/components/transitions/transitions.md b/docs/src/pages/components/transitions/transitions.md
index 66d4e8fb1..cbefb0e4a 100644
--- a/docs/src/pages/components/transitions/transitions.md
+++ b/docs/src/pages/components/transitions/transitions.md
@@ -77,3 +77,13 @@ Expand outwards from the center of the child element.
 This example also demonstrates how to delay the enter transition.

 {{"demo": "pages/components/transitions/SimpleZoom.js"}}
+
+## TransitionComponent prop
+
+A few of the Material-UI components accept a `TransitionComponent` prop to customize the transition. You can use any of the above components or a home-made one.
+It should respect the following conditions:
+
+- Accepts an `in` prop. This corresponds to the open / close state.
+- Call the `onEnter` callback prop when the enter transition starts.
+- Call the `onExited` callback prop when the exit transition is completed.
+These two callbacks allow to unmount the children when in a closed state and fully transitioned.
   /**
    * The component used for the transition.
+   * [Follow our guide](/components/transitions/#transitioncomponent-prop) to learn more about the requirements for this component.
    */
   TransitionComponent: PropTypes.elementType,
  • 👍 for deprecating the onX props in v4 and removing them in v5.

@mbrookes

This comment has been minimized.

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Nov 30, 2019
@oliviertassinari oliviertassinari changed the title [Dialog] API docs missing handler signature for onEnter/onEntering/onEntered/onExit/onExiting/onExited Improve transition documentation Nov 30, 2019
@oliviertassinari oliviertassinari added new feature New feature or request and removed component: dialog This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation labels Nov 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants