Skip to content

Commit

Permalink
Improve accessibility, fixes reactjs#359
Browse files Browse the repository at this point in the history
- adds aria-modal="true" to modal portal
- doesn't make body a default appElement
- warns if appElement is not set in any way
  • Loading branch information
sheerun committed Nov 29, 2017
1 parent 700eb4e commit bd5e012
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 39 deletions.
9 changes: 0 additions & 9 deletions specs/Modal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,15 +339,6 @@ export default () => {
unmountModal();
});

it("uses document.body for aria-hidden if no appElement", () => {
ariaAppHider.documentNotReadyOrSSRTesting();
const node = document.createElement("div");
ReactDOM.render(<Modal isOpen />, node);
document.body.getAttribute("aria-hidden").should.be.eql("true");
ReactDOM.unmountComponentAtNode(node);
should(document.body.getAttribute("aria-hidden")).not.be.ok();
});

it("raises an exception if the appElement selector does not match", () => {
should(() => ariaAppHider.setElement(".test")).throw();
});
Expand Down
17 changes: 7 additions & 10 deletions src/components/ModalPortal.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,7 @@ export default class ModalPortal extends Component {
}

componentWillUnmount() {
// Remove body class
bodyClassList.remove(this.props.bodyOpenClassName);
this.beforeClose();
this.afterClose();
clearTimeout(this.closeTimer);
}

Expand All @@ -127,17 +125,16 @@ export default class ModalPortal extends Component {
}
}

beforeClose() {
afterClose = () => {
const { appElement, ariaHideApp } = this.props;

// Remove body class
bodyClassList.remove(this.props.bodyOpenClassName);

// Reset aria-hidden attribute if all modals have been removed
if (ariaHideApp && refCount.totalCount() < 1) {
ariaAppHider.show(appElement);
}
}

afterClose = () => {
// Remove body class
bodyClassList.remove(this.props.bodyOpenClassName);

if (this.props.shouldFocusAfterRender) {
if (this.props.shouldReturnFocusAfterClose) {
Expand Down Expand Up @@ -171,7 +168,6 @@ export default class ModalPortal extends Component {
};

close = () => {
this.beforeClose();
if (this.props.closeTimeoutMS > 0) {
this.closeWithTimeout();
} else {
Expand Down Expand Up @@ -309,6 +305,7 @@ export default class ModalPortal extends Component {
onClick={this.handleOverlayOnClick}
onMouseDown={this.handleOverlayOnMouseDown}
onMouseUp={this.handleOverlayOnMouseUp}
aria-modal="true"
>
<div
ref={this.setContentRef}
Expand Down
35 changes: 15 additions & 20 deletions src/helpers/ariaAppHider.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,42 +19,37 @@ export function setElement(element) {
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 && !tryForceFallback()) {
throw new Error(
if (!appElement && !globalElement) {
console.warn(
[
"react-modal: Cannot fallback to `document.body`, because it is 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"
"react-modal: App element is not defined.",
"Please use `Modal.setAppElement(el)` or set `appElement` property."
].join(" ")
);

return false;
}

return true;
}

export function hide(appElement) {
validateElement(appElement);
(appElement || globalElement).setAttribute("aria-hidden", "true");
if(validateElement(appElement)) {
(appElement || globalElement).setAttribute("aria-hidden", "true");
}
}

export function show(appElement) {
validateElement(appElement);
(appElement || globalElement).removeAttribute("aria-hidden");
if(validateElement(appElement)) {
(appElement || globalElement).removeAttribute("aria-hidden");
}
}

export function documentNotReadyOrSSRTesting() {
globalElement = null;
}

export function resetForTesting() {
globalElement = document.body;
globalElement = null;
}

0 comments on commit bd5e012

Please sign in to comment.