From adca6ecc3417cb31b97a4f6bc5fe51e3b2e61997 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Tue, 19 Sep 2023 10:16:02 -0700 Subject: [PATCH] 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. --- .../src/components/option/option.e2e.ts | 14 ++++++++ .../src/components/option/option.tsx | 11 ++++-- .../src/components/select/select.e2e.ts | 34 +++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/calcite-components/src/components/option/option.e2e.ts b/packages/calcite-components/src/components/option/option.e2e.ts index 8496020aac6..3494242870d 100644 --- 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 index e3375bb62ef..b5e5b5a91ac 100644 --- 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 index 61a8b80161a..ea746eed39f 100644 --- 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` + + uno + dos + + ` + ); + + 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`