From f6768b751f8a51c796c0f7515b33510b4ee1691a Mon Sep 17 00:00:00 2001 From: Bruno Dias Date: Tue, 14 Feb 2017 20:59:36 -0300 Subject: [PATCH] [change] improve reliability on focus management. this PR allow a stack of modals to give back focus to parent modal. --- examples/basic/app.js | 32 ++++++++++++++++++++++++++------ lib/components/ModalPortal.js | 14 +++++++------- lib/helpers/focusManager.js | 18 +++++++++--------- specs/Modal.spec.js | 32 ++++++++++++++++++++++++++++++++ specs/helper.js | 16 +++++++++------- 5 files changed, 83 insertions(+), 29 deletions(-) diff --git a/examples/basic/app.js b/examples/basic/app.js index 648296a9..7e01f5b2 100644 --- a/examples/basic/app.js +++ b/examples/basic/app.js @@ -9,25 +9,34 @@ Modal.setAppElement('#example'); var App = React.createClass({ getInitialState: function() { - return { modalIsOpen: false }; + return { modalIsOpen: false, modal2: false }; }, openModal: function() { - this.setState({modalIsOpen: true}); + this.setState({ ...this.state, modalIsOpen: true }); }, closeModal: function() { - this.setState({modalIsOpen: false}); + this.setState({ ...this.state, modalIsOpen: false }); + }, + + openSecondModal: function(event) { + event.preventDefault(); + this.setState({ ...this.state, modal2:true }); + }, + + closeSecondModal: function() { + this.setState({ ...this.state, modal2:false }); }, handleModalCloseRequest: function() { // opportunity to validate something and keep the modal open even if it // requested to be closed - this.setState({modalIsOpen: false}); + this.setState({ ...this.state, modalIsOpen: false }); }, handleInputChange: function() { - this.setState({foo: 'bar'}); + this.setState({ foo: 'bar' }); }, handleOnAfterOpenModal: function() { @@ -38,9 +47,11 @@ var App = React.createClass({ render: function() { return (
- + + hi + + {}} + onRequestClose={this.closeSecondModal}> +

test

+
); } diff --git a/lib/components/ModalPortal.js b/lib/components/ModalPortal.js index c1a07b54..62108e0e 100644 --- a/lib/components/ModalPortal.js +++ b/lib/components/ModalPortal.js @@ -72,7 +72,12 @@ var ModalPortal = module.exports = React.createClass({ this.focusAfterRender = focus; }, - open: function() { + afterClose: function () { + focusManager.returnFocus(); + focusManager.teardownScopedFocus(); + }, + + open () { if (this.state.afterOpen && this.state.beforeClose) { clearTimeout(this.closeTimer); this.setState({ beforeClose: false }); @@ -114,12 +119,7 @@ var ModalPortal = module.exports = React.createClass({ beforeClose: false, isOpen: false, afterOpen: false, - }, this.afterClose); - }, - - afterClose: function() { - focusManager.returnFocus(); - focusManager.teardownScopedFocus(); + }, function () { this.afterClose() }.bind(this)) }, handleKeyDown: function(event) { diff --git a/lib/helpers/focusManager.js b/lib/helpers/focusManager.js index 548f69db..5d32db60 100644 --- a/lib/helpers/focusManager.js +++ b/lib/helpers/focusManager.js @@ -1,6 +1,6 @@ var findTabbable = require('../helpers/tabbable'); +var focusLaterElements = []; var modalElement = null; -var focusLaterElement = null; var needToFocus = false; function handleBlur(event) { @@ -15,8 +15,8 @@ function handleFocus(event) { } // need to see how jQuery shims document.on('focusin') so we don't need the // setTimeout, firefox doesn't support focusin, if it did, we could focus - // the element outside of a setTimeout. Side-effect of this implementation - // is that the document.body gets focus, and then we focus our element right + // the element outside of a setTimeout. Side-effect of this implementation + // is that the document.body gets focus, and then we focus our element right // after, seems fine. setTimeout(function() { if (modalElement.contains(document.activeElement)) @@ -28,17 +28,19 @@ function handleFocus(event) { } exports.markForFocusLater = function() { - focusLaterElement = document.activeElement; + focusLaterElements.push(document.activeElement); }; exports.returnFocus = function() { + var toFocus = null; try { - focusLaterElement.focus(); + toFocus = focusLaterElements.pop(); + toFocus.focus(); + return; } catch (e) { - console.warn('You tried to return focus to '+focusLaterElement+' but it is not in the DOM anymore'); + console.warn('You tried to return focus to '+toFocus+' but it is not in the DOM anymore'); } - focusLaterElement = null; }; exports.setupScopedFocus = function(element) { @@ -64,5 +66,3 @@ exports.teardownScopedFocus = function() { document.detachEvent('onFocus', handleFocus); } }; - - diff --git a/specs/Modal.spec.js b/specs/Modal.spec.js index 34f06dc4..1e7c4e82 100644 --- a/specs/Modal.spec.js +++ b/specs/Modal.spec.js @@ -109,6 +109,38 @@ describe('Modal', function () { }); }); + it('give back focus to previous element or modal.', function (done) { + var modal = renderModal({ + isOpen: true, + onRequestClose: function () { + unmountModal(); + done(); + } + }, null, function () {}); + + renderModal({ + isOpen: true, + onRequestClose: function () { + Simulate.keyDown(modal.portal.refs.content, { + // The keyCode is all that matters, so this works + key: 'FakeKeyToTestLater', + keyCode: 27, + which: 27 + }); + expect(document.activeElement).toEqual(modal.portal.refs.content); + } + }, null, function checkPortalFocus () { + expect(document.activeElement).toEqual(this.portal.refs.content); + Simulate.keyDown(this.portal.refs.content, { + // The keyCode is all that matters, so this works + key: 'FakeKeyToTestLater', + keyCode: 27, + which: 27 + }); + }); + }); + + it('does not focus the modal content when a descendent is already focused', function() { var input = ( {children} - , _currentDiv, callback); + , currentDiv, callback); }; export const unmountModal = function() { - ReactDOM.unmountComponentAtNode(_currentDiv); - document.body.removeChild(_currentDiv); - _currentDiv = null; + const currentDiv = divStack.pop(); + ReactDOM.unmountComponentAtNode(currentDiv); + document.body.removeChild(currentDiv); };