feat(Animation): add logic for mount/remove children #2115

mnajdova · 2019-11-15T13:29:58Z

This PR adds the option on the Animation component, 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-group dependency was added, and the Transition component is used behind the scenes from the Animation component. 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.

-spreading unhandled props in Animation and Popper components

-improved duration type

-added example

packages/react/src/components/Animation/Animation.tsx

-used Transition component inside the Animation -removed Animation root element

mnajdova · 2019-11-18T13:59:17Z

packages/react/src/components/Animation/Animation.tsx

+        onExiting={onExiting}
+        onExited={onExited}
+      >
+        {state => result}


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!)

Does this work for adding/removing table items within the Table component or tree items within Tree?

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

packages/react/src/components/Menu/MenuItem.tsx

mnajdova · 2019-11-18T14:00:15Z

packages/react/test/specs/commonTests/isConformant.tsx

@@ -27,6 +27,7 @@ export interface Conformant {
  exportedAtTopLevel?: boolean
  rendersPortal?: boolean
  wrapperComponent?: React.ReactType
+  handlesAsProp?: boolean


As the Animation is not rendering root element, but only copies the children, we don't have the "as" prop there.

mnajdova · 2019-11-18T14:00:52Z

packages/react/package.json

@@ -26,7 +26,8 @@
    "popper.js": "^1.15.0",
    "prop-types": "^15.7.2",
    "react-fela": "^10.6.1",
-    "react-is": "^16.6.3"
+    "react-is": "^16.6.3",
+    "react-transition-group": "^4.3.0"


New dependency added

DustyTheBot · 2019-11-18T14:18:21Z

	Warnings
⚠️	Package (or peer) dependencies changed. Make sure you have approval before merging!

Changed dependencies are detected.

Changed dependencies in packages/react/package.json

package	before	after
react-transition-group	-	^4.3.0

Perf comparison

Status	Scenario	Fluent TPI	Fabric TPI	Ratio	Iterations	Ticks
🔧	Avatar.Fluent	0.56	0.4	1.4:1	2000	1111
🔧	Button.Fluent	1.26	0.17	7.41:1	1000	1258
🔧	Checkbox.Fluent	1.4	0.3	4.67:1	1000	1397
🔧	Dialog.Fluent	0.32	0.18	1.78:1	5000	1602
🔧	Dropdown.Fluent	3.67	0.53	6.92:1	1000	3665
🔧	Icon.Fluent	0.3	0.04	7.5:1	5000	1515
🔧	Image.Fluent	0.11	0.09	1.22:1	5000	559
🔧	Slider.Fluent	2.25	0.35	6.43:1	1000	2246
🦄	Text.Fluent	0.07	0.19	0.37:1	5000	330
🦄	Tooltip.Fluent	0.36	19.54	0.02:1	5000	1791

🔧 Needs work 🎯 On target 🦄 Amazing

Generated by 🚫 dangerJS

codepretty · 2019-11-19T05:33:51Z

packages/react/src/components/Animation/Animation.tsx

+  unmountOnExit?: boolean
+
+  /** The duration of the transition, in milliseconds. */
+  timeout?: number | { enter?: number; exit?: number; appear?: number }


based on your comment, should timeout actually be named duration?

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

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...

packages/react/src/lib/positioner/Popper.tsx

packages/react/src/components/Menu/MenuItem.tsx

packages/react/src/components/Animation/Animation.tsx

ecraig12345 · 2019-12-10T20:51:50Z

Hi, just a heads up that I made a change #2153 renaming all lib folders to utils. So the changes to lib files in this PR will need to be moved to the new location.

# 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

-remove craft

-updated snapshots

CHANGELOG.md

layershifter · 2020-01-27T11:14:37Z

packages/react/src/components/Animation/Animation.tsx

+  onEntered?: (node: HTMLElement, isAppearing: boolean) => void
+
+  /** Callback fired before the "exiting" status is applied. */
+  onExit?: (node: HTMLElement) => void


Suggested change

onExit?: (node: HTMLElement) => void

onExit?: () => void

Do we want to expose node there? 🤔

This is the API from the Transition component itself, I would rather not change anything for now...

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

packages/react/src/components/Animation/Animation.tsx

packages/react/src/utils/positioner/Popper.tsx

mnajdova added 2 commits November 15, 2019 14:21

-added logic for removing/mounting elements in the Animation

0ee6b54

-spreading unhandled props in Animation and Popper components

Merge branch 'master' into feat/animation-remove-element

5181822

mnajdova requested review from dvdzkwsk, layershifter, levithomason and miroslavstastny as code owners November 15, 2019 13:29

mnajdova added 3 commits November 15, 2019 14:33

-reverted changed to MenuItem and Popper

dcf91de

-added MenuItem example

da5b6f1

-improved duration type

-added callbacks

9089ab1

-added example

layershifter reviewed Nov 15, 2019

View reviewed changes

packages/react/src/components/Animation/Animation.tsx Outdated Show resolved Hide resolved

mnajdova added 5 commits November 18, 2019 12:11

-added react transition group as dependency

74ce784

-used Transition component inside the Animation -removed Animation root element

-reorder Animation -> Ref

72a21f4

-fixed type, added default for timeout prop

8fcbd96

-fixed Animation test

d201eca

-updated animation example

1f85a49

mnajdova commented Nov 18, 2019

View reviewed changes

packages/react/src/components/Menu/MenuItem.tsx Outdated Show resolved Hide resolved

mnajdova commented Nov 18, 2019

View reviewed changes

-added comments on the new props

68fe185

codepretty reviewed Nov 19, 2019

View reviewed changes

-added @types

d3c24ba

mnajdova changed the title ~~[WIP] feat(Animation): add logic for mount/remove children~~ feat(Animation): add logic for mount/remove children Nov 28, 2019

mnajdova added the 💥 blocked label Nov 28, 2019

ecraig12345 mentioned this pull request Dec 6, 2019

Rename lib to utils #2153

Merged

layershifter reviewed Dec 9, 2019

View reviewed changes

packages/react/src/lib/positioner/Popper.tsx Outdated Show resolved Hide resolved

layershifter reviewed Dec 9, 2019

View reviewed changes

packages/react/src/components/Menu/MenuItem.tsx Outdated Show resolved Hide resolved

layershifter reviewed Dec 9, 2019

View reviewed changes

packages/react/src/components/Animation/Animation.tsx Show resolved Hide resolved

layershifter reviewed Dec 9, 2019

View reviewed changes

packages/react/src/components/Animation/Animation.tsx Outdated Show resolved Hide resolved

mnajdova requested review from JasonGore, jdhuntington and kenotron as code owners December 9, 2019 19:04

mnajdova added 6 commits December 16, 2019 11:29

-addressed some PR comments

013d5ec

Merge branch 'master' into feat/animation-remove-element

3366212

-example changes

43f308c

-updated changelog

a06b0dd

-updated changelog

8b569e1

-fixed caniuse-lite

4a1df35

mnajdova removed the 💥 blocked label Dec 16, 2019

mnajdova added 7 commits January 21, 2020 16:44

Merge branch 'master' into feat/animation-remove-element

fa39708

# Conflicts: # CHANGELOG.md # packages/react/package.json # packages/react/src/components/Animation/Animation.tsx # packages/react/src/themes/teams/components/Animation/animationStyles.ts

Merge branch 'master' into feat/animation-remove-element

74c0832

# Conflicts: # CHANGELOG.md # packages/react/package.json # packages/react/src/components/Animation/Animation.tsx # packages/react/src/themes/teams/components/Animation/animationStyles.ts

Merge branch 'master' into feat/animation-remove-element

044dfcd

# Conflicts: # CHANGELOG.md

-fixes

fd8ef3d

-remove craft

-fixed tests

c419c3d

-updated snapshots

-updated approve packages

e2f9b7d

-improved and fixed tests

a9c4d0d

layershifter reviewed Jan 27, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

layershifter reviewed Jan 27, 2020

View reviewed changes

packages/react/src/components/Animation/Animation.tsx Show resolved Hide resolved

layershifter reviewed Jan 27, 2020

View reviewed changes

packages/react/src/components/Animation/Animation.tsx Show resolved Hide resolved

layershifter reviewed Jan 27, 2020

View reviewed changes

packages/react/src/components/Animation/Animation.tsx Outdated Show resolved Hide resolved

layershifter reviewed Jan 27, 2020

View reviewed changes

packages/react/src/utils/positioner/Popper.tsx Outdated Show resolved Hide resolved

mnajdova added 2 commits January 27, 2020 12:37

-addressed PR comments

9c3d858

-changed callbacks

f6ad15c

layershifter approved these changes Jan 27, 2020

View reviewed changes

layershifter added ⚙️ enhancement New feature or request 🚀 ready for review labels Jan 27, 2020

mnajdova merged commit f16bf46 into master Jan 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Animation): add logic for mount/remove children #2115

feat(Animation): add logic for mount/remove children #2115

mnajdova commented Nov 15, 2019 •

edited

Loading

mnajdova Nov 18, 2019

codepretty Nov 19, 2019 •

edited

Loading

mnajdova Nov 19, 2019

mnajdova Nov 18, 2019

mnajdova Nov 18, 2019

DustyTheBot commented Nov 18, 2019 •

edited

Loading

codepretty Nov 19, 2019

codepretty Nov 19, 2019

mnajdova Nov 19, 2019

ecraig12345 commented Dec 10, 2019

layershifter Jan 27, 2020

mnajdova Jan 27, 2020

mnajdova Jan 27, 2020

feat(Animation): add logic for mount/remove children #2115

feat(Animation): add logic for mount/remove children #2115

Conversation

mnajdova commented Nov 15, 2019 • edited Loading

Choose a reason for hiding this comment

codepretty Nov 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DustyTheBot commented Nov 18, 2019 • edited Loading

Changed dependencies are detected.

Perf comparison

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecraig12345 commented Dec 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnajdova commented Nov 15, 2019 •

edited

Loading

codepretty Nov 19, 2019 •

edited

Loading

DustyTheBot commented Nov 18, 2019 •

edited

Loading