DEV: Don't include blank option for required selects (discourse#30316)

When using FK select, we include a "None" option automatically. However, for required select fields, "None" isn't a valid option, so we exclude it instead.
jordanvidrine · Dec 17, 2024 · e04f535 · e04f535
1 parent f9e07ff
commit e04f535
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 9 deletions.
diff --git a/app/assets/javascripts/discourse/app/components/d-select.gjs b/app/assets/javascripts/discourse/app/components/d-select.gjs
@@ -46,20 +46,26 @@ export default class DSelect extends Component {
     return this.args.value && this.args.value !== NO_VALUE_OPTION;
   }
 
+  get includeNone() {
+    return this.args.includeNone ?? true;
+  }
+
   <template>
     <select
       value={{@value}}
       ...attributes
       class="d-select"
       {{on "input" this.handleInput}}
     >
-      <DSelectOption @value={{NO_VALUE_OPTION}}>
-        {{#if this.hasSelectedValue}}
-          {{i18n "none_placeholder"}}
-        {{else}}
-          {{i18n "select_placeholder"}}
-        {{/if}}
-      </DSelectOption>
+      {{#if this.includeNone}}
+        <DSelectOption @value={{NO_VALUE_OPTION}}>
+          {{#if this.hasSelectedValue}}
+            {{i18n "none_placeholder"}}
+          {{else}}
+            {{i18n "select_placeholder"}}
+          {{/if}}
+        </DSelectOption>
+      {{/if}}
 
       {{yield (hash Option=(component DSelectOption selected=@value))}}
     </select>

diff --git a/app/assets/javascripts/discourse/app/form-kit/components/fk/control/select.gjs b/app/assets/javascripts/discourse/app/form-kit/components/fk/control/select.gjs
@@ -15,12 +15,17 @@ const SelectOption = <template>
 export default class FKControlSelect extends Component {
   static controlType = "select";
 
+  get includeNone() {
+    return this.args.field.validation !== "required";
+  }
+
   <template>
     <DSelect
       class="form-kit__control-select"
       disabled={{@field.disabled}}
       @value={{@field.value}}
       @onChange={{@field.set}}
+      @includeNone={{this.includeNone}}
       ...attributes
     >
       {{yield (hash Option=(component SelectOption selected=@field.value))}}

diff --git a/app/assets/javascripts/discourse/tests/integration/components/create-invite-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/create-invite-test.gjs
@@ -166,7 +166,7 @@ module("Integration | Component | CreateInvite", function (hooks) {
 
     assert.deepEqual(
       formKit().field("expiresAfterDays").options(),
-      ["__NONE__", "1", "3", "7", "30", "90", "999999"],
+      ["1", "3", "7", "30", "90", "999999"],
       "the value of invite_expiry_days is added to the dropdown"
     );
 
@@ -179,7 +179,7 @@ module("Integration | Component | CreateInvite", function (hooks) {
 
     assert.deepEqual(
       formKit().field("expiresAfterDays").options(),
-      ["__NONE__", "1", "7", "30", "90", "999999"],
+      ["1", "7", "30", "90", "999999"],
       "the value of invite_expiry_days is not added to the dropdown if it's already one of the options"
     );
   });

diff --git a/app/assets/javascripts/discourse/tests/integration/components/d-select-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/d-select-test.gjs
@@ -50,6 +50,16 @@ module("Integration | Component | d-select", function (hooks) {
     });
   });
 
+  test("required field", async function (assert) {
+    await render(<template>
+      <DSelect @includeNone={{false}} as |s|>
+        <s.Option @value="foo">The real foo</s.Option>
+      </DSelect>
+    </template>);
+
+    assert.dselect().hasNoOption(NO_VALUE_OPTION);
+  });
+
   test("select attributes", async function (assert) {
     await render(<template><DSelect class="test" /></template>);