[Modal] Fix aria-hidden restore logic (#15839)

* [ModalManager] exclude aria-hidden nodes from node-hiding logic

* try to simplify the logic

packages/material-ui/src/Modal/Modal.js

@@ -8,10 +8,9 @@ import Portal from '../Portal';
 import { createChainedFunction } from '../utils/helpers';
 import { useForkRef } from '../utils/reactHelpers';
 import zIndex from '../styles/zIndex';
-import ModalManager from './ModalManager';
+import ModalManager, { ariaHidden } from './ModalManager';
 import TrapFocus from './TrapFocus';
 import SimpleBackdrop from './SimpleBackdrop';
-import { ariaHidden } from './manageAriaHidden';
 
 function getContainer(container) {
   container = typeof container === 'function' ? container() : container;

packages/material-ui/src/Modal/ModalManager.js

@@ -1,11 +1,54 @@
 import getScrollbarSize from '../utils/getScrollbarSize';
 import ownerDocument from '../utils/ownerDocument';
-import isOverflowing from './isOverflowing';
-import { ariaHidden, ariaHiddenSiblings } from './manageAriaHidden';
+import ownerWindow from '../utils/ownerWindow';
 
-function findIndexOf(data, callback) {
+// Do we have a vertical scrollbar?
+function isOverflowing(container) {
+  const doc = ownerDocument(container);
+
+  if (doc.body === container) {
+    const win = ownerWindow(doc);
+    return win.innerWidth > doc.documentElement.clientWidth;
+  }
+
+  return container.scrollHeight > container.clientHeight;
+}
+
+export function ariaHidden(node, show) {
+  if (show) {
+    node.setAttribute('aria-hidden', 'true');
+  } else {
+    node.removeAttribute('aria-hidden');
+  }
+}
+
+function getPaddingRight(node) {
+  return parseInt(window.getComputedStyle(node)['padding-right'], 10) || 0;
+}
+
+const BLACKLIST = ['template', 'script', 'style'];
+
+function isHideable(node) {
+  return node.nodeType === 1 && BLACKLIST.indexOf(node.tagName.toLowerCase()) === -1;
+}
+
+function siblings(container, mount, currentNode, nodesToExclude, callback) {
+  const blacklist = [mount, currentNode, ...nodesToExclude];
+
+  [].forEach.call(container.children, node => {
+    if (blacklist.indexOf(node) === -1 && isHideable(node)) {
+      callback(node);
+    }
+  });
+}
+
+function ariaHiddenSiblings(container, mountNode, currentNode, nodesToExclude = [], show) {
+  siblings(container, mountNode, currentNode, nodesToExclude, node => ariaHidden(node, show));
+}
+
+function findIndexOf(containerInfo, callback) {
   let idx = -1;
-  data.some((item, index) => {
+  containerInfo.some((item, index) => {
     if (callback(item)) {
       idx = index;
       return true;
@@ -15,53 +58,63 @@ function findIndexOf(data, callback) {
   return idx;
 }
 
-function getPaddingRight(node) {
-  return parseInt(window.getComputedStyle(node)['padding-right'], 10) || 0;
-}
-
-function setContainerStyle(data) {
+function handleNewContainer(containerInfo) {
   // We are only interested in the actual `style` here because we will override it.
-  data.style = {
-    overflow: data.container.style.overflow,
-    paddingRight: data.container.style.paddingRight,
+  const restoreStyle = {
+    overflow: containerInfo.container.style.overflow,
+    paddingRight: containerInfo.container.style.paddingRight,
   };
 
   const style = {
     overflow: 'hidden',
   };
 
-  if (data.overflowing) {
+  const restorePaddings = [];
+  let fixedNodes;
+
+  if (containerInfo.overflowing) {
     const scrollbarSize = getScrollbarSize();
 
     // Use computed style, here to get the real padding to add our scrollbar width.
-    style.paddingRight = `${getPaddingRight(data.container) + scrollbarSize}px`;
+    style.paddingRight = `${getPaddingRight(containerInfo.container) + scrollbarSize}px`;
 
     // .mui-fixed is a global helper.
-    const fixedNodes = ownerDocument(data.container).querySelectorAll('.mui-fixed');
-    for (let i = 0; i < fixedNodes.length; i += 1) {
-      const paddingRight = getPaddingRight(fixedNodes[i]);
-      data.prevPaddings.push(paddingRight);
-      fixedNodes[i].style.paddingRight = `${paddingRight + scrollbarSize}px`;
-    }
+    fixedNodes = ownerDocument(containerInfo.container).querySelectorAll('.mui-fixed');
+
+    [].forEach.call(fixedNodes, node => {
+      const paddingRight = getPaddingRight(node);
+      restorePaddings.push(paddingRight);
+      node.style.paddingRight = `${paddingRight + scrollbarSize}px`;
+    });
   }
 
   Object.keys(style).forEach(key => {
-    data.container.style[key] = style[key];
+    containerInfo.container.style[key] = style[key];
   });
-}
 
-function removeContainerStyle(data) {
-  // The modal might be closed before it had the chance to be mounted in the DOM.
-  if (data.style) {
-    Object.keys(data.style).forEach(key => {
-      data.container.style[key] = data.style[key];
+  const restore = () => {
+    if (fixedNodes) {
+      [].forEach.call(fixedNodes, (node, i) => {
+        node.style.paddingRight = `${restorePaddings[i]}px`;
+      });
+    }
+
+    Object.keys(restoreStyle).forEach(key => {
+      containerInfo.container.style[key] = restoreStyle[key];
     });
-  }
+  };
 
-  const fixedNodes = ownerDocument(data.container).querySelectorAll('.mui-fixed');
-  for (let i = 0; i < fixedNodes.length; i += 1) {
-    fixedNodes[i].style.paddingRight = `${data.prevPaddings[i]}px`;
-  }
+  return restore;
+}
+
+function getHiddenSiblings(container) {
+  const hiddenSiblings = [];
+  [].forEach.call(container.children, node => {
+    if (node.getAttribute && node.getAttribute('aria-hidden') === 'true') {
+      hiddenSiblings.push(node);
+    }
+  });
+  return hiddenSiblings;
 }
 
 /**
@@ -72,97 +125,97 @@ function removeContainerStyle(data) {
  * Used by the Modal to ensure proper styling of containers.
  */
 export default class ModalManager {
-  constructor(options = {}) {
-    const { hideSiblingNodes = true, handleContainerOverflow = true } = options;
-
-    this.hideSiblingNodes = hideSiblingNodes;
-    this.handleContainerOverflow = handleContainerOverflow;
-
-    // this.modals[modalIdx] = modal
+  constructor() {
+    // this.modals[modalIndex] = modal
     this.modals = [];
-    // this.data[containerIdx] = {
+    // this.contaniners[containerIndex] = {
     //   modals: [],
     //   container,
     //   overflowing,
-    //   prevPaddings,
+    //   restore: null,
     // }
-    this.data = [];
+    this.contaniners = [];
   }
 
   add(modal, container) {
-    let modalIdx = this.modals.indexOf(modal);
-    if (modalIdx !== -1) {
-      return modalIdx;
+    let modalIndex = this.modals.indexOf(modal);
+    if (modalIndex !== -1) {
+      return modalIndex;
     }
 
-    modalIdx = this.modals.length;
+    modalIndex = this.modals.length;
     this.modals.push(modal);
 
     // If the modal we are adding is already in the DOM.
     if (modal.modalRef) {
       ariaHidden(modal.modalRef, false);
     }
-    if (this.hideSiblingNodes) {
-      ariaHiddenSiblings(container, modal.mountNode, modal.modalRef, true);
-    }
 
-    const containerIdx = findIndexOf(this.data, item => item.container === container);
-    if (containerIdx !== -1) {
-      this.data[containerIdx].modals.push(modal);
-      return modalIdx;
+    const hiddenSiblingNodes = getHiddenSiblings(container);
+    ariaHiddenSiblings(container, modal.mountNode, modal.modalRef, hiddenSiblingNodes, true);
+
+    const containerIndex = findIndexOf(this.contaniners, item => item.container === container);
+    if (containerIndex !== -1) {
+      this.contaniners[containerIndex].modals.push(modal);
+      return modalIndex;
     }
 
-    const data = {
+    this.contaniners.push({
       modals: [modal],
       container,
       overflowing: isOverflowing(container),
-      prevPaddings: [],
-    };
-
-    this.data.push(data);
+      restore: null,
+      hiddenSiblingNodes,
+    });
 
-    return modalIdx;
+    return modalIndex;
   }
 
   mount(modal) {
-    const containerIdx = findIndexOf(this.data, item => item.modals.indexOf(modal) !== -1);
-    const data = this.data[containerIdx];
+    const containerIndex = findIndexOf(this.contaniners, item => item.modals.indexOf(modal) !== -1);
+    const containerInfo = this.contaniners[containerIndex];
 
-    if (!data.style && this.handleContainerOverflow) {
-      setContainerStyle(data);
+    if (!containerInfo.restore) {
+      containerInfo.restore = handleNewContainer(containerInfo);
     }
   }
 
   remove(modal) {
-    const modalIdx = this.modals.indexOf(modal);
+    const modalIndex = this.modals.indexOf(modal);
 
-    if (modalIdx === -1) {
-      return modalIdx;
+    if (modalIndex === -1) {
+      return modalIndex;
     }
 
-    const containerIdx = findIndexOf(this.data, item => item.modals.indexOf(modal) !== -1);
-    const data = this.data[containerIdx];
+    const containerIndex = findIndexOf(this.contaniners, item => item.modals.indexOf(modal) !== -1);
+    const containerInfo = this.contaniners[containerIndex];
 
-    data.modals.splice(data.modals.indexOf(modal), 1);
-    this.modals.splice(modalIdx, 1);
+    containerInfo.modals.splice(containerInfo.modals.indexOf(modal), 1);
+    this.modals.splice(modalIndex, 1);
 
     // If that was the last modal in a container, clean up the container.
-    if (data.modals.length === 0) {
-      if (this.handleContainerOverflow) {
-        removeContainerStyle(data);
+    if (containerInfo.modals.length === 0) {
+      // The modal might be closed before it had the chance to be mounted in the DOM.
+      if (containerInfo.restore) {
+        containerInfo.restore();
       }
 
-      // In case the modal wasn't in the DOM yet.
       if (modal.modalRef) {
+        // In case the modal wasn't in the DOM yet.
         ariaHidden(modal.modalRef, true);
       }
-      if (this.hideSiblingNodes) {
-        ariaHiddenSiblings(data.container, modal.mountNode, modal.modalRef, false);
-      }
-      this.data.splice(containerIdx, 1);
-    } else if (this.hideSiblingNodes) {
+
+      ariaHiddenSiblings(
+        containerInfo.container,
+        modal.mountNode,
+        modal.modalRef,
+        containerInfo.hiddenSiblingNodes,
+        false,
+      );
+      this.contaniners.splice(containerIndex, 1);
+    } else {
       // Otherwise make sure the next top modal is visible to a screen reader.
-      const nextTop = data.modals[data.modals.length - 1];
+      const nextTop = containerInfo.modals[containerInfo.modals.length - 1];
       // as soon as a modal is adding its modalRef is undefined. it can't set
       // aria-hidden because the dom element doesn't exist either
       // when modal was unmounted before modalRef gets null
@@ -171,7 +224,7 @@ export default class ModalManager {
       }
     }
 
-    return modalIdx;
+    return modalIndex;
   }
 
   isTopModal(modal) {

packages/material-ui/src/Modal/ModalManager.test.js

@@ -239,5 +239,24 @@ describe('ModalManager', () => {
       modalManager.remove(modal, container2);
       assert.strictEqual(container2.children[0].getAttribute('aria-hidden'), 'true');
     });
+
+    it('should keep previous aria-hidden siblings hidden', () => {
+      const modal = { modalRef: container2.children[0] };
+      const sibling1 = document.createElement('div');
+      const sibling2 = document.createElement('div');
+
+      sibling1.setAttribute('aria-hidden', 'true');
+
+      container2.appendChild(sibling1);
+      container2.appendChild(sibling2);
+
+      modalManager.add(modal, container2);
+      modalManager.mount(modal);
+      assert.strictEqual(container2.children[0].getAttribute('aria-hidden'), null);
+      modalManager.remove(modal, container2);
+      assert.strictEqual(container2.children[0].getAttribute('aria-hidden'), 'true');
+      assert.strictEqual(container2.children[1].getAttribute('aria-hidden'), 'true');
+      assert.strictEqual(container2.children[2].getAttribute('aria-hidden'), null);
+    });
   });
 });