Skip to content

Commit

Permalink
[Modal] Change keydown handling to use synthetic event (#14134)
Browse files Browse the repository at this point in the history
* [Modal] Change keydown handling to use synthetic event

* Integrate PR feedback

* let's merge :)
  • Loading branch information
rfbotto authored and oliviertassinari committed Jan 14, 2019
1 parent 25b14ec commit 69240e5
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
21 changes: 19 additions & 2 deletions packages/material-ui/src/Modal/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ class Modal extends React.Component {
const container = getContainer(this.props.container, doc.body);

this.props.manager.add(this, container);
doc.addEventListener('keydown', this.handleDocumentKeyDown);
doc.addEventListener('focus', this.enforceFocus, true);

if (this.dialogRef) {
Expand Down Expand Up @@ -145,7 +144,6 @@ class Modal extends React.Component {
this.props.manager.remove(this);

const doc = ownerDocument(this.mountNode);
doc.removeEventListener('keydown', this.handleDocumentKeyDown);
doc.removeEventListener('focus', this.enforceFocus, true);

this.restoreLastFocus();
Expand All @@ -170,11 +168,22 @@ class Modal extends React.Component {
};

handleDocumentKeyDown = event => {
// event.defaultPrevented:
//
// Ignore events that have been `event.preventDefault()` marked.
// preventDefault() is meant to stop default behaviours like
// clicking a checkbox to check it, hitting a button to submit a form,
// and hitting left arrow to move the cursor in a text input etc.
// Only special HTML elements have these default bahaviours.
//
// To remove in v4.
if (keycode(event) !== 'esc' || !this.isTopModal() || event.defaultPrevented) {
return;
}

// Swallow the event, in case someone is listening for the escape key on the body.
event.stopPropagation();

if (this.props.onEscapeKeyDown) {
this.props.onEscapeKeyDown(event);
}
Expand Down Expand Up @@ -307,9 +316,17 @@ class Modal extends React.Component {
disablePortal={disablePortal}
onRendered={this.handleRendered}
>
{/*
Marking an element with the role presentation indicates to assistive technology
that this element should be ignored; it exists to support the web application and
is not meant for humans to interact with directly.
https://github.com/evcohen/eslint-plugin-jsx-a11y/blob/master/docs/rules/no-static-element-interactions.md
*/}
<div
data-mui-test="Modal"
ref={this.handleModalRef}
onKeyDown={this.handleDocumentKeyDown}
role="presentation"
className={classNames('mui-fixed', classes.root, className, {
[classes.hidden]: exited,
})}
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/Modal/Modal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ describe('<Modal />', () => {
it('should call onEscapeKeyDown and onClose', () => {
topModalStub.returns(true);
wrapper.setProps({ manager: { isTopModal: topModalStub } });
event = { keyCode: keycode('esc') };
event = { keyCode: keycode('esc'), stopPropagation: () => {} };

instance.handleDocumentKeyDown(event);
assert.strictEqual(topModalStub.callCount, 1);
Expand All @@ -302,7 +302,7 @@ describe('<Modal />', () => {
it('when disableEscapeKeyDown should call only onClose', () => {
topModalStub.returns(true);
wrapper.setProps({ disableEscapeKeyDown: true, manager: { isTopModal: topModalStub } });
event = { keyCode: keycode('esc') };
event = { keyCode: keycode('esc'), stopPropagation: () => {} };

instance.handleDocumentKeyDown(event);
assert.strictEqual(topModalStub.callCount, 1);
Expand Down

0 comments on commit 69240e5

Please sign in to comment.