When using the component Transition, the content "appears" instead of having the default behavior of not appearing on the first mount.
The component should not appear on the first mount, as stated on the documentation.
Steps:
https://codesandbox.io/s/nice-hooks-fpi84?file=/src/Demo.js
| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.11.0 |
| React | 16.13.1 |
| Browser | Google Chrome 84.0.4147.125 (Official Build) (64-bit) |
| TypeScript | 3.9.7 |
I just noticed that here in the Material UI the appear is actually default to true. But why?
This problem seems to be another #11683 or #21924 more recently. We have a prop that is inherited from somewhere else, that we don't document, but yet, we change its default value.
Displaying the props with its custom default prop as with the others could be our "best worse" option. Any better UX in mind? (without considerations for the implementation)
@eps1lon What solution do you have in mind with? What will be the UX?
our API docs will get a major overhaul
But why?
I'm never going to find it, but I do vaguely recall a discussion back when the transitions were added as to what would be the most sane default. @oliviertassinari is right though - it should be documented.
@eps1lon What solution do you have in mind with? What will be the UX?
The way this default is setup is different to any pattern we're currently using.
diff --git a/packages/material-ui/src/Grow/Grow.js b/packages/material-ui/src/Grow/Grow.js
index 6ab04f846d..f3b01095c9 100644
--- a/packages/material-ui/src/Grow/Grow.js
+++ b/packages/material-ui/src/Grow/Grow.js
@@ -28,6 +28,7 @@ const styles = {
*/
const Grow = React.forwardRef(function Grow(props, ref) {
const {
+ appear = true,
children,
in: inProp,
onEnter,
@@ -154,7 +155,7 @@ const Grow = React.forwardRef(function Grow(props, ref) {
return (
<TransitionComponent
- appear
+ appear={appear}
in={inProp}
nodeRef={nodeRef}
onEnter={handleEnter}
is the smallest possible change right now (+ yarn proptypes && yarn docs:api) to add documentation. Just a word of caution: This is backwards incompatible if you had <Grow appear={undefined} /> which would lead to <Transition appear={false} /> but is now <Transition appear={true} />. Probably very unlikely to break but could be possible when spreading props around. We can use v5 for that.
Though I don't think we ever had this default documented.
Though I don't think we ever had this default documented.
@eps1lon As far as I know, we never had it documented. Maybe we could have a new prop disableAppear to have the default as false and avoid the change of behavior.
Hum, maybe not. I'm all 馃挴 for the proposed changes: https://github.com/mui-org/material-ui/issues/22162#issuecomment-673984372.
Maybe we could have a new prop disableAppear to have the default as false and avoid the change of behavior.
I think we can make an argument the documenting appear as a default implies that <Grow appear={undefined} /> leads to <Transition appear={true} />. Let's just call it a bug (appear should default to true; previously omitting appear was treated differently than setting appear to undefined) and schedule it for v5 to reduce risk of unwanted breakage.
Hey! Can I take this issue?
Hey! Can I take this issue?
@fakeharahman All yours. Please let us know if you no longer intend to work on this issue. If you're stuck at any point feel free to open a PR so a maintainer can take a look.
Hey, I got stuck on this so if someone else wants to take this, they are the most welcome to! I will still try to solve this.
Hi! I can take this if it's okay! 馃槃
Most helpful comment
I think we can make an argument the documenting
appearas a default implies that<Grow appear={undefined} />leads to<Transition appear={true} />. Let's just call it a bug (appearshould default totrue; previously omittingappearwas treated differently than settingappeartoundefined) and schedule it for v5 to reduce risk of unwanted breakage.