From 9a3542a8808c463988581a510697b83edb225a28 Mon Sep 17 00:00:00 2001 From: Bruno Dias Date: Tue, 19 Dec 2017 15:54:57 -0300 Subject: [PATCH] [fixed] stop propagating ESC key event. This is now required when nesting modals (the event will trigger both handleKeyDown). closes #583. --- examples/basic/app.js | 6 +- examples/basic/forms/index.js | 3 +- examples/basic/multiple_modals/index.js | 2 +- examples/basic/nested_modals/index.js | 117 ++++++++++++++++++++++++ examples/basic/react-router/index.js | 2 +- examples/basic/simple_usage/index.js | 2 +- specs/Modal.events.spec.js | 30 ++++++ src/components/ModalPortal.js | 2 +- 8 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 examples/basic/nested_modals/index.js diff --git a/examples/basic/app.js b/examples/basic/app.js index 58118a47..6854916d 100644 --- a/examples/basic/app.js +++ b/examples/basic/app.js @@ -5,6 +5,7 @@ import SimpleUsage from './simple_usage'; import MultipleModals from './multiple_modals'; import Forms from './forms'; import ReactRouter from './react-router'; +import NestedModals from './nested_modals'; const appElement = document.getElementById('example'); @@ -14,6 +15,7 @@ const examples = [ SimpleUsage, Forms, MultipleModals, + NestedModals, ReactRouter ]; @@ -24,8 +26,8 @@ class App extends Component { {examples.map((example, key) => { const ExampleApp = example.app; return ( -
-

{example.label}

+
+

{`#${key + 1}. ${example.label}`}

); diff --git a/examples/basic/forms/index.js b/examples/basic/forms/index.js index 550194ec..6826a53e 100644 --- a/examples/basic/forms/index.js +++ b/examples/basic/forms/index.js @@ -39,7 +39,6 @@ class Forms extends Component {

Forms!

This is a description of what it does: nothing :)

-
@@ -73,6 +72,6 @@ class Forms extends Component { } export default { - label: "#3. Modal with forms fields.", + label: "Modal with forms fields.", app: Forms }; diff --git a/examples/basic/multiple_modals/index.js b/examples/basic/multiple_modals/index.js index eadc4aff..abd7de83 100644 --- a/examples/basic/multiple_modals/index.js +++ b/examples/basic/multiple_modals/index.js @@ -109,6 +109,6 @@ class MultipleModals extends Component { } export default { - label: "#2. Working with many modal.", + label: "Working with many modal.", app: MultipleModals }; diff --git a/examples/basic/nested_modals/index.js b/examples/basic/nested_modals/index.js new file mode 100644 index 00000000..4b46795a --- /dev/null +++ b/examples/basic/nested_modals/index.js @@ -0,0 +1,117 @@ +import React, { Component } from 'react'; +import Modal from 'react-modal'; + +class Item extends Component { + constructor(props) { + super(props); + this.state = { + isOpen: false + }; + } + + toggleModal = index => event => { + console.log("NESTED MODAL ITEM", event); + this.setState({ + itemNumber: !this.state.isOpen ? index : null, + isOpen: !this.state.isOpen + }); + }; + + render() { + const { isOpen, itemNumber } = this.state; + const { number, index } = this.props; + + const toggleModal = this.toggleModal(index); + + return ( +
+ {number} + +

Item: {itemNumber}

+
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur pulvinar varius auctor. Aliquam maximus et justo ut faucibus. Nullam sit amet urna molestie turpis bibendum accumsan a id sem. Proin ullamcorper nisl sapien, gravida dictum nibh congue vel. Vivamus convallis dolor vitae ipsum ultricies, vitae pulvinar justo tincidunt. Maecenas a nunc elit. Phasellus fermentum, tellus ut consectetur scelerisque, eros nunc lacinia eros, aliquet efficitur tellus arcu a nibh. Praesent quis consequat nulla. Etiam dapibus ac sem vel efficitur. Nunc faucibus efficitur leo vitae vulputate. Nunc at quam vitae felis pretium vehicula vel eu quam. Quisque sapien mauris, condimentum eget dictum ut, congue id dolor. Donec vitae varius orci, eu faucibus turpis. Morbi eleifend orci non urna bibendum, ac scelerisque augue efficitur.

+
+ +
+ ); + } +} + +class List extends Component { + render() { + return this.props.items.map((n, index) => ( + + )); + } +} + + +class NestedModals extends Component { + constructor(props) { + super(props); + + this.state = { + isOpen: false, + currentItem: -1, + loading: false, + items: [] + }; + } + + toggleModal = event => { + event.preventDefault(); + console.log("NESTEDMODAL", event); + this.setState({ + items: [], + isOpen: !this.state.isOpen, + loading: true + }); + } + + handleOnAfterOpenModal = () => { + // when ready, we can access the available refs. + (new Promise((resolve, reject) => { + setTimeout(() => resolve(true), 500); + })).then(res => { + this.setState({ + items: [1, 2, 3, 4, 5].map(x => `Item ${x}`), + loading: false + }); + }); + } + + render() { + const { isOpen } = this.state; + return ( +
+ + +

List of items

+ {this.state.loading ? ( +

Loading...

+ ) : ( + + )} + +
+ ); + } +} + +export default { + label: "Working with nested modals.", + app: NestedModals +}; diff --git a/examples/basic/react-router/index.js b/examples/basic/react-router/index.js index 02f85781..042c4e4c 100644 --- a/examples/basic/react-router/index.js +++ b/examples/basic/react-router/index.js @@ -42,6 +42,6 @@ class App extends Component { } export default { - label: "#3. react-modal and react-router.", + label: "react-modal and react-router.", app: App }; diff --git a/examples/basic/simple_usage/index.js b/examples/basic/simple_usage/index.js index 4ab6676d..62997040 100644 --- a/examples/basic/simple_usage/index.js +++ b/examples/basic/simple_usage/index.js @@ -90,6 +90,6 @@ class SimpleUsage extends Component { } export default { - label: "#1. Working with one modal at a time.", + label: "Working with one modal at a time.", app: SimpleUsage }; diff --git a/specs/Modal.events.spec.js b/specs/Modal.events.spec.js index 4c3af5fd..af9c2fcc 100644 --- a/specs/Modal.events.spec.js +++ b/specs/Modal.events.spec.js @@ -1,6 +1,8 @@ /* eslint-env mocha */ +import React from "react"; import "should"; import sinon from "sinon"; +import Modal from "react-modal"; import { moverlay, mcontent, @@ -151,4 +153,32 @@ export default () => { const event = requestCloseCallback.getCall(0).args[0]; event.should.be.ok(); }); + + it("on nested modals, only the topmost should handle ESC key.", () => { + const requestCloseCallback = sinon.spy(); + const innerRequestCloseCallback = sinon.spy(); + let innerModal = null; + let innerModalRef = ref => { + innerModal = ref; + }; + + renderModal( + { + isOpen: true, + onRequestClose: requestCloseCallback + }, + + Test + + ); + + const content = mcontent(innerModal); + escKeyDown(content); + innerRequestCloseCallback.called.should.be.ok(); + requestCloseCallback.called.should.not.be.ok(); + }); }; diff --git a/src/components/ModalPortal.js b/src/components/ModalPortal.js index 8571fd23..7d2efeca 100644 --- a/src/components/ModalPortal.js +++ b/src/components/ModalPortal.js @@ -207,7 +207,7 @@ export default class ModalPortal extends Component { } if (this.props.shouldCloseOnEsc && event.keyCode === ESC_KEY) { - event.preventDefault(); + event.stopPropagation(); this.requestClose(event); } };