UX: move admin flag form to form-kit (#28187)

Rewrite the admin flag form to use FormKit. This is a draft because waiting for Checkbox improvements.

app/assets/javascripts/admin/addon/components/admin-flags-form.gjs

@@ -1,18 +1,13 @@
 import Component from "@glimmer/component";
-import { tracked } from "@glimmer/tracking";
+import { cached } from "@glimmer/tracking";
-import { fn, hash } from "@ember/helper";
+import { hash } from "@ember/helper";
-import { TextArea } from "@ember/legacy-built-in-components";
-import { on } from "@ember/modifier";
 import { action } from "@ember/object";
 import { LinkTo } from "@ember/routing";
 import { service } from "@ember/service";
-import { isEmpty } from "@ember/utils";
+import Form from "discourse/components/form";
-import { not } from "truth-helpers";
-import DButton from "discourse/components/d-button";
-import withEventValue from "discourse/helpers/with-event-value";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
-import dIcon from "discourse-common/helpers/d-icon";
+import icon from "discourse-common/helpers/d-icon";
 import i18n from "discourse-common/helpers/i18n";
 import { bind } from "discourse-common/utils/decorators";
 import I18n from "discourse-i18n";
@@ -23,33 +18,26 @@ export default class AdminFlagsForm extends Component {
   @service router;
   @service site;
 
-  @tracked enabled = true;
-  @tracked requireMessage = false;
-  @tracked name;
-  @tracked description;
-  @tracked appliesTo;
-
-  constructor() {
-    super(...arguments);
-    if (this.isUpdate) {
-      this.name = this.args.flag.name;
-      this.description = this.args.flag.description;
-      this.appliesTo = this.args.flag.applies_to;
-      this.requireMessage = this.args.flag.require_message;
-      this.enabled = this.args.flag.enabled;
-    }
-  }
-
   get isUpdate() {
     return this.args.flag;
   }
 
-  get isValid() {
+  @cached
-    return (
+  get formData() {
-      !isEmpty(this.name) &&
+    if (this.isUpdate) {
-      !isEmpty(this.description) &&
+      return {
-      !isEmpty(this.appliesTo)
+        name: this.args.flag.name,
-    );
+        description: this.args.flag.description,
+        appliesTo: this.args.flag.applies_to,
+        requireMessage: this.args.flag.require_message,
+        enabled: this.args.flag.enabled,
+      };
+    } else {
+      return {
+        enabled: true,
+        requireMessage: false,
+      };
+    }
   }
 
   get header() {
@@ -71,64 +59,59 @@ export default class AdminFlagsForm extends Component {
     });
   }
 
-  @action
+  validateAppliesTo(name, value, { addError }) {
-  save() {
+    if (value && value.length === 0) {
-    this.isUpdate ? this.update() : this.create();
+      addError("appliesTo", {
-  }
+        title: i18n("admin.config_areas.flags.form.applies_to"),
-
+        message: i18n("admin.config_areas.flags.form.invalid_applies_to"),
-  @action
-  onToggleRequireMessage(e) {
-    this.requireMessage = e.target.checked;
-  }
-
-  @action
-  onToggleEnabled(e) {
-    this.enabled = e.target.checked;
-  }
-
-  @bind
-  create() {
-    return ajax(`/admin/config/flags`, {
-      type: "POST",
-      data: this.#formData,
-    })
-      .then((response) => {
-        this.site.flagTypes.push(response.flag);
-        this.router.transitionTo("adminConfig.flags");
-      })
-      .catch((error) => {
-        return popupAjaxError(error);
       });
+    }
   }
 
-  @bind
+  @action
-  update() {
+  save({ name, description, appliesTo, requireMessage, enabled }) {
-    return ajax(`/admin/config/flags/${this.args.flag.id}`, {
+    const createOrUpdate = this.isUpdate ? this.update : this.create;
-      type: "PUT",
+    const data = {
-      data: this.#formData,
+      name,
-    })
+      description,
-      .then((response) => {
+      enabled,
-        this.args.flag.name = response.flag.name;
+      applies_to: appliesTo,
-        this.args.flag.description = response.flag.description;
+      require_message: requireMessage,
-        this.args.flag.applies_to = response.flag.applies_to;
-        this.args.flag.require_message = response.flag.require_message;
-        this.args.flag.enabled = response.flag.enabled;
-        this.router.transitionTo("adminConfig.flags");
-      })
-      .catch((error) => {
-        return popupAjaxError(error);
-      });
-  }
-
-  @bind
-  get #formData() {
-    return {
-      name: this.name,
-      description: this.description,
-      applies_to: this.appliesTo,
-      require_message: this.requireMessage,
-      enabled: this.enabled,
     };
+    createOrUpdate(data);
+  }
+
+  @bind
+  async create(data) {
+    try {
+      const response = await ajax("/admin/config/flags", {
+        type: "POST",
+        data,
+      });
+      this.site.flagTypes.push(response.flag);
+      this.router.transitionTo("adminConfig.flags");
+    } catch (error) {
+      popupAjaxError(error);
+    }
+  }
+
+  @bind
+  async update(data) {
+    try {
+      const response = await ajax(`/admin/config/flags/${this.args.flag.id}`, {
+        type: "PUT",
+        data,
+      });
+
+      this.args.flag.name = response.flag.name;
+      this.args.flag.description = response.flag.description;
+      this.args.flag.applies_to = response.flag.applies_to;
+      this.args.flag.require_message = response.flag.require_message;
+      this.args.flag.enabled = response.flag.enabled;
+      this.router.transitionTo("adminConfig.flags");
+    } catch (error) {
+      popupAjaxError(error);
+    }
   }
 
   <template>
@@ -138,89 +121,78 @@ export default class AdminFlagsForm extends Component {
         @route="adminConfig.flags"
         class="btn-default btn btn-icon-text btn-back"
       >
-        {{dIcon "chevron-left"}}
+        {{icon "chevron-left"}}
         {{i18n "admin.config_areas.flags.back"}}
       </LinkTo>
       <div class="admin-config-area__primary-content admin-flag-form">
         <AdminConfigAreaCard @heading={{this.header}}>
-          <div class="control-group">
+          <Form @onSubmit={{this.save}} @data={{this.formData}} as |form|>
-            <label for="name">
+            <form.Field
-              {{i18n "admin.config_areas.flags.form.name"}}
+              @name="name"
-            </label>
+              @title={{i18n "admin.config_areas.flags.form.name"}}
-            <input
+              @validation="required|length:3,200"
-              name="name"
+              @format="large"
-              type="text"
+              as |field|
-              value={{this.name}}
+            >
-              maxlength="200"
+              <field.Input />
-              class="admin-flag-form__name"
+            </form.Field>
-              {{on "input" (withEventValue (fn (mut this.name)))}}
-            />
-          </div>
 
-          <div class="control-group">
+            <form.Field
-            <label for="description">
+              @name="description"
-              {{i18n "admin.config_areas.flags.form.description"}}
+              @title={{i18n "admin.config_areas.flags.form.description"}}
-            </label>
+              @validation="length:0,1000"
-            <TextArea
+              as |field|
-              @value={{this.description}}
+            >
-              maxlength="1000"
+              <field.Textarea @height={{60}} />
-              class="admin-flag-form__description"
+            </form.Field>
-            />
-          </div>
 
-          <div class="control-group">
+            <form.Field
-            <label for="applies-to">
+              @name="appliesTo"
-              {{i18n "admin.config_areas.flags.form.applies_to"}}
+              @title={{i18n "admin.config_areas.flags.form.applies_to"}}
-            </label>
+              @validation="required"
-            <MultiSelect
+              @validate={{this.validateAppliesTo}}
-              @value={{this.appliesTo}}
+              as |field|
-              @content={{this.appliesToValues}}
+            >
-              @options={{hash allowAny=false}}
+              <field.Custom>
-              class="admin-flag-form__applies-to"
+                <MultiSelect
-            />
+                  @id={{field.id}}
-          </div>
+                  @value={{field.value}}
+                  @onChange={{field.set}}
+                  @content={{this.appliesToValues}}
+                  @options={{hash allowAny=false}}
+                  class="admin-flag-form__applies-to"
+                />
+              </field.Custom>
+            </form.Field>
 
-          <div class="control-group">
+            <form.CheckboxGroup as |checkboxGroup|>
-            <label class="checkbox-label admin-flag-form__require-reason">
+              <checkboxGroup.Field
-              <input
+                @name="requireMessage"
-                {{on "input" this.onToggleRequireMessage}}
+                @title={{i18n "admin.config_areas.flags.form.require_message"}}
-                type="checkbox"
+                as |field|
-                checked={{this.requireMessage}}
+              >
-              />
+                <field.Checkbox>
-              <div>
-                {{i18n "admin.config_areas.flags.form.require_message"}}
-                <div class="admin-flag-form__require-message-description">
                   {{i18n
                     "admin.config_areas.flags.form.require_message_description"
                   }}
-                </div>
+                </field.Checkbox>
-              </div>
+              </checkboxGroup.Field>
-            </label>
-          </div>
 
-          <div class="control-group">
+              <checkboxGroup.Field
-            <label class="checkbox-label admin-flag-form__enabled">
+                @name="enabled"
-              <input
+                @title={{i18n "admin.config_areas.flags.form.enabled"}}
-                {{on "input" this.onToggleEnabled}}
+                as |field|
-                type="checkbox"
+              >
-                checked={{this.enabled}}
+                <field.Checkbox />
-              />
+              </checkboxGroup.Field>
-              {{i18n "admin.config_areas.flags.form.enabled"}}
+            </form.CheckboxGroup>
-            </label>
-          </div>
 
-          <div class="alert alert-info admin_flag_form__info">
+            <form.Alert @icon="info-circle">
-            {{dIcon "info-circle"}}
+              {{i18n "admin.config_areas.flags.form.alert"}}
-            {{i18n "admin.config_areas.flags.form.alert"}}
+            </form.Alert>
-          </div>
 
-          <DButton
+            <form.Submit @label="admin.config_areas.flags.form.save" />
-            @action={{this.save}}
+          </Form>
-            @label="admin.config_areas.flags.form.save"
-            @ariaLabel="admin.config_areas.flags.form.save"
-            @disabled={{not this.isValid}}
-            class="btn-primary admin-flag-form__save"
-          />
         </AdminConfigAreaCard>
       </div>
     </div>

app/assets/javascripts/discourse/app/form-kit/components/fk/checkbox-group.gjs

@@ -0,0 +1,31 @@
+import { hash } from "@ember/helper";
+import FKField from "discourse/form-kit/components/fk/field";
+import FKFieldset from "discourse/form-kit/components/fk/fieldset";
+
+const FKCheckboxGroup = <template>
+  <FKFieldset
+    class="form-kit__checkbox-group"
+    @title={{@title}}
+    @description={{@description}}
+  >
+    {{yield
+      (hash
+        Field=(component
+          FKField
+          errors=@errors
+          addError=@addError
+          data=@data
+          set=@set
+          remove=@remove
+          registerField=@registerField
+          unregisterField=@unregisterField
+          triggerRevalidationFor=@triggerRevalidationFor
+          showMeta=false
+          showTitle=false
+        )
+      )
+    }}
+  </FKFieldset>
+</template>;
+
+export default FKCheckboxGroup;

app/assets/javascripts/discourse/app/form-kit/components/fk/control/checkbox.gjs

@@ -22,7 +22,12 @@ export default class FKControlCheckbox extends Component {
         ...attributes
         {{on "change" this.handleInput}}
       />
-      <span>{{yield}}</span>
+      <span class="form-kit__control-checkbox-content">
+        <span class="form-kit__control-checkbox-title">{{@field.title}}</span>
+        {{#if (has-block)}}
+          <span class="form-kit__control-checkbox-description">{{yield}}</span>
+        {{/if}}
+      </span>
     </FKLabel>
   </template>
 }

app/assets/javascripts/discourse/app/form-kit/components/fk/control/radio-group.gjs

@@ -1,6 +1,6 @@
 import Component from "@glimmer/component";
 import { hash } from "@ember/helper";
-import FKText from "discourse/form-kit/components/fk/text";
+import FKFieldset from "discourse/form-kit/components/fk/fieldset";
 import FKControlRadioGroupRadio from "./radio-group/radio";
 
 // eslint-disable-next-line ember/no-empty-glimmer-component-classes
@@ -8,17 +8,12 @@ export default class FKControlRadioGroup extends Component {
   static controlType = "radio-group";
 
   <template>
-    <fieldset class="form-kit__radio-group" ...attributes>
+    <FKFieldset
-      {{#if @title}}
+      class="form-kit__control-radio-group"
-        <legend class="form-kit__radio-group-title">{{@title}}</legend>
+      @title={{@title}}
-      {{/if}}
+      @subtitle={{@subtitle}}
-
+      ...attributes
-      {{#if @subtitle}}
+    >
-        <FKText class="form-kit__radio-group-subtitle">
-          {{@subtitle}}
-        </FKText>
-      {{/if}}
-
       {{yield
         (hash
           Radio=(component
@@ -26,6 +21,6 @@ export default class FKControlRadioGroup extends Component {
           )
         )
       }}
-    </fieldset>
+    </FKFieldset>
   </template>
 }

app/assets/javascripts/discourse/app/form-kit/components/fk/fieldset.gjs

@@ -0,0 +1,19 @@
+import FKText from "discourse/form-kit/components/fk/text";
+
+const FKFieldset = <template>
+  <fieldset name={{@name}} class="form-kit__fieldset" ...attributes>
+    {{#if @title}}
+      <legend class="form-kit__fieldset-title">{{@title}}</legend>
+    {{/if}}
+
+    {{#if @description}}
+      <FKText class="form-kit__fieldset-description">
+        {{@description}}
+      </FKText>
+    {{/if}}
+
+    {{yield}}
+  </fieldset>
+</template>;
+
+export default FKFieldset;

app/assets/javascripts/discourse/app/form-kit/components/fk/form.gjs

@@ -6,14 +6,17 @@ import { action } from "@ember/object";
 import { service } from "@ember/service";
 import DButton from "discourse/components/d-button";
 import FKAlert from "discourse/form-kit/components/fk/alert";
+import FKCheckboxGroup from "discourse/form-kit/components/fk/checkbox-group";
 import FKCollection from "discourse/form-kit/components/fk/collection";
 import FKContainer from "discourse/form-kit/components/fk/container";
 import FKControlConditionalContent from "discourse/form-kit/components/fk/control/conditional-content";
-import FKControlInputGroup from "discourse/form-kit/components/fk/control/input-group";
 import FKErrorsSummary from "discourse/form-kit/components/fk/errors-summary";
 import FKField from "discourse/form-kit/components/fk/field";
+import FKFieldset from "discourse/form-kit/components/fk/fieldset";
+import FKInputGroup from "discourse/form-kit/components/fk/input-group";
 import Row from "discourse/form-kit/components/fk/row";
 import FKSection from "discourse/form-kit/components/fk/section";
+import FKSubmit from "discourse/form-kit/components/fk/submit";
 import { VALIDATION_TYPES } from "discourse/form-kit/lib/constants";
 import FKFieldData from "discourse/form-kit/lib/fk-field-data";
 import FKFormData from "discourse/form-kit/lib/fk-form-data";
@@ -237,17 +240,17 @@ class FKForm extends Component {
         (hash
           Row=Row
           Section=FKSection
+          Fieldset=FKFieldset
           ConditionalContent=(component FKControlConditionalContent)
           Container=FKContainer
           Actions=(component FKSection class="form-kit__actions")
           Button=(component DButton class="form-kit__button")
           Alert=FKAlert
           Submit=(component
-            DButton
+            FKSubmit
             action=this.onSubmit
             forwardEvent=true
             class="btn-primary form-kit__button"
-            label="submit"
             type="submit"
             isLoading=this.isSubmitting
           )
@@ -280,7 +283,18 @@ class FKForm extends Component {
             triggerRevalidationFor=this.triggerRevalidationFor
           )
           InputGroup=(component
-            FKControlInputGroup
+            FKInputGroup
+            errors=this.formData.errors
+            addError=this.addError
+            data=this.formData
+            set=this.set
+            remove=this.remove
+            registerField=this.registerField
+            unregisterField=this.unregisterField
+            triggerRevalidationFor=this.triggerRevalidationFor
+          )
+          CheckboxGroup=(component
+            FKCheckboxGroup
             errors=this.formData.errors
             addError=this.addError
             data=this.formData

app/assets/javascripts/discourse/app/form-kit/components/fk/input-group.gjs

@@ -1,7 +1,7 @@
 import { hash } from "@ember/helper";
 import FKField from "discourse/form-kit/components/fk/field";
 
-const FKControlInputGroup = <template>
+const FKInputGroup = <template>
   <div class="form-kit__input-group">
     {{yield
       (hash
@@ -22,4 +22,4 @@ const FKControlInputGroup = <template>
   </div>
 </template>;
 
-export default FKControlInputGroup;
+export default FKInputGroup;

app/assets/javascripts/discourse/app/form-kit/components/fk/submit.gjs

@@ -0,0 +1,20 @@
+import Component from "@glimmer/component";
+import DButton from "discourse/components/d-button";
+
+export default class FKSubmit extends Component {
+  get label() {
+    return this.args.label ?? "submit";
+  }
+
+  <template>
+    <DButton
+      @label={{this.label}}
+      @action={{@onSubmit}}
+      @forwardEvent="true"
+      class="btn-primary form-kit__button"
+      type="submit"
+      isLoading={{@isSubmitting}}
+      ...attributes
+    />
+  </template>
+}

app/assets/javascripts/discourse/tests/helpers/form-kit-assertions.js

@@ -2,6 +2,38 @@ import { capitalize } from "@ember/string";
 import QUnit from "qunit";
 import { query } from "discourse/tests/helpers/qunit-helpers";
 
+class FieldsetHelper {
+  constructor(element, context, name) {
+    this.element = element;
+    this.name = name;
+    this.context = context;
+  }
+
+  hasTitle(title, message) {
+    this.context
+      .dom(this.element.querySelector(".form-kit__fieldset-title"))
+      .hasText(title, message);
+  }
+
+  hasDescription(description, message) {
+    this.context
+      .dom(this.element.querySelector(".form-kit__fieldset-description"))
+      .hasText(description, message);
+  }
+
+  includesText(content, message) {
+    this.context.dom(this.element).includesText(content, message);
+  }
+
+  doesNotExist(message) {
+    this.context.dom(this.element).doesNotExist(message);
+  }
+
+  exists(message) {
+    this.context.dom(this.element).exists(message);
+  }
+}
+
 class FieldHelper {
   constructor(element, context, name) {
     this.element = element;
@@ -74,12 +106,40 @@ class FieldHelper {
     }
   }
 
-  get isDisabled() {
+  isEnabled(message) {
+    this.context.notOk(this.disabled, message);
+  }
+
+  hasValue(value, message) {
+    this.context.deepEqual(this.value, value, message);
+  }
+
+  isDisabled(message) {
+    this.context.ok(this.disabled, message);
+  }
+
+  get disabled() {
     this.context
       .dom(this.element)
       .exists(`Could not find field (name: ${this.name}).`);
 
-    return this.element.dataset.disabled === "";
+    this.context.ok(this.element.dataset.disabled === "");
+  }
+
+  hasTitle(title, message) {
+    switch (this.element.dataset.controlType) {
+      case "checkbox": {
+        this.context
+          .dom(this.element.querySelector(".form-kit__control-checkbox-title"))
+          .hasText(title, message);
+        break;
+      }
+      default: {
+        this.context
+          .dom(this.element.querySelector(".form-kit__container-title"))
+          .hasText(title, message);
+      }
+    }
   }
 
   hasSubtitle(subtitle, message) {
@@ -89,9 +149,23 @@ class FieldHelper {
   }
 
   hasDescription(description, message) {
-    this.context
+    switch (this.element.dataset.controlType) {
-      .dom(this.element.querySelector(".form-kit__meta-description"))
+      case "checkbox": {
-      .hasText(description, message);
+        this.context
+          .dom(
+            this.element.querySelector(
+              ".form-kit__control-checkbox-description"
+            )
+          )
+          .hasText(description, message);
+        break;
+      }
+      default: {
+        this.context
+          .dom(this.element.querySelector(".form-kit__meta-description"))
+          .hasText(description, message);
+      }
+    }
   }
 
   hasCharCounter(current, max, message) {
@@ -154,54 +228,18 @@ class FormHelper {
       name
     );
   }
+
+  fieldset(name) {
+    return new FieldsetHelper(
+      query(`.form-kit__fieldset[name="${name}"]`, this.element),
+      this.context,
+      name
+    );
+  }
 }
 
 export function setupFormKitAssertions() {
   QUnit.assert.form = function (selector = "form") {
-    const form = new FormHelper(selector, this);
+    return new FormHelper(selector, this);
-    return {
-      hasErrors: (fields, message) => {
-        form.hasErrors(fields, message);
-      },
-      hasNoErrors: (fields, message) => {
-        form.hasNoErrors(fields, message);
-      },
-      field: (name) => {
-        const field = form.field(name);
-
-        return {
-          doesNotExist: (message) => {
-            field.doesNotExist(message);
-          },
-          hasSubtitle: (value, message) => {
-            field.hasSubtitle(value, message);
-          },
-          hasDescription: (value, message) => {
-            field.hasDescription(value, message);
-          },
-          exists: (message) => {
-            field.exists(message);
-          },
-          isDisabled: (message) => {
-            this.ok(field.disabled, message);
-          },
-          isEnabled: (message) => {
-            this.notOk(field.disabled, message);
-          },
-          hasError: (message) => {
-            field.hasError(message);
-          },
-          hasCharCounter: (current, max, message) => {
-            field.hasCharCounter(current, max, message);
-          },
-          hasNoError: (message) => {
-            field.hasNoError(message);
-          },
-          hasValue: (value, message) => {
-            this.deepEqual(field.value, value, message);
-          },
-        };
-      },
-    };
   };
 }

app/assets/javascripts/discourse/tests/integration/components/form-kit/layout/checkbox-group-test.gjs

@@ -0,0 +1,30 @@
+import { render } from "@ember/test-helpers";
+import { module, test } from "qunit";
+import Form from "discourse/components/form";
+import { setupRenderingTest } from "discourse/tests/helpers/component-test";
+
+module(
+  "Integration | Component | FormKit | Layout | CheckboxGroup",
+  function (hooks) {
+    setupRenderingTest(hooks);
+
+    test("default", async function (assert) {
+      await render(<template>
+        <Form as |form|>
+          <form.CheckboxGroup as |checkboxGroup|>
+            <checkboxGroup.Field @name="foo" @title="Foo" as |field|>
+              <field.Checkbox />
+            </checkboxGroup.Field>
+            <checkboxGroup.Field @name="bar" @title="Bar" as |field|>
+              <field.Checkbox>A description</field.Checkbox>
+            </checkboxGroup.Field>
+          </form.CheckboxGroup>
+        </Form>
+      </template>);
+
+      assert.form().field("foo").hasTitle("Foo");
+      assert.form().field("bar").hasTitle("Bar");
+      assert.form().field("bar").hasDescription("A description");
+    });
+  }
+);

app/assets/javascripts/discourse/tests/integration/components/form-kit/layout/fieldset-test.gjs

@@ -0,0 +1,38 @@
+import { render } from "@ember/test-helpers";
+import { module, test } from "qunit";
+import Form from "discourse/components/form";
+import { setupRenderingTest } from "discourse/tests/helpers/component-test";
+
+module(
+  "Integration | Component | FormKit | Layout | Fieldset",
+  function (hooks) {
+    setupRenderingTest(hooks);
+
+    test("default", async function (assert) {
+      await render(<template>
+        <Form as |form|>
+          <form.Fieldset
+            @title="Title"
+            @description="Description"
+            @name="a-fieldset"
+          >
+            Yielded content
+          </form.Fieldset>
+        </Form>
+      </template>);
+
+      assert
+        .form()
+        .fieldset("a-fieldset")
+        .hasTitle("Title", "it renders a title");
+      assert
+        .form()
+        .fieldset("a-fieldset")
+        .hasDescription("Description", "it renders a description");
+      assert
+        .form()
+        .fieldset("a-fieldset")
+        .includesText("Yielded content", "it yields its content");
+    });
+  }
+);

app/assets/javascripts/discourse/tests/integration/components/form-kit/layout/submit-test.gjs

@@ -26,4 +26,16 @@ module("Integration | Component | FormKit | Layout | Submit", function (hooks) {
     assert.dom(".form-kit__button.btn-primary").hasText(I18n.t("submit"));
     assert.deepEqual(value, 1);
   });
+
+  test("@label", async function (assert) {
+    await render(<template>
+      <Form as |form|>
+        <form.Submit @label="cancel" />
+      </Form>
+    </template>);
+
+    assert
+      .dom(".form-kit__button")
+      .hasText(I18n.t("cancel"), "it allows to override the label");
+  });
 });

app/assets/stylesheets/common/admin/flags.scss

@@ -36,37 +36,6 @@
   }
 }
 
-.admin-flag-form {
-  &__enabled,
-  &__applies-to {
-    margin-bottom: 1em;
-  }
-  &__save {
-    margin-top: 1em;
-  }
-  &__info {
-    color: var(--primary-high);
-    svg {
-      color: var(--tertiary);
-    }
-  }
-  &__description {
-    width: 60%;
-  }
-  &__applies-to.select-kit.multi-select {
-    width: 60%;
-  }
-  &__require-message-description {
-    clear: both;
-    flex-basis: 100%;
-    font-size: var(--font-down-2);
-    margin-top: 0.25em;
-  }
-  label {
-    flex-wrap: wrap;
-  }
-}
-
 .admin-flags__header {
   display: flex;
   justify-content: space-between;

app/assets/stylesheets/common/form-kit/_alert.scss

@@ -4,4 +4,5 @@
   width: 100%;
   border-radius: var(--d-border-radius);
   box-sizing: border-box;
+  padding: 0.5em;
 }

app/assets/stylesheets/common/form-kit/_checkbox-group.scss

@@ -0,0 +1,5 @@
+.form-kit__checkbox-group {
+  display: flex;
+  flex-direction: column;
+  gap: 0.75em;
+}

app/assets/stylesheets/common/form-kit/_control-checkbox.scss

@@ -1,25 +1,31 @@
-.form-kit__control-checkbox {
+.form-kit {
-  &[type="checkbox"] {
+  &__control-checkbox {
-    margin: 0.17em;
+    &[type="checkbox"] {
-    margin-right: 0;
+      margin: 0.17em;
-    margin-left: 0;
+      margin-right: 0;
-  }
+      margin-left: 0;
+    }
 
-  &-label {
+    &-label {
-    display: flex;
+      display: flex;
-    gap: 0.5em;
+      gap: 0.5em;
-    font-weight: normal !important;
+      font-weight: normal !important;
-    margin: 0;
+      margin: 0;
-    color: var(--primary);
+      color: var(--primary);
 
-    .form-kit__field[data-disabled] & {
+      .form-kit__field[data-disabled] & {
-      cursor: not-allowed;
+        cursor: not-allowed;
+      }
     }
   }
-}
 
-.form-kit__field-checkbox {
+  &__control-checkbox-content {
-  + .form-kit__field-checkbox {
+    display: flex;
-    margin-top: calc(-1 * var(--form-kit-gutter-y));
+    flex-direction: column;
+    gap: 0.25rem;
+  }
+
+  &__control-checkbox-description {
+    color: var(--primary-medium);
   }
 }

app/assets/stylesheets/common/form-kit/_control-radio-group.scss

@@ -1,14 +0,0 @@
-.form-kit__radio-group {
-  display: flex;
-  flex-direction: column;
-  gap: 0.75em;
-
-  &-title {
-    display: flex;
-    align-items: center;
-    gap: 0.25em;
-    margin: 0;
-    font-size: var(--font-down-1-rem);
-    color: var(--primary-high);
-  }
-}

app/assets/stylesheets/common/form-kit/_control-radio.scss

@@ -16,11 +16,11 @@
 .form-kit__control-radio-content {
   display: flex;
   flex-direction: column;
-  gap: 0.25rem;
 }
 
 .form-kit__control-radio-description {
   color: var(--primary-medium);
+  font-size: var(--font-down-1-rem);
 }
 
 .form-kit__inline-radio {

app/assets/stylesheets/common/form-kit/_control-textarea.scss

@@ -10,6 +10,6 @@
   // prevents firefox/chrome to add spacing under textarea
   display: block;
 
-  height: 150px !important;
+  height: 150px;
   border-radius: var(--d-input-border-radius);
 }

app/assets/stylesheets/common/form-kit/_fieldset.scss

@@ -0,0 +1,22 @@
+.form-kit {
+  &__fieldset {
+    display: flex;
+    flex-direction: column;
+    gap: 0.75rem;
+  }
+
+  &__fieldset-title {
+    display: flex;
+    align-items: center;
+    margin: 0 0 0.25rem;
+    font-size: var(--font-down-1-rem);
+    color: var(--primary-high);
+  }
+
+  &__fieldset-description {
+    display: flex;
+    align-items: center;
+    margin: 0;
+    color: var(--primary);
+  }
+}

app/assets/stylesheets/common/form-kit/_index.scss

@@ -1,4 +1,5 @@
 @import "_default-input-mixin";
+@import "_fieldset";
 @import "_alert";
 @import "_char-counter";
 @import "_col";
@@ -13,9 +14,9 @@
 @import "_control-image";
 @import "_control-input";
 @import "_control-input-group";
+@import "_checkbox-group";
 @import "_control-menu";
 @import "_control-radio";
-@import "_control-radio-group";
 @import "_control-select";
 @import "_control-custom";
 @import "_control-textarea";

app/assets/stylesheets/mobile/admin_flags.scss

@@ -5,13 +5,3 @@
     margin-right: 0;
   }
 }
-
-.admin-config-area__primary-content {
-  .admin-flag-form {
-    &__name,
-    &__applies-to.select-kit.multi-select,
-    &__description {
-      width: 100%;
-    }
-  }
-}

config/locales/client.en.yml

@@ -5560,6 +5560,7 @@ en:
             name: "Name"
             description: "Description"
             applies_to: "Display this flag on"
+            invalid_applies_to: "Required"
             topic: "topics"
             post: "posts"
             chat_message: "chat messages"

plugins/styleguide/assets/javascripts/discourse/components/sections/atoms/05-forms.hbs

@@ -79,11 +79,25 @@
   </Form>
 </StyleguideExample>
 
-<StyleguideExample @title="Checkbox">
+<StyleguideExample @title="CheckboxGroup">
   <Form as |form|>
-    <form.Field @title="Contract" @name="contract" as |field|>
+    <form.CheckboxGroup @title="I give explicit permission" as |checkboxGroup|>
-      <field.Checkbox>Accept the contract</field.Checkbox>
+      <checkboxGroup.Field
-    </form.Field>
+        @title="Use my email for any purpose."
+        @name="contract"
+        as |field|
+      >
+        <field.Checkbox>Including signing up for services I can't unsubscribe
+          to.</field.Checkbox>
+      </checkboxGroup.Field>
+      <checkboxGroup.Field
+        @title="Sign my soul away."
+        @name="contract2"
+        as |field|
+      >
+        <field.Checkbox>Will severly impact the afterlife experience.</field.Checkbox>
+      </checkboxGroup.Field>
+    </form.CheckboxGroup>
   </Form>
 </StyleguideExample>
 

spec/system/admin_flags_spec.rb

@@ -2,163 +2,162 @@
 
 describe "Admin Flags Page", type: :system do
   fab!(:admin)
-  fab!(:topic)
+  fab!(:post)
-  fab!(:post) { Fabricate(:post, topic: topic) }
 
   let(:topic_page) { PageObjects::Pages::Topic.new }
   let(:admin_flags_page) { PageObjects::Pages::AdminFlags.new }
   let(:admin_flag_form_page) { PageObjects::Pages::AdminFlagForm.new }
+  let(:flag_modal) { PageObjects::Modals::Flag.new }
 
   before { sign_in(admin) }
 
   it "allows admin to disable, change order, create, update and delete flags" do
     # disable
-    topic_page.visit_topic(post.topic)
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-    topic_page.open_flag_topic_modal
+
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
+    expect(flag_modal).to have_choices(
-      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else"],
+      "It's Inappropriate",
+      "It's Spam",
+      "It's Illegal",
+      "Something Else",
     )
 
-    visit "/admin/config/flags"
+    admin_flags_page.visit.toggle("spam")
-    admin_flags_page.toggle("spam")
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-    expect(page).not_to have_css(".admin-flag-item.spam.saving")
 
-    topic_page.visit_topic(post.topic)
+    expect(flag_modal).to have_choices("It's Inappropriate", "It's Illegal", "Something Else")
-    topic_page.open_flag_topic_modal
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
-      ["It's Inappropriate", "It's Illegal", "Something Else"],
-    )
 
     Flag.system.where(name: "spam").update!(enabled: true)
 
     # change order
-    topic_page.visit_topic(post.topic)
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-    topic_page.open_flag_topic_modal
+
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
+    expect(flag_modal).to have_choices(
-      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else"],
+      "It's Inappropriate",
+      "It's Spam",
+      "It's Illegal",
+      "Something Else",
     )
 
-    visit "/admin/config/flags"
+    admin_flags_page.visit.move_down("spam")
-    admin_flags_page.move_down("spam")
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-    expect(page).not_to have_css(".admin-flag-item.spam.saving")
 
-    topic_page.visit_topic(post.topic)
+    expect(flag_modal).to have_choices(
-    topic_page.open_flag_topic_modal
+      "It's Inappropriate",
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
+      "It's Illegal",
-      ["It's Inappropriate", "It's Illegal", "It's Spam", "Something Else"],
+      "It's Spam",
+      "Something Else",
     )
 
-    visit "/admin/config/flags"
+    admin_flags_page.visit.move_up("spam")
-    admin_flags_page.move_up("spam")
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-    expect(page).not_to have_css(".admin-flag-item.spam.saving")
 
-    topic_page.visit_topic(post.topic)
+    expect(flag_modal).to have_choices(
-    topic_page.open_flag_topic_modal
+      "It's Inappropriate",
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
+      "It's Spam",
-      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else"],
+      "It's Illegal",
+      "Something Else",
     )
 
     # create
-    topic_page.visit_topic(post.topic)
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-    topic_page.open_flag_topic_modal
+
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
+    expect(flag_modal).to have_choices(
-      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else"],
+      "It's Inappropriate",
+      "It's Spam",
+      "It's Illegal",
+      "Something Else",
     )
 
-    visit "/admin/config/flags"
+    admin_flags_page.visit.click_add_flag
+    admin_flag_form_page
+      .fill_in_name("Vulgar")
+      .fill_in_description("New flag description")
+      .select_applies_to("Topic")
+      .select_applies_to("Post")
+      .click_save
 
-    admin_flags_page.click_add_flag
+    expect(admin_flags_page).to have_flags(
-
+      "Send @%{username} a message",
-    expect(admin_flag_form_page).to have_disabled_save_button
+      "Off-Topic",
-
+      "Inappropriate",
-    admin_flag_form_page.fill_in_name("Vulgar")
+      "Spam",
-    admin_flag_form_page.fill_in_description("New flag description")
+      "Illegal",
-    admin_flag_form_page.fill_in_applies_to("Topic")
+      "Something Else",
-    admin_flag_form_page.fill_in_applies_to("Post")
+      "Vulgar",
-    admin_flag_form_page.click_save
-
-    expect(all(".admin-flag-item__name").map(&:text)).to eq(
-      [
-        "Send @%{username} a message",
-        "Off-Topic",
-        "Inappropriate",
-        "Spam",
-        "Illegal",
-        "Something Else",
-        "Vulgar",
-      ],
     )
 
-    topic_page.visit_topic(post.topic)
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-    topic_page.open_flag_topic_modal
+
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
+    expect(flag_modal).to have_choices(
-      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else", "Vulgar"],
+      "It's Inappropriate",
+      "It's Spam",
+      "It's Illegal",
+      "Something Else",
+      "Vulgar",
     )
 
     # update
-    visit "/admin/config/flags"
+    admin_flags_page.visit.click_edit_flag("vulgar")
+    admin_flag_form_page.fill_in_name("Tasteless").click_save
 
-    admin_flags_page.click_edit_flag("vulgar")
+    expect(admin_flags_page).to have_flags(
-
+      "Send @%{username} a message",
-    admin_flag_form_page.fill_in_name("Tasteless")
+      "Off-Topic",
-    admin_flag_form_page.click_save
+      "Inappropriate",
-
+      "Spam",
-    expect(all(".admin-flag-item__name").map(&:text)).to eq(
+      "Illegal",
-      [
+      "Something Else",
-        "Send @%{username} a message",
+      "Tasteless",
-        "Off-Topic",
-        "Inappropriate",
-        "Spam",
-        "Illegal",
-        "Something Else",
-        "Tasteless",
-      ],
     )
 
-    topic_page.visit_topic(post.topic)
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-    topic_page.open_flag_topic_modal
+
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
+    expect(flag_modal).to have_choices(
-      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else", "Tasteless"],
+      "It's Inappropriate",
+      "It's Spam",
+      "It's Illegal",
+      "Something Else",
+      "Tasteless",
     )
 
     # delete
-    visit "/admin/config/flags"
+    admin_flags_page.visit.click_delete_flag("tasteless").confirm_delete
-    admin_flags_page.click_delete_flag("tasteless")
-    admin_flags_page.confirm_delete
-    expect(page).not_to have_css(".admin-flag-item.tasteless.saving")
 
-    topic_page.visit_topic(post.topic)
+    expect(admin_flags_page).to have_no_flag("tasteless")
-    topic_page.open_flag_topic_modal
+
-    expect(all(".flag-action-type-details strong").map(&:text)).to eq(
+    topic_page.visit_topic(post.topic).open_flag_topic_modal
-      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else"],
+
+    expect(flag_modal).to have_choices(
+      "It's Inappropriate",
+      "It's Spam",
+      "It's Illegal",
+      "Something Else",
     )
   end
 
   it "does not allow to move notify user flag" do
-    visit "/admin/config/flags"
+    admin_flags_page.visit
-    expect(page).not_to have_css(".notify_user .flag-menu-trigger")
+    expect(admin_flags_page).to have_no_action_for_flag("notify_user")
   end
 
   it "does not allow bottom flag to move down" do
-    visit "/admin/config/flags"
+    admin_flags_page.visit.open_flag_menu("notify_moderators")
-    admin_flags_page.open_flag_menu("notify_moderators")
+    expect(admin_flags_page).to have_no_item_action("move-down")
-    expect(page).not_to have_css(".dropdown-menu__item .move-down")
   end
 
   it "does not allow to system flag to be edited" do
-    visit "/admin/config/flags"
+    admin_flags_page.visit
-    expect(page).to have_css(".off_topic .admin-flag-item__edit[disabled]")
+    expect(admin_flags_page).to have_disabled_edit_for_flag("off_topic")
   end
 
   it "does not allow to system flag to be deleted" do
-    visit "/admin/config/flags"
+    admin_flags_page.visit.open_flag_menu("notify_moderators")
-    admin_flags_page.open_flag_menu("notify_moderators")
+    expect(admin_flags_page).to have_disabled_item_action("delete")
-    expect(page).to have_css(".admin-flag-item__delete[disabled]")
   end
 
   it "does not allow top flag to move up" do
-    visit "/admin/config/flags"
+    admin_flags_page.visit.open_flag_menu("off_topic")
-    admin_flags_page.open_flag_menu("off_topic")
+    expect(admin_flags_page).to have_no_item_action("move-up")
-    expect(page).not_to have_css(".dropdown-menu__item .move-up")
   end
 end

spec/system/page_objects/modals/flag.rb

@@ -28,6 +28,10 @@ module PageObjects
       def check_confirmation
         body.check("confirmation")
       end
+
+      def has_choices?(*choices)
+        body.all(".flag-action-type-details strong").map(&:text) == choices
+      end
     end
   end
 end

spec/system/page_objects/pages/admin_flag_form.rb

@@ -3,27 +3,30 @@
 module PageObjects
   module Pages
     class AdminFlagForm < PageObjects::Pages::Base
-      def has_disabled_save_button?
-        find_button("Save", disabled: true)
-      end
-
       def fill_in_name(name)
-        find(".admin-flag-form__name").fill_in(with: name)
+        form.field("name").fill_in(name)
+        self
       end
 
       def fill_in_description(description)
-        find(".admin-flag-form__description").fill_in(with: description)
+        form.field("description").fill_in(description)
+        self
       end
 
-      def fill_in_applies_to(applies_to)
+      def select_applies_to(applies_to)
         dropdown = PageObjects::Components::SelectKit.new(".admin-flag-form__applies-to")
         dropdown.expand
         dropdown.select_row_by_value(applies_to)
         dropdown.collapse
+        self
       end
 
       def click_save
-        find(".admin-flag-form__save").click
+        form.submit
+      end
+
+      def form
+        @form ||= PageObjects::Components::FormKit.new(".admin-flag-form .form-kit")
       end
     end
   end

spec/system/page_objects/pages/admin_flags.rb

@@ -3,39 +3,96 @@
 module PageObjects
   module Pages
     class AdminFlags < PageObjects::Pages::Base
+      def visit
+        page.visit("/admin/config/flags")
+        self
+      end
+
       def toggle(key)
         PageObjects::Components::DToggleSwitch.new(".admin-flag-item__toggle.#{key}").toggle
+        has_saved_flag?(key)
+        self
       end
 
       def open_flag_menu(key)
         find(".#{key} .flag-menu-trigger").click
+        self
+      end
+
+      def has_action_for_flag?(flag)
+        has_selector?(".#{flag} .flag-menu-trigger")
+      end
+
+      def has_no_action_for_flag?(flag)
+        has_no_selector?(".#{flag} .flag-menu-trigger")
+      end
+
+      def has_disabled_edit_for_flag?(flag)
+        has_selector?(".#{flag} .admin-flag-item__edit[disabled]")
+      end
+
+      def has_disabled_item_action?(action)
+        has_selector?(".admin-flag-item__#{action}[disabled]")
+      end
+
+      def has_item_action?(action)
+        has_selector?(".admin-flag-item__#{action}")
+      end
+
+      def has_no_item_action?(action)
+        has_no_selector?(".admin-flag-item__#{action}")
+      end
+
+      def has_flags?(*flags)
+        all(".admin-flag-item__name").map(&:text) == flags
+      end
+
+      def has_flag?(flag)
+        has_css?(".admin-flag-item.#{flag}")
+      end
+
+      def has_no_flag?(flag)
+        has_no_css?(".admin-flag-item.#{flag}")
+      end
+
+      def has_saved_flag?(key)
+        has_css?(".admin-flag-item.#{key}.saving")
+        has_no_css?(".admin-flag-item.#{key}.saving")
       end
 
       def move_down(key)
         open_flag_menu(key)
         find(".admin-flag-item__move-down").click
+        has_saved_flag?(key)
+        self
       end
 
       def move_up(key)
         open_flag_menu(key)
         find(".admin-flag-item__move-up").click
+        has_saved_flag?(key)
+        self
       end
 
       def click_add_flag
         find(".admin-flags__header-add-flag").click
+        self
       end
 
       def click_edit_flag(key)
         find(".#{key} .admin-flag-item__edit").click
+        self
       end
 
       def click_delete_flag(key)
         find(".#{key} .flag-menu-trigger").click
         find(".admin-flag-item__delete").click
+        self
       end
 
       def confirm_delete
         find(".dialog-footer .btn-primary").click
+        self
       end
     end
   end