From ec23ff8b9bffcb46e7874e3796ff9f0227bdb662 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Fri, 15 Feb 2019 15:57:38 +0100 Subject: [PATCH] [Transition] Fix hidden children appearing in a11y tree (#14465) * [Transition] Fix hidden children appearing in a11y tree Squashed commit of the following: commit 491239f60e7cf1769ef159ee447df9001fe63f46 Merge: bf011cecd aad72ed7a Author: Sebastian Silbermann Date: Wed Feb 13 10:29:31 2019 +0100 Merge branch 'next' into docs/a11y-code-variant-switch commit bf011cecdc4cb4a7e38f77f25bfe47e46f8c2e45 Author: Sebastian Silbermann Date: Tue Feb 12 21:13:26 2019 +0100 [Collapse] Don't consider hidden if collapsedHeight > 0 commit 59ee3e5b516acf9687e856d3c82fa1dcdf468b17 Author: Sebastian Silbermann Date: Tue Feb 12 21:07:36 2019 +0100 [Collapse] Fix css hidden property name commit 32dd6ffd6bfcf7a15db8b6d52874427bda819b04 Merge: a2d6e838c 2618e1e42 Author: Sebastian Silbermann Date: Tue Feb 12 21:03:44 2019 +0100 Merge branch 'docs/a11y-code-variant-switch' of github.com:eps1lon/material-ui into docs/a11y-code-variant-switch commit a2d6e838cedebbc9d1ef6c61f45aa915a513bf77 Author: Sebastian Silbermann Date: Sat Feb 9 11:03:56 2019 +0100 [core] Normalize transitions child style implementation commit 8d16305fa23db5e16ac7dd1f743607e04834ddf4 Author: Sebastian Silbermann Date: Sat Feb 9 09:05:49 2019 +0100 fixup commit ae08e115850765cd8cd5b748efc3df59afeb28f6 Author: Sebastian Silbermann Date: Sat Feb 9 09:05:34 2019 +0100 fixup commit 7b2cae2cbe1316ca76ba014baec9808a65628485 Author: Sebastian Silbermann Date: Fri Feb 8 20:27:43 2019 +0100 [Slide] Fix tests commit 9cdb582a32eb8d2c0ff3d5a3c48ce75ba67bac9e Author: Sebastian Silbermann Date: Fri Feb 8 20:19:01 2019 +0100 [core] Normalize transitions API commit 752cde534639332cf0d43ae725e5cb575d6fd5cb Author: Sebastian Silbermann Date: Fri Feb 8 16:31:28 2019 +0100 [Fade] Fix display none on IE 11 commit d10309b86c12f8ed9b91c603f99cf551ce996869 Author: Sebastian Silbermann Date: Fri Feb 8 14:22:31 2019 +0100 [Fade] Fix propTypes not warning on missing child element commit 1ed668206f82166fce2935df2db13259c3ce4d33 Author: Sebastian Silbermann Date: Fri Feb 8 14:21:44 2019 +0100 [Fade] Fix child element appearing in the a11y tree if hidden visually commit 6cbaabee7563e2da2ca9bbcbb596d8183f958686 Author: Sebastian Silbermann Date: Fri Feb 8 14:17:39 2019 +0100 [Fade] Test a11y for invisible content commit 2618e1e42d94d12146f4723b822f8ef68e611a39 Author: Olivier Tassinari Date: Sun Feb 10 10:49:22 2019 +0100 change class name commit ef1e87a64490f4432d28b232903f68d6350f8bef Author: Olivier Tassinari Date: Sun Feb 10 01:55:34 2019 +0100 look into the build fail commit af3d81228f4a46a6caaa4881b6fef7cd73a4c87c Author: Sebastian Silbermann Date: Sat Feb 9 11:03:56 2019 +0100 [core] Normalize transitions child style implementation commit 5082f2f42a6142279cbc0040c82852003ac5a2ce Author: Sebastian Silbermann Date: Sat Feb 9 09:05:49 2019 +0100 fixup commit 32699b19ed0d247edafc87081d4a47d66bef8526 Author: Sebastian Silbermann Date: Sat Feb 9 09:05:34 2019 +0100 fixup commit 459b8f856499edbc0ba6ad94b3c52116fe7aa3f2 Author: Sebastian Silbermann Date: Fri Feb 8 20:27:43 2019 +0100 [Slide] Fix tests commit 5dba229e3e861e76da7281d41530b95e2a8dbb92 Author: Sebastian Silbermann Date: Fri Feb 8 20:19:01 2019 +0100 [core] Normalize transitions API commit 63e91ac8530ed168ba5b6b3c9e7489e367e30ca0 Author: Sebastian Silbermann Date: Fri Feb 8 16:31:28 2019 +0100 [Fade] Fix display none on IE 11 commit fb70b731529a4ac83b1ee4c5b9c359800a13299f Author: Sebastian Silbermann Date: Fri Feb 8 14:22:31 2019 +0100 [Fade] Fix propTypes not warning on missing child element commit 85d75cee8d077a7486eb418df521c3e198feef51 Author: Sebastian Silbermann Date: Fri Feb 8 14:21:44 2019 +0100 [Fade] Fix child element appearing in the a11y tree if hidden visually commit 658b4d38961549afc7ea04f9aeba08b61677f55b Author: Sebastian Silbermann Date: Fri Feb 8 14:17:39 2019 +0100 [Fade] Test a11y for invisible content * [Grow] Fix whitespace assertion in test * [Menu] Fix menuitem not receiving focus in firefox Caused by focus() call race condition: dialog -> item in chrome but item -> dialog in firefox * [Transitions] Fix for wasted render cycle * Remove unnecessary test * [Zoom] Fix in being ignored * cleanup diff --- packages/material-ui/src/Collapse/Collapse.js | 10 +++++- packages/material-ui/src/Fade/Fade.js | 21 +++++------- packages/material-ui/src/Fade/Fade.test.js | 2 ++ packages/material-ui/src/Grow/Grow.js | 20 +++++------ packages/material-ui/src/Slide/Slide.js | 34 ++++++++----------- packages/material-ui/src/Slide/Slide.test.js | 28 +++++++-------- packages/material-ui/src/Zoom/Zoom.js | 21 +++++------- packages/material-ui/src/Zoom/Zoom.test.js | 2 ++ pages/api/collapse.md | 1 + pages/api/fade.md | 2 +- pages/api/grow.md | 2 +- pages/api/slide.md | 2 +- pages/api/zoom.md | 2 +- 13 files changed, 72 insertions(+), 75 deletions(-) diff --git a/packages/material-ui/src/Collapse/Collapse.js b/packages/material-ui/src/Collapse/Collapse.js index e116ce91f9ab3d..4fbea3f37d57bf 100644 --- a/packages/material-ui/src/Collapse/Collapse.js +++ b/packages/material-ui/src/Collapse/Collapse.js @@ -21,6 +21,11 @@ export const styles = theme => ({ height: 'auto', overflow: 'visible', }, + // eslint-disable-next-line max-len + /* Styles applied to the container element when the transition has exited and `collapsedHeight` != 0px. */ + hidden: { + vibility: 'hidden', + }, /* Styles applied to the outer wrapper element. */ wrapper: { // Hack to get children with a negative margin to not falsify the height computation. @@ -128,6 +133,7 @@ class Collapse extends React.Component { className, collapsedHeight, component: Component, + in: inProp, onEnter, onEntered, onEntering, @@ -141,6 +147,7 @@ class Collapse extends React.Component { return ( diff --git a/packages/material-ui/src/Fade/Fade.js b/packages/material-ui/src/Fade/Fade.js index bec8fd6c6769d3..a656b574d6d360 100644 --- a/packages/material-ui/src/Fade/Fade.js +++ b/packages/material-ui/src/Fade/Fade.js @@ -50,25 +50,22 @@ class Fade extends React.Component { }; render() { - const { children, onEnter, onExit, style: styleProp, theme, ...other } = this.props; - - const style = { - ...styleProp, - ...(React.isValidElement(children) ? children.props.style : {}), - }; + const { children, in: inProp, onEnter, onExit, style, theme, ...other } = this.props; return ( - - {(state, childProps) => - React.cloneElement(children, { + + {(state, childProps) => { + return React.cloneElement(children, { style: { opacity: 0, + visibility: state === 'exited' && !inProp ? 'hidden' : undefined, ...styles[state], ...style, + ...children.props.style, }, ...childProps, - }) - } + }); + }} ); } @@ -78,7 +75,7 @@ Fade.propTypes = { /** * A single child content element. */ - children: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), + children: PropTypes.element, /** * If `true`, the component will transition in. */ diff --git a/packages/material-ui/src/Fade/Fade.test.js b/packages/material-ui/src/Fade/Fade.test.js index ca616e394f3a60..eb9e7b228bd4db 100644 --- a/packages/material-ui/src/Fade/Fade.test.js +++ b/packages/material-ui/src/Fade/Fade.test.js @@ -87,6 +87,7 @@ describe('', () => { ); assert.deepEqual(wrapper.find('div').props().style, { opacity: 0, + visibility: 'hidden', }); }); @@ -98,6 +99,7 @@ describe('', () => { ); assert.deepEqual(wrapper.find('div').props().style, { opacity: 0, + visibility: 'hidden', }); }); }); diff --git a/packages/material-ui/src/Grow/Grow.js b/packages/material-ui/src/Grow/Grow.js index 1a83e254d22007..2e0d56d3611b13 100644 --- a/packages/material-ui/src/Grow/Grow.js +++ b/packages/material-ui/src/Grow/Grow.js @@ -103,33 +103,31 @@ class Grow extends React.Component { }; render() { - const { children, onEnter, onExit, style: styleProp, theme, timeout, ...other } = this.props; - - const style = { - ...styleProp, - ...(React.isValidElement(children) ? children.props.style : {}), - }; + const { children, in: inProp, onEnter, onExit, style, theme, timeout, ...other } = this.props; return ( - {(state, childProps) => - React.cloneElement(children, { + {(state, childProps) => { + return React.cloneElement(children, { style: { opacity: 0, transform: getScale(0.75), + visiblity: state === 'exited' && !inProp ? 'hidden' : undefined, ...styles[state], ...style, + ...children.props.style, }, ...childProps, - }) - } + }); + }} ); } @@ -139,7 +137,7 @@ Grow.propTypes = { /** * A single child content element. */ - children: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), + children: PropTypes.element, /** * If `true`, show the component; triggers the enter or exit animation. */ diff --git a/packages/material-ui/src/Slide/Slide.js b/packages/material-ui/src/Slide/Slide.js index e0b807d3c224db..a0c068a5a1fd60 100644 --- a/packages/material-ui/src/Slide/Slide.js +++ b/packages/material-ui/src/Slide/Slide.js @@ -179,7 +179,6 @@ class Slide extends React.Component { updatePosition() { if (this.transitionRef) { - this.transitionRef.style.visibility = 'inherit'; setTranslateValue(this.props, this.transitionRef); } } @@ -188,30 +187,16 @@ class Slide extends React.Component { const { children, direction, + in: inProp, onEnter, onEntering, onExit, onExited, - style: styleProp, + style, theme, ...other } = this.props; - let style = {}; - - // We use this state to handle the server-side rendering. - // We don't know the width of the children ahead of time. - // We need to render it. - if (!this.props.in && !this.mounted) { - style.visibility = 'hidden'; - } - - style = { - ...style, - ...styleProp, - ...(React.isValidElement(children) ? children.props.style : {}), - }; - return ( { this.transitionRef = ReactDOM.findDOMNode(ref); }} {...other} > - {children} + {(state, childProps) => { + return React.cloneElement(children, { + style: { + visibility: state === 'exited' && !inProp ? 'hidden' : undefined, + ...style, + ...children.props.style, + }, + ...childProps, + }); + }} ); @@ -237,7 +231,7 @@ Slide.propTypes = { /** * A single child content element. */ - children: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), + children: PropTypes.element, /** * Direction the child node will enter from. */ diff --git a/packages/material-ui/src/Slide/Slide.test.js b/packages/material-ui/src/Slide/Slide.test.js index 12993b0e749a02..c86fb6217f1ed1 100644 --- a/packages/material-ui/src/Slide/Slide.test.js +++ b/packages/material-ui/src/Slide/Slide.test.js @@ -1,7 +1,6 @@ import React from 'react'; import { assert } from 'chai'; import { spy, useFakeTimers } from 'sinon'; -import Transition from 'react-transition-group/Transition'; import { createShallow, createMount, unwrap } from '@material-ui/core/test-utils'; import Slide, { setTranslateValue } from './Slide'; import transitions, { easing } from '../styles/transitions'; @@ -39,19 +38,14 @@ describe('', () => { style={{ color: 'red', backgroundColor: 'yellow' }} theme={createMuiTheme()} > -
+
, ); - assert.deepEqual( - wrapper - .childAt(0) - .childAt(0) - .props().style, - { - backgroundColor: 'yellow', - color: 'blue', - }, - ); + assert.deepEqual(wrapper.find('#with-slide').props().style, { + backgroundColor: 'yellow', + color: 'blue', + visibility: undefined, + }); }); describe('event callbacks', () => { @@ -241,7 +235,7 @@ describe('', () => { ); const transition = wrapper.instance().transitionRef; - assert.strictEqual(transition.style.visibility, 'inherit'); + assert.strictEqual(transition.style.visibility, 'hidden'); assert.notStrictEqual(transition.style.transform, undefined); }); }); @@ -303,8 +297,12 @@ describe('', () => { describe('server-side', () => { it('should be initially hidden', () => { - const wrapper = shallow(); - assert.strictEqual(wrapper.find(Transition).props().style.visibility, 'hidden'); + const wrapper = mount( + +
+ , + ); + assert.strictEqual(wrapper.find('#with-slide').props().style.visibility, 'hidden'); }); }); }); diff --git a/packages/material-ui/src/Zoom/Zoom.js b/packages/material-ui/src/Zoom/Zoom.js index 56fbeb9803f143..699e77d2a59ec8 100644 --- a/packages/material-ui/src/Zoom/Zoom.js +++ b/packages/material-ui/src/Zoom/Zoom.js @@ -51,25 +51,22 @@ class Zoom extends React.Component { }; render() { - const { children, onEnter, onExit, style: styleProp, theme, ...other } = this.props; - - const style = { - ...styleProp, - ...(React.isValidElement(children) ? children.props.style : {}), - }; + const { children, in: inProp, onEnter, onExit, style, theme, ...other } = this.props; return ( - - {(state, childProps) => - React.cloneElement(children, { + + {(state, childProps) => { + return React.cloneElement(children, { style: { transform: 'scale(0)', + visibility: state === 'exited' && !inProp ? 'hidden' : undefined, ...styles[state], ...style, + ...children.props.style, }, ...childProps, - }) - } + }); + }} ); } @@ -79,7 +76,7 @@ Zoom.propTypes = { /** * A single child content element. */ - children: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), + children: PropTypes.element, /** * If `true`, the component will transition in. */ diff --git a/packages/material-ui/src/Zoom/Zoom.test.js b/packages/material-ui/src/Zoom/Zoom.test.js index 592a9f0f47f381..756caa35c1d51f 100644 --- a/packages/material-ui/src/Zoom/Zoom.test.js +++ b/packages/material-ui/src/Zoom/Zoom.test.js @@ -87,6 +87,7 @@ describe('', () => { ); assert.deepEqual(wrapper.find('div').props().style, { transform: 'scale(0)', + visibility: 'hidden', }); }); @@ -98,6 +99,7 @@ describe('', () => { ); assert.deepEqual(wrapper.find('div').props().style, { transform: 'scale(0)', + visibility: 'hidden', }); }); }); diff --git a/pages/api/collapse.md b/pages/api/collapse.md index cedcb33750cf97..c90071e90bd5b5 100644 --- a/pages/api/collapse.md +++ b/pages/api/collapse.md @@ -39,6 +39,7 @@ This property accepts the following keys: |:-----|:------------| | container | Styles applied to the container element. | entered | Styles applied to the container element when the transition has entered. +| hidden | Styles applied to the container element when the transition has exited and `collapsedHeight` != 0px. | wrapper | Styles applied to the outer wrapper element. | wrapperInner | Styles applied to the inner wrapper element. diff --git a/pages/api/fade.md b/pages/api/fade.md index ce72dc60401eab..97d39f499ef1e3 100644 --- a/pages/api/fade.md +++ b/pages/api/fade.md @@ -19,7 +19,7 @@ It uses [react-transition-group](https://github.com/reactjs/react-transition-gro | Name | Type | Default | Description | |:-----|:-----|:--------|:------------| -| children | union: element |
 func
 |   | A single child content element. | +| children | element |   | A single child content element. | | in | bool |   | If `true`, the component will transition in. | | timeout | union: number |
 { enter?: number, exit?: number }
 | { enter: duration.enteringScreen, exit: duration.leavingScreen,} | The duration for the transition, in milliseconds. You may specify a single timeout for all transitions, or individually with an object. | diff --git a/pages/api/grow.md b/pages/api/grow.md index 55c5de5ac64444..dc7769c3a1b1a4 100644 --- a/pages/api/grow.md +++ b/pages/api/grow.md @@ -20,7 +20,7 @@ It uses [react-transition-group](https://github.com/reactjs/react-transition-gro | Name | Type | Default | Description | |:-----|:-----|:--------|:------------| -| children | union: element |
 func
 |   | A single child content element. | +| children | element |   | A single child content element. | | in | bool |   | If `true`, show the component; triggers the enter or exit animation. | | timeout | union: number |
 { enter?: number, exit?: number } |
 enum: 'auto'

 | 'auto' | The duration for the transition, in milliseconds. You may specify a single timeout for all transitions, or individually with an object.
Set to 'auto' to automatically calculate transition time based on height. | diff --git a/pages/api/slide.md b/pages/api/slide.md index 661745ca966cab..b4e670f67b43a0 100644 --- a/pages/api/slide.md +++ b/pages/api/slide.md @@ -19,7 +19,7 @@ It uses [react-transition-group](https://github.com/reactjs/react-transition-gro | Name | Type | Default | Description | |:-----|:-----|:--------|:------------| -| children | union: element |
 func
 |   | A single child content element. | +| children | element |   | A single child content element. | | direction | enum: 'left' |
 'right' |
 'up' |
 'down'
 | 'down' | Direction the child node will enter from. | | in | bool |   | If `true`, show the component; triggers the enter or exit animation. | | timeout | union: number |
 { enter?: number, exit?: number }
 | { enter: duration.enteringScreen, exit: duration.leavingScreen,} | The duration for the transition, in milliseconds. You may specify a single timeout for all transitions, or individually with an object. | diff --git a/pages/api/zoom.md b/pages/api/zoom.md index 873feef2215919..24ed339bad16ce 100644 --- a/pages/api/zoom.md +++ b/pages/api/zoom.md @@ -20,7 +20,7 @@ It uses [react-transition-group](https://github.com/reactjs/react-transition-gro | Name | Type | Default | Description | |:-----|:-----|:--------|:------------| -| children | union: element |
 func
 |   | A single child content element. | +| children | element |   | A single child content element. | | in | bool |   | If `true`, the component will transition in. | | timeout | union: number |
 { enter?: number, exit?: number }
 | { enter: duration.enteringScreen, exit: duration.leavingScreen,} | The duration for the transition, in milliseconds. You may specify a single timeout for all transitions, or individually with an object. |