[Fade] Fix child element appearing in the a11y tree if hidden visually

eps1lon · Feb 12, 2019 · 1ed6682 · 1ed6682
1 parent 6cbaabe
commit 1ed6682
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 5 deletions.
diff --git a/packages/material-ui/src/Fade/Fade.js b/packages/material-ui/src/Fade/Fade.js
@@ -10,15 +10,22 @@ import { reflow, getTransitionProps } from '../transitions/utils';
 const styles = {
   entering: {
     opacity: 1,
+    visibility: 'visible',
   },
   entered: {
     opacity: 1,
+    visibility: 'visible',
+  },
+  exited: {
+    visibility: 'hidden',
   },
 };
 
 /**
  * The Fade transition is used by the [Modal](/utils/modal/) component.
  * It uses [react-transition-group](https://github.com/reactjs/react-transition-group) internally.
+ *
+ * The `hidden` prop will be set on the child according to the `in` prop.
  */
 class Fade extends React.Component {
   handleEnter = node => {
@@ -50,22 +57,24 @@ class Fade extends React.Component {
   };
 
   render() {
-    const { children, onEnter, onExit, style: styleProp, theme, ...other } = this.props;
+    const { children, in: inProp, onEnter, onExit, style: styleProp, theme, ...other } = this.props;
 
     const style = {
       ...styleProp,
       ...(React.isValidElement(children) ? children.props.style : {}),
     };
 
     return (
-      <Transition appear onEnter={this.handleEnter} onExit={this.handleExit} {...other}>
+      <Transition appear in={inProp} onEnter={this.handleEnter} onExit={this.handleExit} {...other}>
         {(state, childProps) =>
           React.cloneElement(children, {
             style: {
+              display: 'initial',
               opacity: 0,
               ...styles[state],
               ...style,
             },
+            hidden: !inProp,
             ...childProps,
           })
         }

diff --git a/packages/material-ui/src/Fade/Fade.test.js b/packages/material-ui/src/Fade/Fade.test.js
@@ -86,7 +86,9 @@ describe('<Fade />', () => {
         </Fade>,
       );
       assert.deepEqual(wrapper.find('div').props().style, {
+        display: 'initial',
         opacity: 0,
+        visibility: 'hidden',
       });
     });
 
@@ -97,18 +99,20 @@ describe('<Fade />', () => {
         </Fade>,
       );
       assert.deepEqual(wrapper.find('div').props().style, {
+        display: 'initial',
         opacity: 0,
+        visibility: 'hidden',
       });
     });
   });
 
   describe('accessiblity', () => {
-    it('will set `aria-hidden` acording to the `in` prop', () => {
+    it('will set `hidden` acording to the `in` prop', () => {
       const wrapper = mount(<Fade {...defaultProps} />);
-      assert.strictEqual(wrapper.find('div').props()['aria-hidden'], false);
+      assert.strictEqual(wrapper.find('div').props().hidden, false);
 
       wrapper.setProps({ in: false });
-      assert.strictEqual(wrapper.find('div').props()['aria-hidden'], true);
+      assert.strictEqual(wrapper.find('div').props().hidden, true);
     });
   });
 });
diff --git a/pages/api/fade.md b/pages/api/fade.md
@@ -15,6 +15,8 @@ import Fade from '@material-ui/core/Fade';
 The Fade transition is used by the [Modal](/utils/modal/) component.
 It uses [react-transition-group](https://github.com/reactjs/react-transition-group) internally.
 
+The `hidden` prop will be set on the child according to the `in` prop.
+
 ## Props
 
 | Name | Type | Default | Description |