From 2417b090a9993169ae64a26d7451b5de99f38f2c Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Fri, 22 Mar 2024 10:29:33 +0800
Subject: [PATCH] UX: Improve group selector in theme objects editor (#26312)

Why this change?

Prior to this change, the group selector was using the `<GroupChooser>`
component which is a `<MultiSelectComponent>` and is not ideal in our
situation when we only allow a single group to be selected.

The other problem is that we are doing an async load of the groups when
it is already loaded and available in the `Site` service.
---
 .../schema-theme-setting/types/group.gjs      | 29 +++++----
 .../tests/helpers/select-kit-helper.js        |  4 ++
 .../editor-test.gjs                           | 65 +++++++++++--------
 3 files changed, 61 insertions(+), 37 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/group.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/group.gjs
index 45e240275b8..72f2ea2de9f 100644
--- a/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/group.gjs
+++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/group.gjs
@@ -1,29 +1,36 @@
 import Component from "@glimmer/component";
 import { tracked } from "@glimmer/tracking";
-import { hash } from "@ember/helper";
 import { action } from "@ember/object";
-import Group from "discourse/models/group";
+import { service } from "@ember/service";
 import FieldInputDescription from "admin/components/schema-theme-setting/field-input-description";
-import GroupChooser from "select-kit/components/group-chooser";
+import ComboBoxComponent from "select-kit/components/combo-box";
 
 export default class SchemaThemeSettingTypeGroup extends Component {
+  @service site;
   @tracked value = this.args.value;
-  @tracked groups = Group.findAll().then((groups) => {
-    this.groups = groups;
-  });
+
+  required = this.args.spec.required;
 
   @action
   onInput(newVal) {
-    this.value = newVal[0];
-    this.args.onChange(newVal[0]);
+    this.value = newVal;
+    this.args.onChange(newVal);
+  }
+
+  get groupChooserOptions() {
+    return {
+      clearable: !this.required,
+      filterable: true,
+      none: null,
+    };
   }
 
   <template>
-    <GroupChooser
-      @content={{this.groups}}
+    <ComboBoxComponent
+      @content={{this.site.groups}}
       @value={{this.value}}
       @onChange={{this.onInput}}
-      @options={{hash maximum=1}}
+      @options={{this.groupChooserOptions}}
     />
 
     <FieldInputDescription @description={{@description}} />
diff --git a/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js b/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js
index 091c5bb0825..d6438a6db2c 100644
--- a/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js
+++ b/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js
@@ -268,6 +268,10 @@ export default function selectKit(selector) {
       return rowHelper(query(selector).querySelector(".select-kit-row.none"));
     },
 
+    clearButton() {
+      return query(selector).querySelector(".btn-clear");
+    },
+
     validationMessage() {
       const validationMessage = query(selector).querySelector(
         ".validation-message"
diff --git a/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs
index 1010c696d2d..bafa3a9c71d 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs
+++ b/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs
@@ -2,7 +2,6 @@ import { click, fillIn, render } from "@ember/test-helpers";
 import { module, test } from "qunit";
 import schemaAndData from "discourse/tests/fixtures/theme-setting-schema-data";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
-import pretender, { response } from "discourse/tests/helpers/create-pretender";
 import { queryAll } from "discourse/tests/helpers/qunit-helpers";
 import selectKit from "discourse/tests/helpers/select-kit-helper";
 import I18n from "discourse-i18n";
@@ -729,43 +728,57 @@ module(
     });
 
     test("input fields of type group", async function (assert) {
-      pretender.get("/groups/search.json", () => {
-        return response(200, [
-          { id: 23, name: "testers" },
-          { id: 74, name: "devs" },
-          { id: 89, name: "customers" },
-        ]);
+      const setting = ThemeSettings.create({
+        setting: "objects_setting",
+        objects_schema: {
+          name: "something",
+          identifier: "id",
+          properties: {
+            required_group: {
+              type: "group",
+              required: true,
+            },
+            not_required_group: {
+              type: "group",
+            },
+          },
+        },
+        value: [
+          {
+            required_group: 6,
+          },
+        ],
       });
 
-      const setting = schemaAndData(3);
-
       await render(<template>
         <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       const inputFields = new InputFieldsFromDOM();
-      const groupSelector = selectKit(
-        `${inputFields.fields.group_field.selector} .select-kit`
+
+      assert
+        .dom(inputFields.fields.required_group.labelElement)
+        .hasText("required_group*");
+
+      let groupSelector = selectKit(
+        `${inputFields.fields.required_group.selector} .select-kit`
       );
 
-      assert.strictEqual(groupSelector.header().value(), null);
+      assert.strictEqual(groupSelector.header().value(), "6");
+      assert.dom(groupSelector.clearButton()).doesNotExist("is not clearable");
+
+      assert
+        .dom(inputFields.fields.not_required_group.labelElement)
+        .hasText("not_required_group");
+
+      groupSelector = selectKit(
+        `${inputFields.fields.not_required_group.selector} .select-kit`
+      );
 
       await groupSelector.expand();
-      await groupSelector.selectRowByValue(74);
-      assert.strictEqual(groupSelector.header().value(), "74");
+      await groupSelector.selectRowByIndex(1);
 
-      const tree = new TreeFromDOM();
-      await click(tree.nodes[1].element);
-
-      assert.strictEqual(groupSelector.header().value(), null);
-      await groupSelector.expand();
-      await groupSelector.selectRowByValue(23);
-      assert.strictEqual(groupSelector.header().value(), "23");
-
-      tree.refresh();
-
-      await click(tree.nodes[0].element);
-      assert.strictEqual(groupSelector.header().value(), "74");
+      assert.dom(groupSelector.clearButton()).exists("is clearable");
     });
 
     test("generic identifier is used when identifier is not specified in the schema", async function (assert) {