Skip to content

Commit f6768b7

Browse files
diasbrunoclaydiffrient
authored andcommitted
[change] improve reliability on focus management.
this PR allow a stack of modals to give back focus to parent modal.
1 parent 4232477 commit f6768b7

File tree

5 files changed

+83
-29
lines changed

5 files changed

+83
-29
lines changed

Diff for: examples/basic/app.js

+26-6
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,34 @@ Modal.setAppElement('#example');
99
var App = React.createClass({
1010

1111
getInitialState: function() {
12-
return { modalIsOpen: false };
12+
return { modalIsOpen: false, modal2: false };
1313
},
1414

1515
openModal: function() {
16-
this.setState({modalIsOpen: true});
16+
this.setState({ ...this.state, modalIsOpen: true });
1717
},
1818

1919
closeModal: function() {
20-
this.setState({modalIsOpen: false});
20+
this.setState({ ...this.state, modalIsOpen: false });
21+
},
22+
23+
openSecondModal: function(event) {
24+
event.preventDefault();
25+
this.setState({ ...this.state, modal2:true });
26+
},
27+
28+
closeSecondModal: function() {
29+
this.setState({ ...this.state, modal2:false });
2130
},
2231

2332
handleModalCloseRequest: function() {
2433
// opportunity to validate something and keep the modal open even if it
2534
// requested to be closed
26-
this.setState({modalIsOpen: false});
35+
this.setState({ ...this.state, modalIsOpen: false });
2736
},
2837

2938
handleInputChange: function() {
30-
this.setState({foo: 'bar'});
39+
this.setState({ foo: 'bar' });
3140
},
3241

3342
handleOnAfterOpenModal: function() {
@@ -38,9 +47,11 @@ var App = React.createClass({
3847
render: function() {
3948
return (
4049
<div>
41-
<button onClick={this.openModal}>Open Modal</button>
50+
<button onClick={this.openModal}>Open Modal A</button>
51+
<button onClick={this.openSecondModal}>Open Modal B</button>
4252
<Modal
4353
ref="mymodal"
54+
id="test"
4455
closeTimeoutMS={150}
4556
isOpen={this.state.modalIsOpen}
4657
onAfterOpen={this.handleOnAfterOpenModal}
@@ -59,8 +70,17 @@ var App = React.createClass({
5970
<button>hi</button>
6071
<button>hi</button>
6172
<button>hi</button>
73+
<button onClick={this.openSecondModal}>Open Modal B</button>
6274
</form>
6375
</Modal>
76+
<Modal ref="mymodal2"
77+
id="test2"
78+
closeTimeoutMS={150}
79+
isOpen={this.state.modal2}
80+
onAfterOpen={() => {}}
81+
onRequestClose={this.closeSecondModal}>
82+
<p>test</p>
83+
</Modal>
6484
</div>
6585
);
6686
}

Diff for: lib/components/ModalPortal.js

+7-7
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,12 @@ var ModalPortal = module.exports = React.createClass({
7272
this.focusAfterRender = focus;
7373
},
7474

75-
open: function() {
75+
afterClose: function () {
76+
focusManager.returnFocus();
77+
focusManager.teardownScopedFocus();
78+
},
79+
80+
open () {
7681
if (this.state.afterOpen && this.state.beforeClose) {
7782
clearTimeout(this.closeTimer);
7883
this.setState({ beforeClose: false });
@@ -114,12 +119,7 @@ var ModalPortal = module.exports = React.createClass({
114119
beforeClose: false,
115120
isOpen: false,
116121
afterOpen: false,
117-
}, this.afterClose);
118-
},
119-
120-
afterClose: function() {
121-
focusManager.returnFocus();
122-
focusManager.teardownScopedFocus();
122+
}, function () { this.afterClose() }.bind(this))
123123
},
124124

125125
handleKeyDown: function(event) {

Diff for: lib/helpers/focusManager.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
var findTabbable = require('../helpers/tabbable');
2+
var focusLaterElements = [];
23
var modalElement = null;
3-
var focusLaterElement = null;
44
var needToFocus = false;
55

66
function handleBlur(event) {
@@ -15,8 +15,8 @@ function handleFocus(event) {
1515
}
1616
// need to see how jQuery shims document.on('focusin') so we don't need the
1717
// setTimeout, firefox doesn't support focusin, if it did, we could focus
18-
// the element outside of a setTimeout. Side-effect of this implementation
19-
// is that the document.body gets focus, and then we focus our element right
18+
// the element outside of a setTimeout. Side-effect of this implementation
19+
// is that the document.body gets focus, and then we focus our element right
2020
// after, seems fine.
2121
setTimeout(function() {
2222
if (modalElement.contains(document.activeElement))
@@ -28,17 +28,19 @@ function handleFocus(event) {
2828
}
2929

3030
exports.markForFocusLater = function() {
31-
focusLaterElement = document.activeElement;
31+
focusLaterElements.push(document.activeElement);
3232
};
3333

3434
exports.returnFocus = function() {
35+
var toFocus = null;
3536
try {
36-
focusLaterElement.focus();
37+
toFocus = focusLaterElements.pop();
38+
toFocus.focus();
39+
return;
3740
}
3841
catch (e) {
39-
console.warn('You tried to return focus to '+focusLaterElement+' but it is not in the DOM anymore');
42+
console.warn('You tried to return focus to '+toFocus+' but it is not in the DOM anymore');
4043
}
41-
focusLaterElement = null;
4244
};
4345

4446
exports.setupScopedFocus = function(element) {
@@ -64,5 +66,3 @@ exports.teardownScopedFocus = function() {
6466
document.detachEvent('onFocus', handleFocus);
6567
}
6668
};
67-
68-

Diff for: specs/Modal.spec.js

+32
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,38 @@ describe('Modal', function () {
109109
});
110110
});
111111

112+
it('give back focus to previous element or modal.', function (done) {
113+
var modal = renderModal({
114+
isOpen: true,
115+
onRequestClose: function () {
116+
unmountModal();
117+
done();
118+
}
119+
}, null, function () {});
120+
121+
renderModal({
122+
isOpen: true,
123+
onRequestClose: function () {
124+
Simulate.keyDown(modal.portal.refs.content, {
125+
// The keyCode is all that matters, so this works
126+
key: 'FakeKeyToTestLater',
127+
keyCode: 27,
128+
which: 27
129+
});
130+
expect(document.activeElement).toEqual(modal.portal.refs.content);
131+
}
132+
}, null, function checkPortalFocus () {
133+
expect(document.activeElement).toEqual(this.portal.refs.content);
134+
Simulate.keyDown(this.portal.refs.content, {
135+
// The keyCode is all that matters, so this works
136+
key: 'FakeKeyToTestLater',
137+
keyCode: 27,
138+
which: 27
139+
});
140+
});
141+
});
142+
143+
112144
it('does not focus the modal content when a descendent is already focused', function() {
113145
var input = (
114146
<input

Diff for: specs/helper.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,21 @@ import React from 'react';
22
import ReactDOM from 'react-dom';
33
import Modal from '../lib/components/Modal';
44

5-
let _currentDiv = null;
5+
6+
const divStack = [];
67

78
export const renderModal = function(props, children, callback) {
89
props.ariaHideApp = false;
9-
_currentDiv = document.createElement('div');
10-
document.body.appendChild(_currentDiv);
10+
const currentDiv = document.createElement('div');
11+
divStack.push(currentDiv);
12+
document.body.appendChild(currentDiv);
1113
return ReactDOM.render(
1214
<Modal {...props}>{children}</Modal>
13-
, _currentDiv, callback);
15+
, currentDiv, callback);
1416
};
1517

1618
export const unmountModal = function() {
17-
ReactDOM.unmountComponentAtNode(_currentDiv);
18-
document.body.removeChild(_currentDiv);
19-
_currentDiv = null;
19+
const currentDiv = divStack.pop();
20+
ReactDOM.unmountComponentAtNode(currentDiv);
21+
document.body.removeChild(currentDiv);
2022
};

0 commit comments

Comments
 (0)