[fixed] improvements on setAppElement...

diasbruno · diasbruno · commit f8edc2bfc3c6 · 2017-06-25T17:27:26.000-03:00
There was some pitfalls on how `setAppElement` work.

If your &lt;script /&gt; was in &lt;head /&gt;, there was a change that it tries
to use `document.body` that is not yet ready.

Another one was using an selector string that does not find any
elements, causing it to try to perform all call on `null`.

This patch can also help if you want to do server-side rendering,
but this was not tested and, perhaps, it's better to use this function
correctly.
diff --git a/README.md b/README.md
@@ -48,6 +48,34 @@ Example:
 </Modal>
 ```
 
+### App Element
+
+The app element allows you to specify the portion
+of your app that should be hidden (via aria-hidden)
+to prevent assistive technologies such as screenreaders
+from reading content outside of the content of
+your modal.
+
+It's optional and if not specified it will try to use
+`document.body` as your app element.
+
+If your are doing server-side rendering, you should use
+this property.
+
+It can be specified in the following ways:
+
+- DOMElement
+
+```js
+Modal.setAppElement(appElement);
+```
+
+- query selector - uses the first element found if you pass in a class.
+
+```js
+Modal.setAppElement('#your-app-element');
+```
+
 ## Styles
 
 Styles are passed as an object with 2 keys, 'overlay' and 'content' like so
@@ -160,21 +188,6 @@ import React from 'react';
 import ReactDOM from 'react-dom';
 import Modal from 'react-modal';
 
-
-/*
-The app element allows you to specify the portion of your app that should be hidden (via aria-hidden)
-to prevent assistive technologies such as screenreaders from reading content outside of the content of
-your modal.  It can be specified in the following ways:
-
-* element
-Modal.setAppElement(appElement);
-
-* query selector - uses the first element found if you pass in a class.
-Modal.setAppElement('#your-app-element');
-
-*/
-const appElement = document.getElementById('your-app-element');
-
 const customStyles = {
   content : {
     top                   : '50%',
diff --git a/specs/Modal.spec.js b/specs/Modal.spec.js
@@ -240,6 +240,21 @@ describe('State', () => {
     expect(!isBodyWithReactModalOpenClass()).toBeTruthy();
   });
 
+  it('adding/removing aria-hidden without an appElement will try to fallback to document.body', () => {
+    ariaAppHider.documentNotReadyOrSSRTesting();
+    const node = document.createElement('div');
+    ReactDOM.render((
+      <Modal isOpen />
+    ), node);
+    expect(document.body.getAttribute('aria-hidden')).toEqual('true');
+    ReactDOM.unmountComponentAtNode(node);
+    expect(document.body.getAttribute('aria-hidden')).toEqual(null);
+  });
+
+  it('raise an exception if appElement is a selector and no elements were found.', () => {
+    expect(() => ariaAppHider.setElement('.test')).toThrow();
+  });
+
   it('removes aria-hidden from appElement when unmounted w/o closing', () => {
     const el = document.createElement('div');
     const node = document.createElement('div');
diff --git a/src/components/Modal.js b/src/components/Modal.js
@@ -11,15 +11,14 @@ const EE = ExecutionEnvironment;
 const renderSubtreeIntoContainer = ReactDOM.unstable_renderSubtreeIntoContainer;
 
 const SafeHTMLElement = EE.canUseDOM ? window.HTMLElement : {};
-const AppElement = EE.canUseDOM ? document.body : { appendChild() {} };
 
 function getParentElement(parentSelector) {
   return parentSelector();
 }
 
 export default class Modal extends Component {
   static setAppElement(element) {
-    ariaAppHider.setElement(element || AppElement);
+    ariaAppHider.setElement(element);
   }
 
   /* eslint-disable no-console */
diff --git a/src/helpers/ariaAppHider.js b/src/helpers/ariaAppHider.js
@@ -1,19 +1,38 @@
-let globalElement = typeof document !== 'undefined' ? document.body : null;
+let globalElement = null;
+
+export function assertNodeList(nodeList, selector) {
+  if (!nodeList || !nodeList.length) {
+    throw new Error(
+      `react-modal: No elements were found for selector ${selector}.`
+    );
+  }
+}
 
 export function setElement(element) {
   let useElement = element;
   if (typeof useElement === 'string') {
     const el = document.querySelectorAll(useElement);
+    assertNodeList(el, useElement);
     useElement = 'length' in el ? el[0] : el;
   }
   globalElement = useElement || globalElement;
   return globalElement;
 }
 
+export function tryForceFallback() {
+  if (document && document.body) {
+    // force fallback to document.body
+    setElement(document.body);
+    return true;
+  }
+  return false;
+}
+
 export function validateElement(appElement) {
-  if (!appElement && !globalElement) {
+  if (!appElement && !globalElement && !tryForceFallback()) {
     throw new Error([
-      'react-modal: You must set an element with',
+      'react-modal: Cannot fallback to `document.body`, because it\'s not ready or available.',
+      'If you are doing server-side rendering, use this function to defined an element.',
       '`Modal.setAppElement(el)` to make this accessible'
     ]);
   }
@@ -34,6 +53,10 @@ export function toggle(shouldHide, appElement) {
   apply(appElement);
 }
 
+export function documentNotReadyOrSSRTesting() {
+  globalElement = null;
+}
+
 export function resetForTesting() {
   globalElement = document.body;
 }

Original file line number	Diff line number	Diff line change
`@@ -11,15 +11,14 @@ const EE = ExecutionEnvironment;`
`11`	`11`	`const renderSubtreeIntoContainer = ReactDOM.unstable_renderSubtreeIntoContainer;`
`12`	`12`
`13`	`13`	`const SafeHTMLElement = EE.canUseDOM ? window.HTMLElement : {};`
`14`		`-const AppElement = EE.canUseDOM ? document.body : { appendChild() {} };`
`15`	`14`
`16`	`15`	`function getParentElement(parentSelector) {`
`17`	`16`	`return parentSelector();`
`18`	`17`	`}`
`19`	`18`
`20`	`19`	`export default class Modal extends Component {`
`21`	`20`	`static setAppElement(element) {`
`22`		`- ariaAppHider.setElement(element \|\| AppElement);`
	`21`	`+ ariaAppHider.setElement(element);`
`23`	`22`	`}`
`24`	`23`
`25`	`24`	`/* eslint-disable no-console */`