From af2af4327494e04b3cf3388fa892d952d15a6ee1 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Mon, 10 Feb 2020 13:25:34 +0100 Subject: [PATCH 1/7] chore(Animation): refactor to FC --- .../src/components/Animation/Animation.tsx | 224 ++++++++++++------ .../react/src/themes/teams/componentStyles.ts | 2 - .../src/themes/teams/componentVariables.ts | 2 - .../components/Animation/animationStyles.ts | 22 -- .../Animation/animationVariables.ts | 1 - 5 files changed, 145 insertions(+), 106 deletions(-) delete mode 100644 packages/react/src/themes/teams/components/Animation/animationStyles.ts delete mode 100644 packages/react/src/themes/teams/components/Animation/animationVariables.ts diff --git a/packages/react/src/components/Animation/Animation.tsx b/packages/react/src/components/Animation/Animation.tsx index cc586f87d5..57ea72b513 100644 --- a/packages/react/src/components/Animation/Animation.tsx +++ b/packages/react/src/components/Animation/Animation.tsx @@ -1,17 +1,20 @@ -import * as PropTypes from 'prop-types' -import * as React from 'react' +import { + ComponentAnimationProp, + getUnhandledProps, + unstable_createAnimationStyles as createAnimationStyles, + unstable_getStyles as getStyles, + useTelemetry, +} from '@fluentui/react-bindings' import cx from 'classnames' import * as _ from 'lodash' +import * as PropTypes from 'prop-types' +import * as React from 'react' +// @ts-ignore +import { ThemeContext } from 'react-fela' import { Transition } from 'react-transition-group' -import { - UIComponent, - childrenExist, - StyledComponentProps, - commonPropTypes, - ShorthandFactory, -} from '../../utils' -import { ComponentEventHandler } from '../../types' +import { childrenExist, StyledComponentProps, commonPropTypes } from '../../utils' +import { ComponentEventHandler, ProviderContextPrepared } from '../../types' export type AnimationChildrenProp = (props: { classes: string }) => React.ReactNode @@ -139,83 +142,99 @@ export interface AnimationProps extends StyledComponentProps { /** * An Animation provides animation effects to rendered elements. */ -class Animation extends UIComponent { - static create: ShorthandFactory - - static className = 'ui-animation' - - static displayName = 'Animation' - - static propTypes = { - ...commonPropTypes.createCommon({ - accessibility: false, - as: false, - content: false, - children: false, - }), - children: PropTypes.oneOfType([PropTypes.func, PropTypes.element]), - name: PropTypes.string, - delay: PropTypes.string, - direction: PropTypes.string, - duration: PropTypes.string, - fillMode: PropTypes.string, - iterationCount: PropTypes.string, - keyframeParams: PropTypes.object, - playState: PropTypes.string, - timingFunction: PropTypes.string, - visible: PropTypes.bool, - appear: PropTypes.bool, - mountOnEnter: PropTypes.bool, - unmountOnExit: PropTypes.bool, - timeout: PropTypes.oneOfType([ - PropTypes.number, - PropTypes.shape({ - appear: PropTypes.number, - enter: PropTypes.number, - exit: PropTypes.number, - }), - ]), - onEnter: PropTypes.func, - onEntering: PropTypes.func, - onEntered: PropTypes.func, - onExit: PropTypes.func, - onExiting: PropTypes.func, - onExited: PropTypes.func, - } - - static defaultProps = { - timeout: 0, - } - - handleAnimationEvent = ( +const Animation: React.FC & { + className: string + handledProps: (keyof AnimationProps)[] +} = props => { + const context: ProviderContextPrepared = React.useContext(ThemeContext) + const { setStart, setEnd } = useTelemetry(Animation.displayName, context.telemetry) + setStart() + + const { + appear, + children, + delay, + direction, + duration, + fillMode, + iterationCount, + keyframeParams, + mountOnEnter, + name, + playState, + timeout, + timingFunction, + visible, + unmountOnExit, + } = props + + const handleAnimationEvent = ( event: 'onEnter' | 'onEntering' | 'onEntered' | 'onExit' | 'onExiting' | 'onExited', ) => () => { - _.invoke(this.props, event, null, this.props) + _.invoke(props, event, null, props) } - renderComponent({ ElementType, classes, unhandledProps }) { - const { children, mountOnEnter, unmountOnExit, timeout, appear, visible } = this.props - - const isChildrenFunction = typeof children === 'function' - - const child = - childrenExist(children) && - !isChildrenFunction && - (React.Children.only(children) as React.ReactElement) - - return ( + const { classes } = React.useMemo(() => { + const animation: ComponentAnimationProp = { + name, + keyframeParams, + duration, + delay, + iterationCount, + direction, + fillMode, + playState, + timingFunction, + } + + return getStyles({ + className: Animation.className, + displayName: Animation.displayName, + props: { + styles: createAnimationStyles(animation, context.theme), + }, + + disableAnimations: context.disableAnimations, + renderer: context.renderer, + rtl: context.rtl, + saveDebug: _.noop, + theme: context.theme, + _internal_resolvedComponentVariables: context._internal_resolvedComponentVariables, + }) + }, [ + context, + name, + delay, + direction, + duration, + fillMode, + iterationCount, + keyframeParams, + playState, + timingFunction, + ]) + const unhandledProps = getUnhandledProps(Animation.handledProps, props) + + const isChildrenFunction = typeof children === 'function' + const child = + childrenExist(children) && + !isChildrenFunction && + (React.Children.only(children) as React.ReactElement) + + const element = ( + <> @@ -223,8 +242,55 @@ class Animation extends UIComponent { ? () => (children as AnimationChildrenProp)({ classes: classes.root }) : child} - ) - } + + ) + setEnd() + + return element +} + +Animation.className = 'ui-animation' +Animation.displayName = 'Animation' + +Animation.defaultProps = { + timeout: 0, +} +Animation.propTypes = { + ...commonPropTypes.createCommon({ + accessibility: false, + as: false, + content: false, + children: false, + }), + children: PropTypes.oneOfType([PropTypes.func, PropTypes.element]), + name: PropTypes.string, + delay: PropTypes.string, + direction: PropTypes.string, + duration: PropTypes.string, + fillMode: PropTypes.string, + iterationCount: PropTypes.string, + keyframeParams: PropTypes.object, + playState: PropTypes.string, + timingFunction: PropTypes.string, + visible: PropTypes.bool, + appear: PropTypes.bool, + mountOnEnter: PropTypes.bool, + unmountOnExit: PropTypes.bool, + timeout: PropTypes.oneOfType([ + PropTypes.number, + PropTypes.shape({ + appear: PropTypes.number, + enter: PropTypes.number, + exit: PropTypes.number, + }), + ]), + onEnter: PropTypes.func, + onEntering: PropTypes.func, + onEntered: PropTypes.func, + onExit: PropTypes.func, + onExiting: PropTypes.func, + onExited: PropTypes.func, } +Animation.handledProps = Object.keys(Animation.propTypes) as any export default Animation diff --git a/packages/react/src/themes/teams/componentStyles.ts b/packages/react/src/themes/teams/componentStyles.ts index 2da1609a24..007b33b122 100644 --- a/packages/react/src/themes/teams/componentStyles.ts +++ b/packages/react/src/themes/teams/componentStyles.ts @@ -103,8 +103,6 @@ export { default as Tree } from './components/Tree/treeStyles' export { default as TreeItem } from './components/Tree/treeItemStyles' export { default as TreeTitle } from './components/Tree/treeTitleStyles' -export { default as Animation } from './components/Animation/animationStyles' - export { default as Video } from './components/Video/videoStyles' export { default as Tooltip } from './components/Tooltip/tooltipStyles' diff --git a/packages/react/src/themes/teams/componentVariables.ts b/packages/react/src/themes/teams/componentVariables.ts index 025fd37ba0..e43df336c2 100644 --- a/packages/react/src/themes/teams/componentVariables.ts +++ b/packages/react/src/themes/teams/componentVariables.ts @@ -85,8 +85,6 @@ export { default as ToolbarMenuRadioGroup } from './components/Toolbar/toolbarMe export { default as TreeTitle } from './components/Tree/treeTitleVariables' -export { default as Animation } from './components/Animation/animationVariables' - export { default as Video } from './components/Video/videoVariables' export { default as Tooltip } from './components/Tooltip/tooltipVariables' diff --git a/packages/react/src/themes/teams/components/Animation/animationStyles.ts b/packages/react/src/themes/teams/components/Animation/animationStyles.ts deleted file mode 100644 index aeacc8904d..0000000000 --- a/packages/react/src/themes/teams/components/Animation/animationStyles.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { - ComponentAnimationProp, - unstable_createAnimationStyles as createAnimationStyles, -} from '@fluentui/react-bindings' - -export default { - root: ({ props: p, theme }) => { - const animation: ComponentAnimationProp = { - name: p.name, - keyframeParams: p.keyframeParams, - duration: p.duration, - delay: p.delay, - iterationCount: p.iterationCount, - direction: p.direction, - fillMode: p.fillMode, - playState: p.playState, - timingFunction: p.timingFunction, - } - - return createAnimationStyles(animation, theme) - }, -} diff --git a/packages/react/src/themes/teams/components/Animation/animationVariables.ts b/packages/react/src/themes/teams/components/Animation/animationVariables.ts deleted file mode 100644 index ead516c976..0000000000 --- a/packages/react/src/themes/teams/components/Animation/animationVariables.ts +++ /dev/null @@ -1 +0,0 @@ -export default () => {} From af260bb68b9e44c73af9a7f567fbf2bead33247c Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Mon, 10 Feb 2020 13:32:54 +0100 Subject: [PATCH 2/7] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a720508ed8..0cb4c515bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,11 +19,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### BREAKING CHANGES - Restricted prop set in the `Checkbox`, `Icon`, `Label`, `Slider`, `Status`, `Text` @layershifter ([#2307](https://github.com/microsoft/fluent-ui-react/pull/2307)) +- Styles for the `Animation` component are removed from Teams theme @layershifter ([#2258](https://github.com/microsoft/fluent-ui-react/pull/2258)) ### Fixes - Remove dependency on Lodash in TypeScript typings @layershifter ([#2323](https://github.com/microsoft/fluent-ui-react/pull/2323)) - Fix missing aria-describedby in AlertBehavior @delprzemo ([#2208](https://github.com/microsoft/fluent-ui-react/pull/2208)) +### Performance +- Styles for `Animation` component are computed again only on prop changes @layershifter ([#2258](https://github.com/microsoft/fluent-ui-react/pull/2258)) + ## [v0.44.0](https://github.com/microsoft/fluent-ui-react/tree/v0.44.0) (2020-02-05) [Compare changes](https://github.com/microsoft/fluent-ui-react/compare/v0.43.2..v0.44.0) From 1a599ddf3e42b6da7bcb43bf9b6ba861db138376 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Mon, 10 Feb 2020 13:35:50 +0100 Subject: [PATCH 3/7] cleanup tests --- .../components/Animation/Animation-test.tsx | 1 + .../test/specs/utils/felaRenderer-test.tsx | 28 ------------------- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/packages/react/test/specs/components/Animation/Animation-test.tsx b/packages/react/test/specs/components/Animation/Animation-test.tsx index 628a7b55bf..e95f8955f1 100644 --- a/packages/react/test/specs/components/Animation/Animation-test.tsx +++ b/packages/react/test/specs/components/Animation/Animation-test.tsx @@ -5,6 +5,7 @@ import Animation from 'src/components/Animation/Animation' describe('Animation', () => { isConformant(Animation, { + constructorName: 'Animation', hasAccessibilityProp: false, requiredProps: { children:
}, handlesAsProp: false, diff --git a/packages/react/test/specs/utils/felaRenderer-test.tsx b/packages/react/test/specs/utils/felaRenderer-test.tsx index dce6e30627..524ccafcc7 100644 --- a/packages/react/test/specs/utils/felaRenderer-test.tsx +++ b/packages/react/test/specs/utils/felaRenderer-test.tsx @@ -6,31 +6,6 @@ import Animation from 'src/components/Animation/Animation' import Provider from 'src/components/Provider/Provider' import Text from 'src/components/Text/Text' import { felaRenderer } from 'src/utils' -import { - ComponentAnimationProp, - unstable_createAnimationStyles as createAnimationStyles, -} from '@fluentui/react-bindings' - -// Animation component depends on theme styles 💣 -// Issue: https://github.com/microsoft/fluent-ui-react/issues/2247 -// This adds required styles when needed. -const AnimationComponentStyles = { - root: ({ props: p, theme }) => { - const animation: ComponentAnimationProp = { - name: p.name, - keyframeParams: p.keyframeParams, - duration: p.duration, - delay: p.delay, - iterationCount: p.iterationCount, - direction: p.direction, - fillMode: p.fillMode, - playState: p.playState, - timingFunction: p.timingFunction, - } - - return createAnimationStyles(animation, theme) - }, -} describe('felaRenderer', () => { test('basic styles are rendered', () => { @@ -75,7 +50,6 @@ describe('felaRenderer', () => { const snapshot = createSnapshot( @@ -106,7 +80,6 @@ describe('felaRenderer', () => { const snapshot = createSnapshot( @@ -138,7 +111,6 @@ describe('felaRenderer', () => { From 34b06859f4275b122f84b50b99ab327310eda354 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Mon, 10 Feb 2020 13:45:26 +0100 Subject: [PATCH 4/7] fix className issue --- packages/react/src/components/Animation/Animation.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/react/src/components/Animation/Animation.tsx b/packages/react/src/components/Animation/Animation.tsx index 57ea72b513..adf24fb578 100644 --- a/packages/react/src/components/Animation/Animation.tsx +++ b/packages/react/src/components/Animation/Animation.tsx @@ -153,6 +153,7 @@ const Animation: React.FC & { const { appear, children, + className, delay, direction, duration, @@ -191,6 +192,7 @@ const Animation: React.FC & { className: Animation.className, displayName: Animation.displayName, props: { + className, styles: createAnimationStyles(animation, context.theme), }, @@ -202,6 +204,7 @@ const Animation: React.FC & { _internal_resolvedComponentVariables: context._internal_resolvedComponentVariables, }) }, [ + className, context, name, delay, From 70675ac8e7b73517d7e89711c9f60011e70198d6 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 12 Feb 2020 16:28:24 +0100 Subject: [PATCH 5/7] fix review comments --- .../src/components/Animation/Animation.tsx | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/packages/react/src/components/Animation/Animation.tsx b/packages/react/src/components/Animation/Animation.tsx index adf24fb578..2824f311e4 100644 --- a/packages/react/src/components/Animation/Animation.tsx +++ b/packages/react/src/components/Animation/Animation.tsx @@ -201,7 +201,6 @@ const Animation: React.FC & { rtl: context.rtl, saveDebug: _.noop, theme: context.theme, - _internal_resolvedComponentVariables: context._internal_resolvedComponentVariables, }) }, [ className, @@ -225,27 +224,25 @@ const Animation: React.FC & { (React.Children.only(children) as React.ReactElement) const element = ( - <> - - {isChildrenFunction - ? () => (children as AnimationChildrenProp)({ classes: classes.root }) - : child} - - + + {isChildrenFunction + ? () => (children as AnimationChildrenProp)({ classes: classes.root }) + : child} + ) setEnd() From 05b95f3c7f9ef875a0fd35b05fa499ff7f9970a6 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 12 Feb 2020 16:32:50 +0100 Subject: [PATCH 6/7] fix merge issues --- packages/react/src/components/Animation/Animation.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react/src/components/Animation/Animation.tsx b/packages/react/src/components/Animation/Animation.tsx index 2824f311e4..7a9765be83 100644 --- a/packages/react/src/components/Animation/Animation.tsx +++ b/packages/react/src/components/Animation/Animation.tsx @@ -199,6 +199,8 @@ const Animation: React.FC & { disableAnimations: context.disableAnimations, renderer: context.renderer, rtl: context.rtl, + // Animation component doesn't have any styles, no perf optimizations can be applied + performance: {}, saveDebug: _.noop, theme: context.theme, }) From 383233284f5799976f1948ccd32e896437e35368 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 12 Feb 2020 16:54:39 +0100 Subject: [PATCH 7/7] remove comment --- packages/react/src/components/Animation/Animation.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react/src/components/Animation/Animation.tsx b/packages/react/src/components/Animation/Animation.tsx index 7a9765be83..b168959786 100644 --- a/packages/react/src/components/Animation/Animation.tsx +++ b/packages/react/src/components/Animation/Animation.tsx @@ -199,7 +199,6 @@ const Animation: React.FC & { disableAnimations: context.disableAnimations, renderer: context.renderer, rtl: context.rtl, - // Animation component doesn't have any styles, no perf optimizations can be applied performance: {}, saveDebug: _.noop, theme: context.theme,