feat(Animation): add logic for mount/remove children#2115
Conversation
-spreading unhandled props in Animation and Popper components
mnajdova
requested review from
dvdzkwsk,
layershifter,
levithomason and
miroslavstastny
as code owners
-improved duration type
-added example
-used Transition component inside the Animation -removed Animation root element
| onExiting={onExiting} | ||
| onExited={onExited} | ||
| > | ||
| {state => result} |
There was a problem hiding this comment.
I am currently not using the state, as in the usage I am changing the animation. We don't have the children as function concept anywhere else so far, so I didn't add it for the Animation component. (it would be easy to migrate if we decide in the future!)
There was a problem hiding this comment.
Does this work for adding/removing table items within the Table component or tree items within Tree?
There was a problem hiding this comment.
It should work, if not, we can Implement the AnimationGroup component in a similar way of how the TransitionGroup (https://reactcommunity.org/react-transition-group/transition-group) component is implemented from the same library). But, to be honest, I would try to avoid adding different APIs and try to implement it with this API
| exportedAtTopLevel?: boolean | ||
| rendersPortal?: boolean | ||
| wrapperComponent?: React.ReactType | ||
| handlesAsProp?: boolean |
There was a problem hiding this comment.
As the Animation is not rendering root element, but only copies the children, we don't have the "as" prop there.
| "react-fela": "^10.6.1", | ||
| "react-is": "^16.6.3" | ||
| "react-is": "^16.6.3", | ||
| "react-transition-group": "^4.3.0" |
There was a problem hiding this comment.
New dependency added
Changed dependencies are detected.Changed dependencies in
Perf comparison
Generated by 🚫 dangerJS |
| unmountOnExit?: boolean | ||
|
|
||
| /** The duration of the transition, in milliseconds. */ | ||
| timeout?: number | { enter?: number; exit?: number; appear?: number } |
There was a problem hiding this comment.
based on your comment, should timeout actually be named duration?
There was a problem hiding this comment.
ah, nevermind. i see that it is what it's called in react-transition-group... https://reactcommunity.org/react-transition-group/transition#Transition-prop-timeout
There was a problem hiding this comment.
Yeah, this two are different concepts (duration is the css animationDuraion), and this is timeout for removing the elements. Maybe I should come up with a better name...
mnajdova
changed the title
mnajdova
requested review from
JasonGore,
jdhuntington and
kenotron
as code owners
|
Hi, just a heads up that I made a change #2153 renaming all |
# Conflicts: # CHANGELOG.md # packages/react/package.json # packages/react/src/components/Animation/Animation.tsx # packages/react/src/themes/teams/components/Animation/animationStyles.ts
# Conflicts: # CHANGELOG.md # packages/react/package.json # packages/react/src/components/Animation/Animation.tsx # packages/react/src/themes/teams/components/Animation/animationStyles.ts
# Conflicts: # CHANGELOG.md
-updated snapshots
| onEntered?: (node: HTMLElement, isAppearing: boolean) => void | ||
|
|
||
| /** Callback fired before the "exiting" status is applied. */ | ||
| onExit?: (node: HTMLElement) => void |
There was a problem hiding this comment.
| onExit?: (node: HTMLElement) => void | |
| onExit?: () => void |
Do we want to expose node there? 🤔
There was a problem hiding this comment.
This is the API from the Transition component itself, I would rather not change anything for now...
There was a problem hiding this comment.
But maybe we should return here null as first argument for the event, and the node afterwards, to be consistent with the other callbacks. What do you think?
Maybe I am over-complicating, but let me know what you think @layershifter
5 participants
This PR adds the option on the
Animationcomponent, to wait with the mounting of the component until it is visible, as well as remove the dom element if the element is hidden.For this, the
react-transition-groupdependency was added, and theTransitioncomponent is used behind the scenes from theAnimationcomponent. The API exposed is partially the Transition's API.There are example usages in the changes.
BREAKING CHANGE
Other change is that, the Animation component is not rendering the additional div anymore, it is rendering only the children.