From 300ef67481f22c6232bb7b0cf0f5dff545a0d535 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Mon, 5 Aug 2024 11:01:25 +1000 Subject: [PATCH] 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. --- .../addon/components/admin-flags-form.gjs | 284 ++++++++---------- .../form-kit/components/fk/checkbox-group.gjs | 31 ++ .../components/fk/control/checkbox.gjs | 7 +- .../components/fk/control/radio-group.gjs | 21 +- .../app/form-kit/components/fk/fieldset.gjs | 19 ++ .../app/form-kit/components/fk/form.gjs | 22 +- .../fk/{control => }/input-group.gjs | 4 +- .../app/form-kit/components/fk/submit.gjs | 20 ++ .../tests/helpers/form-kit-assertions.js | 138 ++++++--- .../form-kit/layout/checkbox-group-test.gjs | 30 ++ .../form-kit/layout/fieldset-test.gjs | 38 +++ .../form-kit/layout/submit-test.gjs | 12 + .../stylesheets/common/admin/flags.scss | 31 -- .../stylesheets/common/form-kit/_alert.scss | 1 + .../common/form-kit/_checkbox-group.scss | 5 + .../common/form-kit/_control-checkbox.scss | 42 +-- .../common/form-kit/_control-radio-group.scss | 14 - .../common/form-kit/_control-radio.scss | 2 +- .../common/form-kit/_control-textarea.scss | 2 +- .../common/form-kit/_fieldset.scss | 22 ++ .../stylesheets/common/form-kit/_index.scss | 3 +- .../stylesheets/mobile/admin_flags.scss | 10 - config/locales/client.en.yml | 1 + .../components/sections/atoms/05-forms.hbs | 22 +- spec/system/admin_flags_spec.rb | 203 +++++++------ spec/system/page_objects/modals/flag.rb | 4 + .../page_objects/pages/admin_flag_form.rb | 19 +- spec/system/page_objects/pages/admin_flags.rb | 57 ++++ 28 files changed, 648 insertions(+), 416 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/form-kit/components/fk/checkbox-group.gjs create mode 100644 app/assets/javascripts/discourse/app/form-kit/components/fk/fieldset.gjs rename app/assets/javascripts/discourse/app/form-kit/components/fk/{control => }/input-group.gjs (87%) create mode 100644 app/assets/javascripts/discourse/app/form-kit/components/fk/submit.gjs create mode 100644 app/assets/javascripts/discourse/tests/integration/components/form-kit/layout/checkbox-group-test.gjs create mode 100644 app/assets/javascripts/discourse/tests/integration/components/form-kit/layout/fieldset-test.gjs create mode 100644 app/assets/stylesheets/common/form-kit/_checkbox-group.scss delete mode 100644 app/assets/stylesheets/common/form-kit/_control-radio-group.scss create mode 100644 app/assets/stylesheets/common/form-kit/_fieldset.scss diff --git a/app/assets/javascripts/admin/addon/components/admin-flags-form.gjs b/app/assets/javascripts/admin/addon/components/admin-flags-form.gjs index ec15722b6bc..c4f9e6cff30 100644 --- a/app/assets/javascripts/admin/addon/components/admin-flags-form.gjs +++ b/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 { fn, hash } from "@ember/helper"; -import { TextArea } from "@ember/legacy-built-in-components"; -import { on } from "@ember/modifier"; +import { cached } from "@glimmer/tracking"; +import { hash } from "@ember/helper"; import { action } from "@ember/object"; import { LinkTo } from "@ember/routing"; import { service } from "@ember/service"; -import { isEmpty } from "@ember/utils"; -import { not } from "truth-helpers"; -import DButton from "discourse/components/d-button"; -import withEventValue from "discourse/helpers/with-event-value"; +import Form from "discourse/components/form"; 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() { - return ( - !isEmpty(this.name) && - !isEmpty(this.description) && - !isEmpty(this.appliesTo) - ); + @cached + get formData() { + if (this.isUpdate) { + return { + 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 - save() { - this.isUpdate ? this.update() : this.create(); - } - - @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); + validateAppliesTo(name, value, { addError }) { + if (value && value.length === 0) { + addError("appliesTo", { + title: i18n("admin.config_areas.flags.form.applies_to"), + message: i18n("admin.config_areas.flags.form.invalid_applies_to"), }); + } } - @bind - update() { - return ajax(`/admin/config/flags/${this.args.flag.id}`, { - type: "PUT", - data: this.#formData, - }) - .then((response) => { - 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) => { - return popupAjaxError(error); - }); - } - - @bind - get #formData() { - return { - name: this.name, - description: this.description, - applies_to: this.appliesTo, - require_message: this.requireMessage, - enabled: this.enabled, + @action + save({ name, description, appliesTo, requireMessage, enabled }) { + const createOrUpdate = this.isUpdate ? this.update : this.create; + const data = { + name, + description, + enabled, + applies_to: appliesTo, + require_message: requireMessage, }; + 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); + } }