FIX: Allow saving category when color attributes are 3-digits hex

app/assets/javascripts/admin/addon/components/color-input.gjs

@@ -32,16 +32,16 @@ export default class ColorInput extends Component {
   }
 
   @computed("hexValueWithFallback")
-  get normalizedValue() {
-    return this.normalize(this.hexValueWithFallback);
+  get valueForPicker() {
+    return this.normalize(this.hexValueWithFallback, { forPicker: true });
   }
 
-  normalize(color) {
+  normalize(color, { forPicker = false } = {}) {
     if (this._valid(color)) {
       if (!color.startsWith("#")) {
         color = "#" + color;
       }
-      if (color.length === 4) {
+      if (color.length === 4 && (!this.skipNormalize || forPicker)) {
         color =
           "#" +
           color
@@ -109,8 +109,8 @@ export default class ColorInput extends Component {
     <input
       class="picker"
       type="color"
-      value={{this.normalizedValue}}
-      title={{this.normalizedValue}}
+      value={{this.valueForPicker}}
+      title={{this.valueForPicker}}
       {{on "input" this.onPickerInput}}
       aria-labelledby={{this.ariaLabelledby}}
     />

app/assets/javascripts/discourse/app/components/edit-category-general.gjs

@@ -129,6 +129,10 @@ export default class EditCategoryGeneral extends Component {
   updateColor(field, newColor) {
     const color = newColor.replace("#", "");
 
+    if (color === field.value) {
+      return;
+    }
+
     if (field.name === "color") {
       const whiteDiff = this.colorDifference(color, CATEGORY_TEXT_COLORS[0]);
       const blackDiff = this.colorDifference(color, CATEGORY_TEXT_COLORS[1]);
@@ -160,6 +164,41 @@ export default class EditCategoryGeneral extends Component {
     return rDiff + gDiff + bDiff;
   }
 
+  @action
+  validateColor(name, color, { addError }) {
+    color = color.trim();
+
+    let title;
+    if (name === "color") {
+      title = i18n("category.background_color");
+    } else if (name === "text_color") {
+      title = i18n("category.foreground_color");
+    } else {
+      throw new Error(`unknown title for category attribute ${name}`);
+    }
+
+    if (!color) {
+      addError(name, {
+        title,
+        message: i18n("category.color_validations.cant_be_empty"),
+      });
+    }
+
+    if (color.length !== 3 && color.length !== 6) {
+      addError(name, {
+        title,
+        message: i18n("category.color_validations.incorrect_length"),
+      });
+    }
+
+    if (!/^[0-9A-Fa-f]+$/.test(color)) {
+      addError(name, {
+        title,
+        message: i18n("category.color_validations.non_hexdecimal"),
+      });
+    }
+  }
+
   get categoryDescription() {
     if (this.args.category.description) {
       return htmlSafe(this.args.category.description);
@@ -318,6 +357,8 @@ export default class EditCategoryGeneral extends Component {
           @name="color"
           @title={{i18n "category.background_color"}}
           @format="full"
+          @validate={{this.validateColor}}
+          @validation="required"
           as |field|
         >
           <field.Custom>
@@ -328,6 +369,7 @@ export default class EditCategoryGeneral extends Component {
                   @valid={{@category.colorValid}}
                   @ariaLabelledby="background-color-label"
                   @onChangeColor={{fn this.updateColor field}}
+                  @skipNormalize={{true}}
                 />
                 <ColorPicker
                   @colors={{this.backgroundColors}}
@@ -345,6 +387,8 @@ export default class EditCategoryGeneral extends Component {
           @name="text_color"
           @title={{i18n "category.foreground_color"}}
           @format="full"
+          @validate={{this.validateColor}}
+          @validation="required"
           as |field|
         >
           <field.Custom>
@@ -353,7 +397,8 @@ export default class EditCategoryGeneral extends Component {
                 <ColorInput
                   @hexValue={{readonly field.value}}
                   @ariaLabelledby="foreground-color-label"
-                  @onBlur={{fn this.updateColor field}}
+                  @onChangeColor={{fn this.updateColor field}}
+                  @skipNormalize={{true}}
                 />
                 <ColorPicker
                   @colors={{CATEGORY_TEXT_COLORS}}

app/assets/javascripts/discourse/app/controllers/edit-category/tabs.js

@@ -61,14 +61,6 @@ export default class EditCategoryTabsController extends Controller {
       return false;
     }
 
-    if (!transientData.color) {
-      return false;
-    }
-
-    if (transientData.text_color.length < 6) {
-      return false;
-    }
-
     if (this.saving || this.deleting) {
       return true;
     }
@@ -120,13 +112,12 @@ export default class EditCategoryTabsController extends Controller {
   }
 
   @action
-  saveCategory(transientData) {
+  saveCategory(data) {
     if (this.validators.some((validator) => validator())) {
       return;
     }
 
-    this.model.setProperties(transientData);
-
+    this.model.setProperties(data);
     this.set("saving", true);
 
     this.model

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

@@ -225,7 +225,7 @@ class FKForm extends Component {
     try {
       this.isSubmitting = true;
 
-      await this.validate(this.fields.values());
+      await this.validate([...this.fields.values()]);
 
       if (this.formData.isValid) {
         this.formData.save();

app/assets/javascripts/discourse/app/templates/edit-category/tabs.gjs

@@ -1,4 +1,3 @@
-import { fn } from "@ember/helper";
 import { getOwner } from "@ember/owner";
 import { htmlSafe } from "@ember/template";
 import RouteTemplate from "ember-route-template";
@@ -93,6 +92,7 @@ export default RouteTemplate(
       <Form
         @data={{@controller.formData}}
         @onDirtyCheck={{@controller.isLeavingForm}}
+        @onSubmit={{@controller.saveCategory}}
         as |form transientData|
       >
         <form.Section
@@ -104,7 +104,7 @@ export default RouteTemplate(
               <Tab
                 @selectedTab={{@controller.selectedTab}}
                 @category={{@controller.model}}
-                @action={{@controller.registerValidator}}
+                @registerValidator={{@controller.registerValidator}}
                 @transientData={{transientData}}
                 @form={{form}}
               />
@@ -119,12 +119,10 @@ export default RouteTemplate(
         {{/if}}
 
         <form.Actions class="edit-category-footer">
-          <form.Button
+          <form.Submit
             @disabled={{not (@controller.canSaveForm transientData)}}
-            @action={{fn @controller.saveCategory transientData}}
             @label={{@controller.saveLabel}}
             id="save-category"
-            class="btn-primary"
           />
 
           {{#if @controller.model.can_delete}}

app/models/category.rb

@@ -104,6 +104,8 @@ class Category < ActiveRecord::Base
             },
             allow_nil: true
   validates :slug, exclusion: { in: RESERVED_SLUGS }
+  validates :color, format: { with: /\A(\h{6}|\h{3})\z/ }
+  validates :text_color, format: { with: /\A(\h{6}|\h{3})\z/ }
 
   after_create :create_category_definition
   after_destroy :trash_category_definition

config/locales/client.en.yml

@@ -4459,6 +4459,10 @@ en:
       style: "Styles"
       background_color: "Color"
       foreground_color: "Text color"
+      color_validations:
+        cant_be_empty: "can't be empty"
+        incorrect_length: "must match the format #RRGGBB or #RGB"
+        non_hexdecimal: "must only contain hexadecimal characters (0-9, A-F)"
       styles:
         type: "Style"
         icon: "Icon"

spec/models/category_spec.rb

@@ -38,6 +38,34 @@
     expect(cats.errors[:name]).to be_present
   end
 
+  it "validates format of color" do
+    expect(Fabricate.build(:category, color: "fff", user:)).to be_valid
+    expect(Fabricate.build(:category, color: "1ac", user:)).to be_valid
+    expect(Fabricate.build(:category, color: "fffeee", user:)).to be_valid
+    expect(Fabricate.build(:category, color: "FF11CC", user:)).to be_valid
+    expect(Fabricate.build(:category, color: "ABC", user:)).to be_valid
+
+    expect(Fabricate.build(:category, color: "ffq1e1", user:)).to_not be_valid
+    expect(Fabricate.build(:category, color: "", user:)).to_not be_valid
+    expect(Fabricate.build(:category, color: "f", user:)).to_not be_valid
+    expect(Fabricate.build(:category, color: "21", user:)).to_not be_valid
+    expect(Fabricate.build(:category, color: "XCA", user:)).to_not be_valid
+  end
+
+  it "validates format of text_color" do
+    expect(Fabricate.build(:category, text_color: "fff", user:)).to be_valid
+    expect(Fabricate.build(:category, text_color: "1ac", user:)).to be_valid
+    expect(Fabricate.build(:category, text_color: "fffeee", user:)).to be_valid
+    expect(Fabricate.build(:category, text_color: "FF11CC", user:)).to be_valid
+    expect(Fabricate.build(:category, text_color: "ABC", user:)).to be_valid
+
+    expect(Fabricate.build(:category, text_color: "ffq1e1", user:)).to_not be_valid
+    expect(Fabricate.build(:category, text_color: "", user:)).to_not be_valid
+    expect(Fabricate.build(:category, text_color: "f", user:)).to_not be_valid
+    expect(Fabricate.build(:category, text_color: "21", user:)).to_not be_valid
+    expect(Fabricate.build(:category, text_color: "XCA", user:)).to_not be_valid
+  end
+
   describe "Associations" do
     it { is_expected.to have_one(:category_setting).dependent(:destroy) }
 

spec/system/edit_category_general_spec.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+describe "Edit Category General", type: :system do
+  fab!(:admin)
+  fab!(:category)
+  let(:category_page) { PageObjects::Pages::Category.new }
+  let(:form) { PageObjects::Components::FormKit.new(".form-kit") }
+  before { sign_in(admin) }
+
+  context "when changing background color" do
+    it "displays an error when the hex code is invalid" do
+      category_page.visit_general(category)
+
+      form.field("color").component.find("input.hex-input").fill_in(with: "ABZ")
+      category_page.save_settings
+      expect(form.field("color")).to have_errors(
+        I18n.t("js.category.color_validations.non_hexdecimal"),
+      )
+
+      form.field("color").component.find("input.hex-input").fill_in(with: "")
+      category_page.save_settings
+      expect(form.field("color")).to have_errors(
+        I18n.t("js.category.color_validations.cant_be_empty"),
+      )
+
+      form.field("color").component.find("input.hex-input").fill_in(with: "A")
+      category_page.save_settings
+      expect(form.field("color")).to have_errors(
+        I18n.t("js.category.color_validations.incorrect_length"),
+      )
+    end
+
+    it "saves successfully when the hex code is valid" do
+      category_page.visit_general(category)
+
+      form.field("color").component.find("input.hex-input").fill_in(with: "AB1")
+      category_page.save_settings
+      expect(form.field("color")).to have_no_errors
+    end
+  end
+
+  context "when changing text color" do
+    it "displays an error when the hex code is invalid" do
+      category_page.visit_general(category)
+
+      form.field("text_color").component.find("input.hex-input").fill_in(with: "ABZ")
+      category_page.save_settings
+      expect(form.field("text_color")).to have_errors(
+        I18n.t("js.category.color_validations.non_hexdecimal"),
+      )
+
+      form.field("text_color").component.find("input.hex-input").fill_in(with: "")
+      category_page.save_settings
+      expect(form.field("text_color")).to have_errors(
+        I18n.t("js.category.color_validations.cant_be_empty"),
+      )
+
+      form.field("text_color").component.find("input.hex-input").fill_in(with: "A")
+      category_page.save_settings
+      expect(form.field("text_color")).to have_errors(
+        I18n.t("js.category.color_validations.incorrect_length"),
+      )
+    end
+
+    it "saves successfully when the hex code is valid" do
+      category_page.visit_general(category)
+
+      form.field("text_color").component.find("input.hex-input").fill_in(with: "AB1")
+      category_page.save_settings
+      expect(form.field("text_color")).to have_no_errors
+    end
+  end
+end

spec/system/page_objects/components/form_kit.rb

@@ -80,7 +80,7 @@ def has_errors?(*messages)
       end
 
       def has_no_errors?
-        !has_css?(".form-kit__errors")
+        has_no_css?(".form-kit__errors")
       end
 
       def control_type

spec/system/page_objects/pages/category.rb

@@ -15,6 +15,11 @@ def visit_settings(category)
         self
       end
 
+      def visit_general(category)
+        page.visit("/c/#{category.slug}/edit/general")
+        self
+      end
+
       def visit_edit_template(category)
         page.visit("/c/#{category.slug}/edit/topic-template")
         self