Skip to content

Commit

Permalink
[fixed] Make the modal portal render into body again (#176)
Browse files Browse the repository at this point in the history
In c13fed9 behavior was changed
so that the modal portal ended up rendering into the app element.

In the default case where the appElement receives aria-hidden
when the modal is open, this meant that the modal itself was
also contained inside the aria-hidden element.  This limits the
ability for screenreaders to read the modal contents.

Server-side rendering should not be affected by this change
because componentDidMount is only fired on the client-side thus
having a reference to document.body there should be acceptable.

Upgrade Path:
  - Make sure that you are setting an app element (rather than
    using the default document.body) so that modal contents are
    not being aria-hidden
  • Loading branch information
claydiffrient committed May 17, 2016
1 parent e9aff7a commit 9089a2d
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 17 deletions.
4 changes: 2 additions & 2 deletions lib/components/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ var Modal = React.createClass({
componentDidMount: function() {
this.node = document.createElement('div');
this.node.className = 'ReactModalPortal';
AppElement.appendChild(this.node);
document.body.appendChild(this.node);
this.renderPortal(this.props);
},

Expand All @@ -60,7 +60,7 @@ var Modal = React.createClass({

componentWillUnmount: function() {
ReactDOM.unmountComponentAtNode(this.node);
AppElement.removeChild(this.node);
document.body.removeChild(this.node);
elementClass(document.body).remove('ReactModal__Body--open');
},

Expand Down
17 changes: 2 additions & 15 deletions specs/Modal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,6 @@ describe('Modal', function () {
unmountModal();
});

it('uses the global appElement', function() {
var app = document.createElement('div');
var node = document.createElement('div');
Modal.setAppElement(app);
ReactDOM.render(React.createElement(Modal, {isOpen: true}), node);
var modalParent = app.querySelector('.ReactModalPortal').parentNode;
assert.notEqual(modalParent, document.body);
assert.equal(modalParent, app);
equal(app.getAttribute('aria-hidden'), 'true');
ariaAppHider.resetForTesting();
ReactDOM.unmountComponentAtNode(node);
});

it('accepts appElement as a prop', function() {
var el = document.createElement('div');
var node = document.createElement('div');
Expand All @@ -54,10 +41,10 @@ describe('Modal', function () {
var node = document.createElement('div');
var App = React.createClass({
render: function() {
return React.DOM.div({}, React.createElement(Modal, {isOpen: true, ariaHideApp: false}, 'hello'));
return React.DOM.div({}, React.createElement(Modal, {isOpen: true}, 'hello'));
}
});
Modal.setAppElement(document.body);
Modal.setAppElement(node);
ReactDOM.render(React.createElement(App), node);
var modalParent = document.body.querySelector('.ReactModalPortal').parentNode;
equal(modalParent, document.body);
Expand Down

0 comments on commit 9089a2d

Please sign in to comment.