Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(components): remove animation prop from all components #2239

Merged
merged 6 commits into from
Jan 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### BREAKING CHANGES
- Add `@fluentui/styles` package for all styles' related utilities and TS types @layershifter, @mnajdova ([#2222](https://github.com/microsoft/fluent-ui-react/pull/2222))
- Remove `animation` prop from all components @joschect ([#2239](https://github.com/microsoft/fluent-ui-react/pull/2239))

### Fixes
- Fix event lister leak in `FocusZone` @miroslavstastny ([#2227](https://github.com/microsoft/fluent-ui-react/pull/2227))
Expand Down
26 changes: 5 additions & 21 deletions docs/src/views/Theming.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,7 @@ export default () => (
<p>
This is done with the Provider's <code>theme</code> prop. The animations are then applied
based on their name by using the <NavLink to="components/Animation">Animation</NavLink>{' '}
component, or the <code>animation</code> property available on all Fluent UI component. Here's
how we can use them in our components.
component. Here's how we can use them in our components.
</p>
<ExampleSnippet
render={() => (
Expand All @@ -244,16 +243,14 @@ export default () => (
<Animation name="spinner">
<Icon name="calendar" circular />
</Animation>
<Icon name="emoji" animation="spinner" circular />
</div>
</Provider>
)}
/>

<p>
You can also override some of the defined <code>animation</code> properties, by providing
additional properties to the <code>Animation</code> component, or the <code>animation</code>{' '}
prop.
additional properties to the <code>Animation</code> component.
</p>

<blockquote>
Expand Down Expand Up @@ -283,26 +280,13 @@ export default () => (
<Animation name="spinner" delay="2s" duration="1s">
<Icon name="calendar" circular />
</Animation>
<Icon
name="emoji"
animation={{ name: 'spinner', delay: '5s', duration: '2s' }}
circular
/>
<Animation name="spinner" delay="5s" duration="2s">
<Icon name="emoji" circular />
</Animation>
</div>
</Provider>
)}
/>

<p>
The difference between using the Animation component versus the animation property is that,
the Animation component can be safely used for applying animations on{' '}
<i>all components (Fluent UI, custom and third party components)</i>. For the Fluent UI
components, we recommend using the animation property as there will be no wrapper element
added just for the purpose of defining the animation. For more details, please see the
examples in the <NavLink to="components/Animation">Animation</NavLink> component, or the
structure of the <code>animation</code> property in any of the Fluent UI components.
</p>

<GuidesNavigationFooter
previous={{ name: 'Accessibility', url: 'accessibility' }}
next={{ name: 'Theming Examples', url: 'theming-examples' }}
Expand Down
6 changes: 0 additions & 6 deletions packages/react-bindings/src/styles/getStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
import cx from 'classnames'
import * as _ from 'lodash'

import createAnimationStyles from './createAnimationStyles'
import resolveStylesAndClasses from './resolveStylesAndClasses'
import {
ComponentDesignProp,
Expand Down Expand Up @@ -69,16 +68,11 @@ const getStyles = (options: GetStylesOptions): GetStylesResult => {
)(theme.siteVariables)
: resolvedComponentVariables[displayName]

const animationStyles = props.animation
? createAnimationStyles(props.animation, theme)
: undefined

// Resolve styles using resolved variables, merge results, allow props.styles to override
const mergedStyles: ComponentSlotStylesPrepared = mergeComponentStyles(
theme.componentStyles[displayName],
props.design && withDebugId({ root: props.design }, 'props.design'),
props.styles && withDebugId({ root: props.styles } as ComponentSlotStylesInput, 'props.styles'),
animationStyles && withDebugId({ root: animationStyles }, 'props.animation'),
)

const styleParam: ComponentStyleFunctionParam = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export default {
color,
important,
timestamp,
animation,
truncated,
disabled,
error,
Expand All @@ -28,8 +27,6 @@ export default {
const colors = v.colorScheme[getColorSchemeKey(color)]
return {
...(color && { color: colors.foreground }),
// animations are not working with span, unless display is set to 'inline-block'
...(animation && as === 'span' && { display: 'inline-block' }),
...(atMention === true && { color: v.atMentionOtherColor }),
...(truncated && { overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }),
...(disabled && { color: v.disabledColor }),
Expand Down
12 changes: 2 additions & 10 deletions packages/react/src/utils/commonPropInterfaces.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { ComponentAnimationProp, ComponentDesignProp } from '@fluentui/react-bindings'
import { ComponentDesignProp } from '@fluentui/react-bindings'
import { ComponentSlotStyle, ComponentVariablesInput } from '@fluentui/styles'
import * as React from 'react'

import { ReactChildren } from '../types'

export interface StyledComponentProps<P = any, V = any> {
Expand All @@ -12,14 +11,7 @@ export interface StyledComponentProps<P = any, V = any> {
variables?: ComponentVariablesInput
}

export interface AnimatedComponentProps {
/** Generic animation property that can be used for applying different theme animations. */
animation?: ComponentAnimationProp
}

export interface UIComponentProps<P = any, V = any>
extends StyledComponentProps<P, V>,
AnimatedComponentProps {
export interface UIComponentProps<P = any, V = any> extends StyledComponentProps<P, V> {
/** Additional CSS class name(s) to apply. */
className?: string
design?: ComponentDesignProp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,15 @@ exports[`felaRenderer CSS fallback values are rendered 1`] = `
`;

exports[`felaRenderer animations are not applied if animations are disabled 1`] = `
"
".a {
display: inline-block;
}


<div className=ui-provider__box dir=ltr>
<div className=ui-box />
<div className=ui-animation a>
<div className=ui-box />
</div>
</div>;
"
`;
Expand Down Expand Up @@ -58,12 +63,17 @@ exports[`felaRenderer array returned by keyframe results in CSS fallback values
}
}
.a {
display: inline-block;
}
.b {
animation-name: k1;
}


<div className=ui-provider__box dir=ltr>
<div className=ui-box a />
<div className=ui-animation a>
<div className=ui-box b />
</div>
</div>;
"
`;
Expand Down Expand Up @@ -104,15 +114,20 @@ exports[`felaRenderer keyframe colors are rendered 1`] = `
}
}
.a {
animation-name: k1;
display: inline-block;
}
.b {
animation-name: k1;
}
.c {
animation-duration: 5s;
}


<div className=ui-provider__box dir=ltr>
<div className=ui-box a b />
<div className=ui-animation a>
<div className=ui-box b c />
</div>
</div>;
"
`;
Expand Down
63 changes: 57 additions & 6 deletions packages/react/test/specs/utils/felaRenderer-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,38 @@ import * as React from 'react'
import { createSnapshot } from 'jest-react-fela'
import { EmptyThemeProvider } from 'test/utils'
import Box from 'src/components/Box/Box'
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: () => ({
display: 'inline-block',
}),
children: ({ 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', () => {
Expand Down Expand Up @@ -47,8 +76,15 @@ describe('felaRenderer', () => {
}

const snapshot = createSnapshot(
<Provider theme={{ animations: { spinner } }}>
<Box animation="spinner" />
<Provider
theme={{
componentStyles: { Animation: AnimationComponentStyles },
animations: { spinner },
}}
>
<Animation name="spinner">
<Box />
</Animation>
</Provider>,
{},
felaRenderer,
Expand All @@ -71,8 +107,15 @@ describe('felaRenderer', () => {
}

const snapshot = createSnapshot(
<Provider theme={{ animations: { spinner } }}>
<Box animation="spinner" />
<Provider
theme={{
componentStyles: { Animation: AnimationComponentStyles },
animations: { spinner },
}}
>
<Animation name="spinner">
<Box />
</Animation>
</Provider>,
{},
felaRenderer,
Expand All @@ -95,8 +138,16 @@ describe('felaRenderer', () => {
}

const snapshot = createSnapshot(
<Provider disableAnimations theme={{ animations: { spinner } }}>
<Box animation="spinner" />
<Provider
disableAnimations
theme={{
componentStyles: { Animation: AnimationComponentStyles },
animations: { spinner },
}}
>
<Animation name="spinner">
<Box />
</Animation>
</Provider>,
{},
felaRenderer,
Expand Down