FIX: Do not rerender widget-dropdown on all clicks (#10100)

Because of how the dropdown was structured, as long it was in the DOM, all clicks outside the widget would rerender it.

This commit introduces `widget-dropdown-body` that handles the `clickOutside` callback and is rendered conditionally, so it won't get called when the dropdown is closed.

app/assets/javascripts/discourse/app/widgets/widget-dropdown.js

@@ -140,6 +140,66 @@ export const WidgetDropdownItemClass = {
 
 createWidget("widget-dropdown-item", WidgetDropdownItemClass);
 
+export const WidgetDropdownBodyClass = {
+  tagName: "div",
+
+  buildClasses(attrs) {
+    return `widget-dropdown-body ${attrs.class || ""}`;
+  },
+
+  init(attrs) {
+    schedule("afterRender", () => {
+      const dropdownHeader = document.querySelector(
+        `#${attrs.id} .widget-dropdown-header`
+      );
+      const dropdownBody = document.querySelector(
+        `#${attrs.id} .widget-dropdown-body`
+      );
+
+      if (dropdownHeader && dropdownBody) {
+        /* global Popper:true */
+        this._popper = Popper.createPopper(dropdownHeader, dropdownBody, {
+          strategy: "fixed",
+          placement: "bottom-start",
+          modifiers: [
+            {
+              name: "preventOverflow"
+            },
+            {
+              name: "offset",
+              options: {
+                offset: [0, 5]
+              }
+            }
+          ]
+        });
+      }
+    });
+  },
+
+  destroy() {
+    if (this._popper) {
+      this._popper.destroy();
+      this._popper = null;
+    }
+  },
+
+  clickOutside() {
+    this.sendWidgetAction("hideBody");
+  },
+
+  template: hbs`
+    {{#each attrs.content as |item|}}
+      {{attach
+        widget="widget-dropdown-item"
+        attrs=(hash item=item)
+      }}
+    {{/each}}
+  `
+};
+
+createWidget("widget-dropdown-body", WidgetDropdownBodyClass);
+
 export const WidgetDropdownClass = {
   tagName: "div",
 
@@ -178,21 +238,18 @@ export const WidgetDropdownClass = {
   },
 
   transform(attrs) {
-    const options = attrs.options || {};
-
     return {
-      options,
-      bodyClass: `widget-dropdown-body ${options.bodyClass || ""}`
+      options: attrs.options || {}
     };
   },
 
-  clickOutside() {
+  hideBody() {
     this.state.opened = false;
-    this.scheduleRerender();
   },
 
   _onChange(params) {
     this.state.opened = false;
+
     if (this.attrs.onChange) {
       if (typeof this.attrs.onChange === "string") {
         this.sendWidgetAction(this.attrs.onChange, params);
@@ -203,22 +260,7 @@ export const WidgetDropdownClass = {
   },
 
   _onTrigger() {
-    if (this.state.opened) {
-      this.state.opened = false;
-      this._closeDropdown(this.attrs.id);
-    } else {
-      this.state.opened = true;
-      this._openDropdown(this.attrs.id);
-    }
-
-    this._popper && this._popper.update();
-  },
-
-  destroy() {
-    if (this._popper) {
-      this._popper.destroy();
-      this._popper = null;
-    }
+    this.state.opened = !this.state.opened;
   },
 
   template: hbs`
@@ -234,50 +276,18 @@ export const WidgetDropdownClass = {
         )
       }}
 
-      <div class={{transformed.bodyClass}}>
-        {{#each attrs.content as |item|}}
-          {{attach
-            widget="widget-dropdown-item"
-            attrs=(hash item=item)
-          }}
-        {{/each}}
-      </div>
+      {{#if this.state.opened}}
+        {{attach
+          widget="widget-dropdown-body"
+          attrs=(hash
+            id=attrs.id
+            class=this.transformed.options.bodyClass
+            content=attrs.content
+          )
+        }}
+      {{/if}}
     {{/if}}
-  `,
-
-  _closeDropdown() {
-    this._popper && this._popper.destroy();
-  },
-
-  _openDropdown(id) {
-    const dropdownHeader = document.querySelector(
-      `#${id} .widget-dropdown-header`
-    );
-    const dropdownBody = document.querySelector(`#${id} .widget-dropdown-body`);
-
-    if (dropdownHeader && dropdownBody) {
-      /* global Popper:true */
-      this._popper = Popper.createPopper(dropdownHeader, dropdownBody, {
-        strategy: "fixed",
-        placement: "bottom-start",
-        modifiers: [
-          {
-            name: "preventOverflow"
-          },
-          {
-            name: "offset",
-            options: {
-              offset: [0, 5]
-            }
-          }
-        ]
-      });
-    }
-
-    schedule("afterRender", () => {
-      this._popper && this._popper.update();
-    });
-  }
+  `
 };
 
 export default createWidget("widget-dropdown", WidgetDropdownClass);

test/javascripts/widgets/widget-dropdown-test.js

@@ -112,7 +112,8 @@ widgetTest("content", {
     this.setProperties(DEFAULT_CONTENT);
   },
 
-  test(assert) {
+  async test(assert) {
+    await toggle();
     assert.equal(rowById(1).dataset.id, 1, "it creates rows");
     assert.equal(rowById(2).dataset.id, 2, "it creates rows");
     assert.equal(rowById(3).dataset.id, 3, "it creates rows");
@@ -143,6 +144,7 @@ widgetTest("onChange action", {
   },
 
   async test(assert) {
+    await toggle();
     await clickRowById(2);
     assert.equal(find("#test").text(), 2, "it calls the onChange actions");
   }
@@ -157,10 +159,14 @@ widgetTest("can be opened and closed", {
 
   async test(assert) {
     assert.ok(exists("#my-dropdown.closed"));
+    assert.ok(!exists("#my-dropdown .widget-dropdown-body"));
     await toggle();
+    assert.equal(rowById(2).innerText.trim(), "FooBar");
     assert.ok(exists("#my-dropdown.opened"));
+    assert.ok(exists("#my-dropdown .widget-dropdown-body"));
     await toggle();
     assert.ok(exists("#my-dropdown.closed"));
+    assert.ok(!exists("#my-dropdown .widget-dropdown-body"));
   }
 });
 
@@ -197,7 +203,8 @@ widgetTest("content with translatedLabel", {
     this.setProperties(DEFAULT_CONTENT);
   },
 
-  test(assert) {
+  async test(assert) {
+    await toggle();
     assert.equal(rowById(2).innerText.trim(), "FooBar");
   }
 });
@@ -216,7 +223,8 @@ widgetTest("content with label", {
     I18n.translations = this._translations;
   },
 
-  test(assert) {
+  async test(assert) {
+    await toggle();
     assert.equal(rowById(1).innerText.trim(), "FooBaz");
   }
 });
@@ -228,7 +236,8 @@ widgetTest("content with icon", {
     this.setProperties(DEFAULT_CONTENT);
   },
 
-  test(assert) {
+  async test(assert) {
+    await toggle();
     assert.ok(exists(rowById(3).querySelector(".d-icon-times")));
   }
 });
@@ -240,7 +249,8 @@ widgetTest("content with html", {
     this.setProperties(DEFAULT_CONTENT);
   },
 
-  test(assert) {
+  async test(assert) {
+    await toggle();
     assert.equal(rowById(4).innerHTML.trim(), "<span><b>baz</b></span>");
   }
 });
@@ -252,7 +262,8 @@ widgetTest("separator", {
     this.setProperties(DEFAULT_CONTENT);
   },
 
-  test(assert) {
+  async test(assert) {
+    await toggle();
     assert.ok(
       find(
         "#my-dropdown .widget-dropdown-item:nth-child(3)"
@@ -297,7 +308,8 @@ widgetTest("bodyClass option", {
     this.set("options", { bodyClass: "gigantic and-yet-small" });
   },
 
-  test(assert) {
+  async test(assert) {
+    await toggle();
     assert.ok(body().classList.contains("widget-dropdown-body"));
     assert.ok(body().classList.contains("gigantic"));
     assert.ok(body().classList.contains("and-yet-small"));