fix(select): allow setting an option value to an empty string (#7769)

**Related Issue:** #4032 ## Summary This fixes an issue where an empty string value was treated as value not set and would therefore fall back to the label as the value.
Esri · Sep 19, 2023 · adca6ec · adca6ec
1 parent b9bd0de
commit adca6ec
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 2 deletions.
diff --git a/packages/calcite-components/src/components/option/option.e2e.ts b/packages/calcite-components/src/components/option/option.e2e.ts
@@ -60,6 +60,20 @@ describe("calcite-option", () => {
     expect(await option.getProperty("label")).toBe(optionText);
     expect(await option.getProperty("value")).toBe(optionText);
 
+    option.setProperty("label", "");
+    option.setProperty("value", "");
+    await page.waitForChanges();
+
+    expect(await option.getProperty("label")).toBe(optionText);
+    expect(await option.getProperty("value")).toBe("");
+
+    option.setProperty("label", null);
+    option.setProperty("value", null);
+    await page.waitForChanges();
+
+    expect(await option.getProperty("label")).toBe(optionText);
+    expect(await option.getProperty("value")).toBe(optionText);
+
     const alternateLabel = "dos";
     await option.setProperty("innerText", alternateLabel);
     await page.waitForChanges();

diff --git a/packages/calcite-components/src/components/option/option.tsx b/packages/calcite-components/src/components/option/option.tsx
@@ -92,14 +92,21 @@ export class Option {
   private ensureTextContentDependentProps(): void {
     const {
       el: { textContent },
+      internallySetLabel,
+      internallySetValue,
+      label,
+      value,
     } = this;
 
-    if (!this.label || this.label === this.internallySetLabel) {
+    if (!label || label === internallySetLabel) {
       this.label = textContent;
       this.internallySetLabel = textContent;
     }
 
-    if (!this.value || this.value === this.internallySetValue) {
+    if (
+      value == null /* intentional loose equals to handle both undefined & null */ ||
+      value === internallySetValue
+    ) {
       this.value = textContent;
       this.internallySetValue = textContent;
     }

diff --git a/packages/calcite-components/src/components/select/select.e2e.ts b/packages/calcite-components/src/components/select/select.e2e.ts
@@ -352,6 +352,40 @@ describe("calcite-select", () => {
     expect(selectedOptionId).toBe("2");
   });
 
+  it("honors empty value", async () => {
+    const page = await newE2EPage();
+    await page.setContent(
+      html`
+        <calcite-select id="calcite-select">
+          <calcite-option id="1" value="uno">uno</calcite-option>
+          <calcite-option id="2" value="">dos</calcite-option>
+        </calcite-select>
+      `
+    );
+
+    type TestWindow = typeof window & { selectedOptionId: string };
+
+    await page.$eval("calcite-select", (select: HTMLCalciteSelectElement) =>
+      select.addEventListener("calciteSelectChange", (event) => {
+        (window as TestWindow).selectedOptionId = (event.target as HTMLElement).querySelector(
+          "calcite-option[selected]"
+        ).id;
+      })
+    );
+
+    const internalSelect = await page.evaluateHandle(() =>
+      document.querySelector("calcite-select").shadowRoot.querySelector("select")
+    );
+
+    await internalSelect.asElement().select("");
+    await page.waitForChanges();
+
+    const selectedOptionId = await page.evaluate(() => (window as TestWindow).selectedOptionId);
+
+    expect(selectedOptionId).toBe("2");
+    expect(await (await page.find("calcite-select")).getProperty("value")).toBe("");
+  });
+
   describe("is form-associated", () => {
     formAssociated(
       html`