From 131974b3a6f56d4b877123a93724c16dea34e5d5 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Mon, 9 May 2022 10:50:29 +0200
Subject: [PATCH] FIX: ensures d-popover closes when clicking on popper
 (#16675)

I think the no-invalid-interaction is fine here as on click Is not actually used for an expected interaction but as an event bubbling barrier.
---
 .../discourse/app/components/d-popover.js     | 49 +++++++++++++++----
 .../app/templates/components/d-popover.hbs    |  3 +-
 .../integration/components/d-popover-test.js  |  8 ++-
 3 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/d-popover.js b/app/assets/javascripts/discourse/app/components/d-popover.js
index a5c935fc400..6e2d25f456a 100644
--- a/app/assets/javascripts/discourse/app/components/d-popover.js
+++ b/app/assets/javascripts/discourse/app/components/d-popover.js
@@ -2,6 +2,8 @@ import Component from "@ember/component";
 import { iconHTML } from "discourse-common/lib/icon-library";
 import tippy from "tippy.js";
 import { guidFor } from "@ember/object/internals";
+import { action } from "@ember/object";
+import { next } from "@ember/runloop";
 
 export default class DiscoursePopover extends Component {
   tagName = "";
@@ -15,13 +17,30 @@ export default class DiscoursePopover extends Component {
   didInsertElement() {
     this._super(...arguments);
 
-    this._setupTippy();
+    this._tippyInstance = this._setupTippy();
+  }
+
+  willDestroyElement() {
+    this._super(...arguments);
+
+    this._tippyInstance?.destroy();
   }
 
   get componentId() {
     return guidFor(this);
   }
 
+  @action
+  close(event) {
+    event.preventDefault();
+
+    if (!this.isExpanded) {
+      return;
+    }
+
+    this._tippyInstance?.hide();
+  }
+
   _setupTippy() {
     const baseOptions = {
       trigger: "click",
@@ -30,6 +49,7 @@ export default class DiscoursePopover extends Component {
       interactive: true,
       allowHTML: false,
       appendTo: "parent",
+      hideOnClick: true,
       content:
         this.options?.content ||
         document
@@ -38,20 +58,27 @@ export default class DiscoursePopover extends Component {
             ":scope > .d-popover-content, :scope > div, :scope > ul"
           ),
       onShow: () => {
-        if (this.isDestroyed || this.isDestroying) {
-          return;
-        }
-        this.set("isExpanded", true);
+        next(() => {
+          if (this.isDestroyed || this.isDestroying) {
+            return;
+          }
+
+          this.set("isExpanded", true);
+        });
+        return true;
       },
       onHide: () => {
-        if (this.isDestroyed || this.isDestroying) {
-          return;
-        }
-        this.set("isExpanded", false);
+        next(() => {
+          if (this.isDestroyed || this.isDestroying) {
+            return;
+          }
+          this.set("isExpanded", false);
+        });
+        return true;
       },
     };
 
-    tippy(
+    const instance = tippy(
       document
         .getElementById(this.componentId)
         .querySelector(
@@ -59,5 +86,7 @@ export default class DiscoursePopover extends Component {
         ),
       Object.assign({}, baseOptions, this.options || {})
     );
+
+    return instance?.id ? instance : null;
   }
 }
diff --git a/app/assets/javascripts/discourse/app/templates/components/d-popover.hbs b/app/assets/javascripts/discourse/app/templates/components/d-popover.hbs
index f17c8d5f9dc..1b5582abab2 100644
--- a/app/assets/javascripts/discourse/app/templates/components/d-popover.hbs
+++ b/app/assets/javascripts/discourse/app/templates/components/d-popover.hbs
@@ -1,3 +1,4 @@
-<div id={{componentId}} class="d-popover {{class}} {{if isExpanded "is-expanded"}}">
+{{!-- template-lint-disable no-invalid-interactive --}}
+<div {{on "click" (action "close")}} id={{componentId}} class="d-popover {{class}} {{if isExpanded "is-expanded"}}">
   {{yield (hash isExpanded=isExpanded)}}
 </div>
diff --git a/app/assets/javascripts/discourse/tests/integration/components/d-popover-test.js b/app/assets/javascripts/discourse/tests/integration/components/d-popover-test.js
index 85ebf875ab5..a0608adcacd 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/d-popover-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/d-popover-test.js
@@ -39,16 +39,20 @@ discourseModule("Integration | Component | d-popover", function (hooks) {
   });
 
   componentTest("show/hide popover from component", {
-    template: hbs`{{#d-popover}}{{d-button icon="chevron-down"}}<ul><li class="test">foo</li></ul>{{/d-popover}}`,
+    template: hbs`{{#d-popover}}{{d-button class="trigger" icon="chevron-down"}}<ul><li class="test">foo</li><li>{{d-button icon="times" class="closer"}}</li></ul>{{/d-popover}}`,
 
     async test(assert) {
       assert.notOk(exists(".d-popover.is-expanded"));
       assert.notOk(exists(".test"));
 
-      await click(".btn");
+      await click(".trigger");
 
       assert.ok(exists(".d-popover.is-expanded"));
       assert.equal(query(".test").innerText.trim(), "foo");
+
+      await click(".closer");
+
+      assert.notOk(exists(".d-popover.is-expanded"));
     },
   });