fix(Popover): do not trigger toggle on popover click

TheSharpieOne · TheSharpieOne · commit 50a8fd41a508 · 2017-09-27T21:27:16.000-04:00
Closes #594
diff --git a/src/Popover.js b/src/Popover.js
@@ -50,6 +50,7 @@ class Popover extends React.Component {
     this.addTargetEvents = this.addTargetEvents.bind(this);
     this.handleDocumentClick = this.handleDocumentClick.bind(this);
     this.removeTargetEvents = this.removeTargetEvents.bind(this);
+    this.getRef = this.getRef.bind(this);
     this.toggle = this.toggle.bind(this);
     this.show = this.show.bind(this);
     this.hide = this.hide.bind(this);
@@ -70,6 +71,10 @@ class Popover extends React.Component {
     this.removeTargetEvents();
   }
 
+  getRef(ref) {
+    this._popover = ref;
+  }
+
   getDelay(key) {
     const { delay } = this.props;
     if (typeof delay === 'object') {
@@ -115,7 +120,7 @@ class Popover extends React.Component {
   }
 
   handleDocumentClick(e) {
-    if (e.target !== this._target && !this._target.contains(e.target)) {
+    if (e.target !== this._target && !this._target.contains(e.target) && e.target !== this._popover && !(this._popover && this._popover.contains(e.target))) {
       if (this._hideTimeout) {
         this.clearHideTimeout();
       }
@@ -172,7 +177,7 @@ class Popover extends React.Component {
         placementPrefix={this.props.placementPrefix}
         container={this.props.container}
       >
-        <div {...attributes} className={classes} />
+        <div {...attributes} className={classes} ref={this.getRef} />
       </PopperContent>
     );
   }
diff --git a/src/__tests__/Popover.spec.js b/src/__tests__/Popover.spec.js
@@ -16,10 +16,10 @@ describe('Popover', () => {
     container = document.createElement('div');
     element.innerHTML = '<p id="outerTarget">This is the popover <span id="innerTarget">target</span>.</p>';
     container.setAttribute('id', 'container');
-    outerTarget = document.getElementById('outerTarget');
-    innerTarget = document.getElementById('innerTarget');
     element.appendChild(container);
     document.body.appendChild(element);
+    outerTarget = document.getElementById('outerTarget');
+    innerTarget = document.getElementById('innerTarget');
     isOpen = false;
     toggle = () => { isOpen = !isOpen; };
     placement = 'top';
@@ -221,7 +221,7 @@ describe('Popover', () => {
     wrapper.detach();
   });
 
-  it('should NOT handle inner target clicks', () => {
+  it('should NOT handle inner target clicks when isOpen is false (user is responsible)', () => {
     const wrapper = mount(
       <Popover target="innerTarget" isOpen={isOpen} toggle={toggle}>
         Popover Content
@@ -238,6 +238,42 @@ describe('Popover', () => {
     wrapper.detach();
   });
 
+  it('should NOT handle inner target clicks when isOpen is true (user is responsible)', () => {
+    isOpen = true;
+    const wrapper = mount(
+      <Popover target="innerTarget" isOpen={isOpen} toggle={toggle}>
+        Popover Content
+      </Popover>,
+      { attachTo: container }
+    );
+    const instance = wrapper.instance();
+
+    expect(isOpen).toBe(true);
+    instance.handleDocumentClick({ target: innerTarget });
+    jest.runTimersToTime(0); // toggle is still async
+    expect(isOpen).toBe(true);
+
+    wrapper.detach();
+  });
+
+  it('should NOT handle popover target clicks', () => {
+    isOpen = true;
+    const wrapper = mount(
+      <Popover target="innerTarget" isOpen={isOpen} toggle={toggle}>
+        Popover Content
+      </Popover>,
+      { attachTo: container }
+    );
+    const instance = wrapper.instance();
+
+    expect(isOpen).toBe(true);
+    instance.handleDocumentClick({ target: instance._popover });
+    jest.runTimersToTime(0); // toggle is still async
+    expect(isOpen).toBe(true);
+
+    wrapper.detach();
+  });
+
   it('should not do anything when document click outside of target', () => {
     const wrapper = mount(
       <Popover target="innerTarget" isOpen={isOpen} toggle={toggle}>
