From b67081b81d7c74edce6d341f8f54a880380d65b8 Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Sun, 14 Jun 2015 15:21:13 -0400 Subject: [PATCH] [fixed] rootClose behavior on replaced elements Fixes #802 --- src/OverlayMixin.js | 8 ++++-- src/RootCloseWrapper.js | 31 ++++++++++++++++---- test/OverlayTriggerSpec.js | 59 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 88 insertions(+), 10 deletions(-) diff --git a/src/OverlayMixin.js b/src/OverlayMixin.js index 76524ed61c..f8874a8828 100644 --- a/src/OverlayMixin.js +++ b/src/OverlayMixin.js @@ -37,7 +37,7 @@ export default { let overlay = this.renderOverlay(); - // Save reference to help testing + // Save reference for future access. if (overlay !== null) { this._overlayInstance = React.render(overlay, this._overlayTarget); } else { @@ -57,7 +57,11 @@ export default { } if (this._overlayInstance) { - return React.findDOMNode(this._overlayInstance); + if (this._overlayInstance.getWrappedDOMNode) { + return this._overlayInstance.getWrappedDOMNode(); + } else { + return React.findDOMNode(this._overlayInstance); + } } return null; diff --git a/src/RootCloseWrapper.js b/src/RootCloseWrapper.js index 146c688ab9..35fecab769 100644 --- a/src/RootCloseWrapper.js +++ b/src/RootCloseWrapper.js @@ -4,6 +4,15 @@ import EventListener from './utils/EventListener'; // TODO: Merge this logic with dropdown logic once #526 is done. +// TODO: Consider using an ES6 symbol here, once we use babel-runtime. +const CLICK_WAS_INSIDE = '__click_was_inside'; + +function suppressRootClose(event) { + // Tag the native event to prevent the root close logic on document click. + // This seems safer than using event.nativeEvent.stopImmediatePropagation(), + // which is only supported in IE >= 9. + event.nativeEvent[CLICK_WAS_INSIDE] = true; +} export default class RootCloseWrapper extends React.Component { constructor(props) { @@ -23,10 +32,8 @@ export default class RootCloseWrapper extends React.Component { } handleDocumentClick(e) { - // If the click originated from within this component, don't do anything. - // e.srcElement is required for IE8 as e.target is undefined - let target = e.target || e.srcElement; - if (domUtils.contains(React.findDOMNode(this), target)) { + // This is now the native event. + if (e[CLICK_WAS_INSIDE]) { return; } @@ -54,7 +61,21 @@ export default class RootCloseWrapper extends React.Component { } render() { - return React.Children.only(this.props.children); + // Wrap the child in a new element, so the child won't have to handle + // potentially combining multiple onClick listeners. + return ( +
+ {React.Children.only(this.props.children)} +
+ ); + } + + getWrappedDOMNode() { + // We can't use a ref to identify the wrapped child, since we might be + // stealing the ref from the owner, but we know exactly the DOM structure + // that will be rendered, so we can just do this to get the child's DOM + // node for doing size calculations in OverlayMixin. + return React.findDOMNode(this).children[0]; } componentWillUnmount() { diff --git a/test/OverlayTriggerSpec.js b/test/OverlayTriggerSpec.js index 756cb4a46d..75ae2fcf40 100644 --- a/test/OverlayTriggerSpec.js +++ b/test/OverlayTriggerSpec.js @@ -236,13 +236,66 @@ describe('OverlayTrigger', function() { }); it('Should have correct isOverlayShown state', function () { - const event = document.createEvent('HTMLEvents'); - event.initEvent('click', true, true); - document.documentElement.dispatchEvent(event); + document.documentElement.click(); + // Need to click this way for it to propagate to document element. instance.state.isOverlayShown.should.equal(testCase.shownAfterClick); }); }); }); + + describe('replaced overlay', function () { + let instance; + + beforeEach(function () { + class ReplacedOverlay extends React.Component { + constructor(props) { + super(props); + + this.handleClick = this.handleClick.bind(this); + this.state = {replaced: false}; + } + + handleClick() { + this.setState({replaced: true}); + } + + render() { + if (this.state.replaced) { + return ( +
replaced
+ ); + } else { + return ( +
+ + original + +
+ ); + } + } + } + + instance = ReactTestUtils.renderIntoDocument( + } + trigger='click' rootClose={true} + > + + + ); + const overlayTrigger = React.findDOMNode(instance); + ReactTestUtils.Simulate.click(overlayTrigger); + }); + + it('Should still be shown', function () { + // Need to click this way for it to propagate to document element. + const replaceOverlay = document.getElementById('replace-overlay'); + replaceOverlay.click(); + + instance.state.isOverlayShown.should.be.true; + }); + }); }); });