From b8f778a1f0cd95136ab9c6d7579dd956e859da4f Mon Sep 17 00:00:00 2001 From: Ben Elan Date: Fri, 2 Feb 2024 14:23:05 -0800 Subject: [PATCH 1/5] fix: don't override existing validationMessage when displaying after form submission --- .../src/components/combobox/combobox.e2e.ts | 2 +- .../input-date-picker.e2e.ts | 2 +- .../input-number/input-number.e2e.ts | 1 + .../components/input-text/input-text.e2e.ts | 2 +- .../input-time-picker.e2e.ts | 2 +- .../src/components/input/input.e2e.ts | 1 + .../src/components/select/select.e2e.ts | 2 +- .../src/components/text-area/text-area.e2e.ts | 1 + .../src/tests/commonTests.ts | 121 ++++++- .../calcite-components/src/utils/form.e2e.ts | 301 ------------------ .../calcite-components/src/utils/form.tsx | 52 ++- 11 files changed, 164 insertions(+), 323 deletions(-) delete mode 100644 packages/calcite-components/src/utils/form.e2e.ts diff --git a/packages/calcite-components/src/components/combobox/combobox.e2e.ts b/packages/calcite-components/src/components/combobox/combobox.e2e.ts index 2ecfb7b404c..dba3ad7548c 100644 --- a/packages/calcite-components/src/components/combobox/combobox.e2e.ts +++ b/packages/calcite-components/src/components/combobox/combobox.e2e.ts @@ -1609,7 +1609,7 @@ describe("calcite-combobox", () => { `, - { testValue: "two", submitsOnEnter: true }, + { testValue: "two", submitsOnEnter: true, validation: true }, ); }); diff --git a/packages/calcite-components/src/components/input-date-picker/input-date-picker.e2e.ts b/packages/calcite-components/src/components/input-date-picker/input-date-picker.e2e.ts index e50c14aeb54..f9a73d5dbd8 100644 --- a/packages/calcite-components/src/components/input-date-picker/input-date-picker.e2e.ts +++ b/packages/calcite-components/src/components/input-date-picker/input-date-picker.e2e.ts @@ -715,7 +715,7 @@ describe("calcite-input-date-picker", () => { describe("is form-associated", () => { describe("supports single value", () => { - formAssociated("calcite-input-date-picker", { testValue: "1985-03-23", submitsOnEnter: true }); + formAssociated("calcite-input-date-picker", { testValue: "1985-03-23", submitsOnEnter: true, validation: true }); }); describe("supports range", () => { diff --git a/packages/calcite-components/src/components/input-number/input-number.e2e.ts b/packages/calcite-components/src/components/input-number/input-number.e2e.ts index 9d7d5492786..c4cdde852f7 100644 --- a/packages/calcite-components/src/components/input-number/input-number.e2e.ts +++ b/packages/calcite-components/src/components/input-number/input-number.e2e.ts @@ -1749,6 +1749,7 @@ describe("calcite-input-number", () => { testValue: 5, submitsOnEnter: true, inputType: "number", + validation: true, }); testPostValidationFocusing("calcite-input-number"); diff --git a/packages/calcite-components/src/components/input-text/input-text.e2e.ts b/packages/calcite-components/src/components/input-text/input-text.e2e.ts index 0840551a35f..0211ca26fb4 100644 --- a/packages/calcite-components/src/components/input-text/input-text.e2e.ts +++ b/packages/calcite-components/src/components/input-text/input-text.e2e.ts @@ -462,7 +462,7 @@ describe("calcite-input-text", () => { }); describe("is form-associated", () => { - formAssociated("calcite-input-text", { testValue: "test", submitsOnEnter: true }); + formAssociated("calcite-input-text", { testValue: "test", submitsOnEnter: true, validation: true }); testPostValidationFocusing("calcite-input-text"); }); diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts index e2d3398c050..77c44a55c7c 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts @@ -593,7 +593,7 @@ describe("calcite-input-time-picker", () => { }); describe("is form-associated", () => { - formAssociated("calcite-input-time-picker", { testValue: "03:23", submitsOnEnter: true }); + formAssociated("calcite-input-time-picker", { testValue: "03:23", submitsOnEnter: true, validation: true }); }); it("updates value appropriately as step changes", async () => { diff --git a/packages/calcite-components/src/components/input/input.e2e.ts b/packages/calcite-components/src/components/input/input.e2e.ts index 9f02f810ff4..22ab160dcd2 100644 --- a/packages/calcite-components/src/components/input/input.e2e.ts +++ b/packages/calcite-components/src/components/input/input.e2e.ts @@ -2053,6 +2053,7 @@ describe("calcite-input", () => { testValue: value, submitsOnEnter: true, inputType: type, + validation: true, }); } diff --git a/packages/calcite-components/src/components/select/select.e2e.ts b/packages/calcite-components/src/components/select/select.e2e.ts index 8deac1487ed..7d0223692f1 100644 --- a/packages/calcite-components/src/components/select/select.e2e.ts +++ b/packages/calcite-components/src/components/select/select.e2e.ts @@ -411,7 +411,7 @@ describe("calcite-select", () => { tres `, - { testValue: "dos" }, + { testValue: "dos", validation: true }, ); }); }); diff --git a/packages/calcite-components/src/components/text-area/text-area.e2e.ts b/packages/calcite-components/src/components/text-area/text-area.e2e.ts index 255cf87f099..a9be384c767 100644 --- a/packages/calcite-components/src/components/text-area/text-area.e2e.ts +++ b/packages/calcite-components/src/components/text-area/text-area.e2e.ts @@ -99,6 +99,7 @@ describe("calcite-text-area", () => { testValue: "zion national park", expectedSubmitValue: "zion national park", submitsOnEnter: false, + validation: true, }); }); diff --git a/packages/calcite-components/src/tests/commonTests.ts b/packages/calcite-components/src/tests/commonTests.ts index 8ccbc4027be..e4178a94008 100644 --- a/packages/calcite-components/src/tests/commonTests.ts +++ b/packages/calcite-components/src/tests/commonTests.ts @@ -6,7 +6,7 @@ import { toHaveNoViolations } from "jest-axe"; import { config } from "../../stencil.config"; import { html } from "../../support/formatting"; import type { JSX } from "../components"; -import { hiddenFormInputSlotName } from "../utils/form"; +import { getClearValidationEventName, hiddenFormInputSlotName } from "../utils/form"; import { MessageBundle } from "../utils/t9n"; import { GlobalTestProps, @@ -703,6 +703,11 @@ interface FormAssociatedOptions { * Specifies if the component supports clearing its value (i.e., setting to null). */ clearable?: boolean; + + /** + * Specifies whether the component supports preventing submission and displaying validation messages + */ + validation?: boolean; } /** @@ -720,6 +725,10 @@ export function formAssociated( it("supports association via ancestry", () => testAncestorFormAssociated()); it("supports association via form ID", () => testIdFormAssociated()); + if (options.validation) { + it("supports required property validation", () => testRequiredPropertyValidation()); + } + async function testAncestorFormAssociated(): Promise { const { beforeContent, tagOrHTML } = getTagOrHTMLWithBeforeContent(componentTagOrHtml); const tag = getTag(tagOrHTML); @@ -777,14 +786,48 @@ export function formAssociated( } } - function ensureForm(html: string, componentTag: string): string { - return html.includes("form=") ? html : html.replace(componentTag, `${componentTag} form="test-form" `); + async function testRequiredPropertyValidation(): Promise { + const requiredValidationMessage = "Please fill out this field."; + const { beforeContent, tagOrHTML } = getTagOrHTMLWithBeforeContent(componentTagOrHtml); + const tag = getTag(tagOrHTML); + const componentHtml = ensureRequired(ensureName(isHTML(tagOrHTML) ? tagOrHTML : `<${tag}>`, tag), tag); + + const page = await newE2EPage(); + await beforeContent?.(page); + + const content = html`
+ ${componentHtml} + + +
`; + + await page.setContent(content); + await page.waitForChanges(); + const component = await page.find(tag); + + const submitButton = await page.find("input"); + const spyEvent = await page.spyOnEvent(getClearValidationEventName(tag)); + + await assertPreventsFormSubmission(page, component, options, submitButton, requiredValidationMessage); + await assertClearsValidationOnValueChange(page, component, options, spyEvent, tag); + await assertUserMessageNotOverridden(page, component, options, submitButton); } function ensureName(html: string, componentTag: string): string { return html.includes("name=") ? html : html.replace(componentTag, `${componentTag} name="testName" `); } + function ensureRequired(html: string, componentTag: string): string { + return html.includes("required") ? html : html.replace(componentTag, `${componentTag} required `); + } + + function ensureForm(html: string, componentTag: string): string { + return html.includes("form=") ? html : html.replace(componentTag, `${componentTag} form="test-form" `); + } + async function isCheckable(page: E2EPage, component: E2EElement, options: FormAssociatedOptions): Promise { return ( typeof options.testValue === "boolean" && @@ -983,6 +1026,78 @@ export function formAssociated( expect(called).toBe(true); } + + async function assertPreventsFormSubmission( + page: E2EPage, + component: E2EElement, + options: FormAssociatedOptions, + submitButton: E2EElement, + message: string, + ) { + await (options.submitsOnEnter ? page.keyboard.press("Enter") : submitButton.click()); + await page.waitForChanges(); + + await expectValidationInvalid(component, message); + } + + async function assertClearsValidationOnValueChange( + page: E2EPage, + component: E2EElement, + options: FormAssociatedOptions, + event: EventSpy, + tag: string, + ) { + await component.callMethod("setFocus"); + await page.waitForChanges(); + + if (tag === "calcite-combobox") { + await page.keyboard.press("Space"); + await page.keyboard.press("Enter"); + } else if (tag === "calcite-select") { + await page.keyboard.press("ArrowDown"); + } else { + await page.keyboard.type(options.testValue); + await page.keyboard.press("Tab"); + } + + await page.waitForChanges(); + expect(event).toHaveReceivedEventTimes(1); + + await expectValidationIdle(component); + } + + async function assertUserMessageNotOverridden( + page: E2EPage, + component: E2EElement, + options: FormAssociatedOptions, + submitButton: E2EElement, + ) { + const customValidationMessage = "This is a custom message."; + const customValidationIcon = "banana"; + + // don't override custom validation message and icon + component.setProperty("validationMessage", customValidationMessage); + component.setProperty("validationIcon", customValidationIcon); + component.setProperty("value", undefined); + await page.waitForChanges(); + + await (options.submitsOnEnter ? page.keyboard.press("Enter") : submitButton.click()); + await page.waitForChanges(); + + await expectValidationInvalid(component, customValidationMessage, customValidationIcon); + } + + async function expectValidationIdle(element: E2EElement) { + expect(await element.getProperty("status")).toBe("idle"); + expect(await element.getProperty("validationMessage")).toBe(""); + expect(await element.getProperty("validationIcon")).toBe(false); + } + + async function expectValidationInvalid(element: E2EElement, message: string, icon: string | boolean = true) { + expect(await element.getProperty("status")).toBe("invalid"); + expect(await element.getProperty("validationMessage")).toBe(message); + expect(element.getAttribute("validation-icon")).toBe(icon); + } } interface TabAndClickTargets { diff --git a/packages/calcite-components/src/utils/form.e2e.ts b/packages/calcite-components/src/utils/form.e2e.ts deleted file mode 100644 index 96fe0980d2b..00000000000 --- a/packages/calcite-components/src/utils/form.e2e.ts +++ /dev/null @@ -1,301 +0,0 @@ -import { E2EElement, newE2EPage } from "@stencil/core/testing"; -import { html } from "../../support/formatting"; -import { componentsWithInputEvent } from "./form"; - -async function assertValidationIdle(element: E2EElement) { - expect(await element.getProperty("status")).toBe("idle"); - expect(await element.getProperty("validationMessage")).toBe(""); - expect(await element.getProperty("validationIcon")).toBe(false); -} - -async function assertValidationInvalid(element: E2EElement, message: string) { - expect(await element.getProperty("status")).toBe("invalid"); - expect(await element.getProperty("validationMessage")).toBe(message); - expect(element.getAttribute("validation-icon")).toBe(""); -} - -describe("form", () => { - describe("constraint validation", () => { - describe("required property", () => { - const requiredValidationMessage = "Please fill out this field."; - - const getInputEventName = (component: string): string => - component - .split("-") - .map((part: string, index: number) => (index === 0 ? part : `${part[0].toUpperCase()}${part.slice(1)}`)) - .join("") - .concat("Input"); - - for (const component of ["calcite-input", "calcite-input-number", "calcite-input-text"]) { - it(`${component} - enter to submit`, async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- <${component} required name="${component}"> -
- `); - - const element = await page.find(component); - - const clearValidationEventName = getInputEventName(component); - const inputEvent = await page.spyOnEvent(clearValidationEventName); - - await element.callMethod("setFocus"); - await page.waitForChanges(); - - await page.keyboard.press("Enter"); - await page.waitForChanges(); - - await assertValidationInvalid(element, requiredValidationMessage); - - await page.keyboard.press("1"); - await page.waitForChanges(); - - expect(inputEvent).toHaveReceivedEventTimes(1); - expect(await element.getProperty("value")).toBe("1"); - - await assertValidationIdle(element); - }); - } - - for (const component of componentsWithInputEvent) { - it(`${component}`, async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- <${component} required name="${component}"> - Submit -
- `); - - const submitButton = await page.find("calcite-button"); - const element = await page.find(component); - - const clearValidationEventName = getInputEventName(component); - const inputEvent = await page.spyOnEvent(clearValidationEventName); - - await submitButton.click(); - await page.waitForChanges(); - - await assertValidationInvalid(element, requiredValidationMessage); - - await element.callMethod("setFocus"); - await page.waitForChanges(); - - await page.keyboard.press("1"); - await page.waitForChanges(); - - expect(inputEvent).toHaveReceivedEventTimes(1); - expect(await element.getProperty("value")).toBe("1"); - - await assertValidationIdle(element); - }); - } - - it(`calcite-input-date-picker`, async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - Submit -
- `); - - const submitButton = await page.find("calcite-button"); - const element = await page.find("calcite-input-date-picker"); - const changeEvent = await page.spyOnEvent("calciteInputDatePickerChange"); - - await submitButton.click(); - await page.waitForChanges(); - - await assertValidationInvalid(element, requiredValidationMessage); - - await element.callMethod("setFocus"); - await page.waitForChanges(); - - await page.keyboard.type("12/12/2012"); - await page.keyboard.press("Tab"); - await page.waitForChanges(); - - expect(changeEvent).toHaveReceivedEventTimes(1); - - await assertValidationIdle(element); - }); - - it(`calcite-input-time-picker`, async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - Submit -
- `); - - const submitButton = await page.find("calcite-button"); - const element = await page.find("calcite-input-time-picker"); - const changeEvent = await page.spyOnEvent("calciteInputTimePickerChange"); - - await submitButton.click(); - await page.waitForChanges(); - - await assertValidationInvalid(element, requiredValidationMessage); - - await element.callMethod("setFocus"); - await page.waitForChanges(); - - await page.keyboard.type("12:00 PM"); - await page.keyboard.press("Tab"); - await page.waitForChanges(); - - expect(changeEvent).toHaveReceivedEventTimes(1); - expect(await element.getProperty("value")).toBe("12:00"); - - await assertValidationIdle(element); - }); - - it(`calcite-select`, async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - - uno - dos - tres - - Submit -
- `); - - const submitButton = await page.find("calcite-button"); - const element = await page.find("calcite-select"); - const changeEvent = await page.spyOnEvent("calciteSelectChange"); - - await submitButton.click(); - await page.waitForChanges(); - - await assertValidationInvalid(element, requiredValidationMessage); - - await element.callMethod("setFocus"); - await page.waitForChanges(); - - await page.keyboard.press("ArrowDown"); - await page.waitForChanges(); - - expect(changeEvent).toHaveReceivedEventTimes(1); - expect(await element.getProperty("value")).toBe("uno"); - - await assertValidationIdle(element); - }); - - it(`calcite-combobox`, async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - - - Submit -
- `); - - const submitButton = await page.find("calcite-button"); - const element = await page.find("calcite-combobox"); - const changeEvent = await page.spyOnEvent("calciteComboboxChange"); - - await submitButton.click(); - await page.waitForChanges(); - - await assertValidationInvalid(element, requiredValidationMessage); - - await element.callMethod("setFocus"); - await page.waitForChanges(); - - await page.keyboard.press("Space"); - await page.keyboard.press("Enter"); - await page.waitForChanges(); - - expect(changeEvent).toHaveReceivedEventTimes(1); - expect(await element.getProperty("value")).toBe("Pine"); - await assertValidationIdle(element); - }); - - it.skip(`calcite-radio-button-group`, async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - - 1 - - - - 2 - - - - 3 - - - - Submit -
- `); - - const submitButton = await page.find("calcite-button"); - const element = await page.find("calcite-radio-button-group"); - const changeEvent = await page.spyOnEvent("calciteRadioButtonGroupChange"); - - await submitButton.click(); - await page.waitForChanges(); - - await assertValidationInvalid(element, requiredValidationMessage); - - await element.callMethod("setFocus"); - await page.waitForChanges(); - - await page.keyboard.press("Space"); - await page.waitForChanges(); - - expect(changeEvent).toHaveReceivedEventTimes(1); - expect(await element.getProperty("value")).toBe("1"); - - await assertValidationIdle(element); - }); - - it.skip(`calcite-segmented-control`, async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - 1 - 2 - 3 - - Submit -
- `); - - const submitButton = await page.find("calcite-button"); - const element = await page.find("calcite-segmented-control"); - const changeEvent = await page.spyOnEvent("calciteSegmentedControlChange"); - - await submitButton.click(); - await page.waitForChanges(); - - await assertValidationInvalid(element, requiredValidationMessage); - - await element.callMethod("setFocus"); - await page.waitForChanges(); - - await page.keyboard.press("Space"); - await page.waitForChanges(); - - expect(changeEvent).toHaveReceivedEventTimes(1); - expect(await element.getProperty("value")).toBe("1"); - - await assertValidationIdle(element); - }); - }); - }); -}); diff --git a/packages/calcite-components/src/utils/form.tsx b/packages/calcite-components/src/utils/form.tsx index 1476bf9dd29..89db053f323 100644 --- a/packages/calcite-components/src/utils/form.tsx +++ b/packages/calcite-components/src/utils/form.tsx @@ -3,15 +3,40 @@ import { FunctionalComponent, h } from "@stencil/core"; /** * Any form with a `calciteInput` event needs to be included in this array. - * Exported for testing purposes. */ -export const componentsWithInputEvent = [ +const componentsWithInputEvent = [ "calcite-input", "calcite-input-number", "calcite-input-text", "calcite-text-area", ]; +/** + * Get the event name to listen for that, when emitted, will clear the + * validation message that displays after form submission. Only validation + * messages that are set by the browser will be cleared. If a user sets + * validationMessage to a custom value, they are responsible for clearing it. + * + * Exported for testing purposes. + * + * @param componentTag the tag of the component, e.g. "calcite-input" + * @returns the event name + */ +export function getClearValidationEventName(componentTag: string): string { + const componentTagCamelCase = componentTag + .split("-") + .map((part: string, index: number) => + index === 0 ? part : `${part[0].toUpperCase()}${part.slice(1)}`, + ) + .join(""); + + const clearValidationEvent = `${componentTagCamelCase}${ + componentsWithInputEvent.includes(componentTag) ? "Input" : "Change" + }`; + + return clearValidationEvent; +} + /** * Exported for testing purposes. */ @@ -180,8 +205,12 @@ function setInvalidFormValidation( message: string, ): void { "status" in component && (component.status = "invalid"); - "validationIcon" in component && (component.validationIcon = true); - "validationMessage" in component && (component.validationMessage = message); + + "validationIcon" in component && !component.validationIcon && (component.validationIcon = true); + + "validationMessage" in component && + !component.validationMessage && + (component.validationMessage = message); } function displayValidationMessage(event: Event) { @@ -201,18 +230,13 @@ function displayValidationMessage(event: Event) { // prevent the browser from showing the native validation popover event?.preventDefault(); - setInvalidFormValidation(formComponent, hiddenInput?.validationMessage || ""); - - const componentTagCamelCase = componentTagParts - .map((part: string, index: number) => - index === 0 ? part : `${part[0].toUpperCase()}${part.slice(1)}`, - ) - .join(""); + setInvalidFormValidation(formComponent, hiddenInput?.validationMessage); - const clearValidationEvent = `${componentTagCamelCase}${ - componentsWithInputEvent.includes(componentTag) ? "Input" : "Change" - }`; + if (formComponent?.validationMessage !== hiddenInput?.validationMessage) { + return; + } + const clearValidationEvent = getClearValidationEventName(componentTag); formComponent.addEventListener(clearValidationEvent, () => clearFormValidation(formComponent), { once: true, }); From 7e1b5573a3894f3d072337adee2ac72cf2675da6 Mon Sep 17 00:00:00 2001 From: Ben Elan Date: Fri, 2 Feb 2024 15:15:34 -0800 Subject: [PATCH 2/5] test: fixes --- .../input-number/input-number.e2e.ts | 2 +- .../src/components/select/select.e2e.ts | 1 + .../src/tests/commonTests.ts | 66 +++++++++++-------- .../calcite-components/src/utils/form.tsx | 2 +- 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/packages/calcite-components/src/components/input-number/input-number.e2e.ts b/packages/calcite-components/src/components/input-number/input-number.e2e.ts index c4cdde852f7..70d7c939e21 100644 --- a/packages/calcite-components/src/components/input-number/input-number.e2e.ts +++ b/packages/calcite-components/src/components/input-number/input-number.e2e.ts @@ -1746,7 +1746,7 @@ describe("calcite-input-number", () => { describe("is form-associated", () => { formAssociated("calcite-input-number", { - testValue: 5, + testValue: "5", submitsOnEnter: true, inputType: "number", validation: true, diff --git a/packages/calcite-components/src/components/select/select.e2e.ts b/packages/calcite-components/src/components/select/select.e2e.ts index 7d0223692f1..67be28c8c42 100644 --- a/packages/calcite-components/src/components/select/select.e2e.ts +++ b/packages/calcite-components/src/components/select/select.e2e.ts @@ -406,6 +406,7 @@ describe("calcite-select", () => { formAssociated( html` + uno dos tres diff --git a/packages/calcite-components/src/tests/commonTests.ts b/packages/calcite-components/src/tests/commonTests.ts index e4178a94008..b61c6c7b043 100644 --- a/packages/calcite-components/src/tests/commonTests.ts +++ b/packages/calcite-components/src/tests/commonTests.ts @@ -6,7 +6,7 @@ import { toHaveNoViolations } from "jest-axe"; import { config } from "../../stencil.config"; import { html } from "../../support/formatting"; import type { JSX } from "../components"; -import { getClearValidationEventName, hiddenFormInputSlotName } from "../utils/form"; +import { getClearValidationEventName, hiddenFormInputSlotName, componentsWithInputEvent } from "../utils/form"; import { MessageBundle } from "../utils/t9n"; import { GlobalTestProps, @@ -722,11 +722,13 @@ export function formAssociated( componentTagOrHtml: TagOrHTML | TagOrHTMLWithBeforeContent, options: FormAssociatedOptions, ): void { - it("supports association via ancestry", () => testAncestorFormAssociated()); - it("supports association via form ID", () => testIdFormAssociated()); + const inputTypeContext = options.inputType ? `(input type="${options.inputType}")` : ""; - if (options.validation) { - it("supports required property validation", () => testRequiredPropertyValidation()); + it(`supports association via ancestry ${inputTypeContext}`, () => testAncestorFormAssociated()); + it(`supports association via form ID ${inputTypeContext}`, () => testIdFormAssociated()); + + if (options.validation && !["color", "month", "time"].includes(options.inputType)) { + it(`supports required property validation ${inputTypeContext}`, () => testRequiredPropertyValidation()); } async function testAncestorFormAssociated(): Promise { @@ -790,30 +792,30 @@ export function formAssociated( const requiredValidationMessage = "Please fill out this field."; const { beforeContent, tagOrHTML } = getTagOrHTMLWithBeforeContent(componentTagOrHtml); const tag = getTag(tagOrHTML); - const componentHtml = ensureRequired(ensureName(isHTML(tagOrHTML) ? tagOrHTML : `<${tag}>`, tag), tag); + const componentHtml = ensureUnchecked( + ensureRequired(ensureName(isHTML(tagOrHTML) ? tagOrHTML : `<${tag}>`, tag), tag), + ); const page = await newE2EPage(); await beforeContent?.(page); - const content = html`
- ${componentHtml} - - -
`; + const content = html` +
+ ${componentHtml} + Submit +
+ `; await page.setContent(content); await page.waitForChanges(); const component = await page.find(tag); - const submitButton = await page.find("input"); + const submitButton = await page.find("calcite-button"); const spyEvent = await page.spyOnEvent(getClearValidationEventName(tag)); - await assertPreventsFormSubmission(page, component, options, submitButton, requiredValidationMessage); + await assertPreventsFormSubmission(page, component, submitButton, requiredValidationMessage); await assertClearsValidationOnValueChange(page, component, options, spyEvent, tag); - await assertUserMessageNotOverridden(page, component, options, submitButton); + await assertUserMessageNotOverridden(page, component, submitButton); } function ensureName(html: string, componentTag: string): string { @@ -824,6 +826,10 @@ export function formAssociated( return html.includes("required") ? html : html.replace(componentTag, `${componentTag} required `); } + function ensureUnchecked(html: string): string { + return html.replace(/(checked|selected)/, ""); + } + function ensureForm(html: string, componentTag: string): string { return html.includes("form=") ? html : html.replace(componentTag, `${componentTag} form="test-form" `); } @@ -1030,11 +1036,10 @@ export function formAssociated( async function assertPreventsFormSubmission( page: E2EPage, component: E2EElement, - options: FormAssociatedOptions, submitButton: E2EElement, message: string, ) { - await (options.submitsOnEnter ? page.keyboard.press("Enter") : submitButton.click()); + await submitButton.click(); await page.waitForChanges(); await expectValidationInvalid(component, message); @@ -1056,22 +1061,25 @@ export function formAssociated( } else if (tag === "calcite-select") { await page.keyboard.press("ArrowDown"); } else { - await page.keyboard.type(options.testValue); + // time picker needs to know whether it's AM or PM before it will commit the value + await page.keyboard.type(`${options.testValue}${tag === "calcite-input-time-picker" ? "PM" : ""}`); await page.keyboard.press("Tab"); } await page.waitForChanges(); - expect(event).toHaveReceivedEventTimes(1); + + // the components with Input events will emit multiple times, depending on + // the length of the test value + if (componentsWithInputEvent.includes(tag)) { + expect(event.length).toBeGreaterThanOrEqual(1); + } else { + expect(event).toHaveReceivedEventTimes(1); + } await expectValidationIdle(component); } - async function assertUserMessageNotOverridden( - page: E2EPage, - component: E2EElement, - options: FormAssociatedOptions, - submitButton: E2EElement, - ) { + async function assertUserMessageNotOverridden(page: E2EPage, component: E2EElement, submitButton: E2EElement) { const customValidationMessage = "This is a custom message."; const customValidationIcon = "banana"; @@ -1081,7 +1089,7 @@ export function formAssociated( component.setProperty("value", undefined); await page.waitForChanges(); - await (options.submitsOnEnter ? page.keyboard.press("Enter") : submitButton.click()); + await submitButton.click(); await page.waitForChanges(); await expectValidationInvalid(component, customValidationMessage, customValidationIcon); @@ -1093,7 +1101,7 @@ export function formAssociated( expect(await element.getProperty("validationIcon")).toBe(false); } - async function expectValidationInvalid(element: E2EElement, message: string, icon: string | boolean = true) { + async function expectValidationInvalid(element: E2EElement, message: string, icon: string = "") { expect(await element.getProperty("status")).toBe("invalid"); expect(await element.getProperty("validationMessage")).toBe(message); expect(element.getAttribute("validation-icon")).toBe(icon); diff --git a/packages/calcite-components/src/utils/form.tsx b/packages/calcite-components/src/utils/form.tsx index 89db053f323..d46c5d1c0e7 100644 --- a/packages/calcite-components/src/utils/form.tsx +++ b/packages/calcite-components/src/utils/form.tsx @@ -4,7 +4,7 @@ import { FunctionalComponent, h } from "@stencil/core"; /** * Any form with a `calciteInput` event needs to be included in this array. */ -const componentsWithInputEvent = [ +export const componentsWithInputEvent = [ "calcite-input", "calcite-input-number", "calcite-input-text", From 63cc643200a678062a1660110806dfba98eb0ac6 Mon Sep 17 00:00:00 2001 From: Ben Elan Date: Mon, 5 Feb 2024 11:02:48 -0800 Subject: [PATCH 3/5] test: review feedback --- .../src/components/combobox/combobox.e2e.ts | 2 +- .../input-time-picker.e2e.ts | 7 +++- .../src/components/select/select.e2e.ts | 2 +- .../src/tests/commonTests.ts | 37 ++++++++++++++----- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/packages/calcite-components/src/components/combobox/combobox.e2e.ts b/packages/calcite-components/src/components/combobox/combobox.e2e.ts index dba3ad7548c..e07a1427da7 100644 --- a/packages/calcite-components/src/components/combobox/combobox.e2e.ts +++ b/packages/calcite-components/src/components/combobox/combobox.e2e.ts @@ -1609,7 +1609,7 @@ describe("calcite-combobox", () => { `, - { testValue: "two", submitsOnEnter: true, validation: true }, + { testValue: "two", submitsOnEnter: true, validation: true, changeValueKeys: ["Space", "Enter"] }, ); }); diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts index 77c44a55c7c..fbabd7beb20 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts @@ -593,7 +593,12 @@ describe("calcite-input-time-picker", () => { }); describe("is form-associated", () => { - formAssociated("calcite-input-time-picker", { testValue: "03:23", submitsOnEnter: true, validation: true }); + formAssociated("calcite-input-time-picker", { + testValue: "03:23", + submitsOnEnter: true, + validation: true, + validUserInputTestValue: "03:23 AM", + }); }); it("updates value appropriately as step changes", async () => { diff --git a/packages/calcite-components/src/components/select/select.e2e.ts b/packages/calcite-components/src/components/select/select.e2e.ts index 67be28c8c42..0ab52454c9e 100644 --- a/packages/calcite-components/src/components/select/select.e2e.ts +++ b/packages/calcite-components/src/components/select/select.e2e.ts @@ -412,7 +412,7 @@ describe("calcite-select", () => { tres
`, - { testValue: "dos", validation: true }, + { testValue: "dos", validation: true, changeValueKeys: ["ArrowDown"] }, ); }); }); diff --git a/packages/calcite-components/src/tests/commonTests.ts b/packages/calcite-components/src/tests/commonTests.ts index b61c6c7b043..ef975771871 100644 --- a/packages/calcite-components/src/tests/commonTests.ts +++ b/packages/calcite-components/src/tests/commonTests.ts @@ -15,6 +15,7 @@ import { newProgrammaticE2EPage, skipAnimations, } from "./utils"; +import { KeyInput } from "puppeteer"; expect.extend(toHaveNoViolations); @@ -689,6 +690,24 @@ interface FormAssociatedOptions { */ expectedSubmitValue?: any; + /* + * Set this if the value required to emit an input/change event is different from `testValue`. + * The value is passed to `page.keyboard.type()`. For example, input-time-picker requires + * appending AM or PM before the value commits and calciteInputTimePickerChange emits. + * + * This option is only relevant when the `validation` option is enabled. + */ + validUserInputTestValue?: any; + + /* + * Set this if emitting an input/change event requires key presses. Each array item will be passed + * to `page.keyboard.press()`. For example, the combobox value can be changed by pressing "Space" + * to open the component and "Enter" to select a value. + * + * This option is only relevant when the `validation` option is enabled. + */ + changeValueKeys?: KeyInput[]; + /** * Specifies the input type that will be used to capture the value. */ @@ -705,7 +724,7 @@ interface FormAssociatedOptions { clearable?: boolean; /** - * Specifies whether the component supports preventing submission and displaying validation messages + * Specifies whether the component supports preventing submission and displaying validation messages. */ validation?: boolean; } @@ -802,7 +821,7 @@ export function formAssociated( const content = html`
${componentHtml} - Submit + Submit
`; @@ -810,7 +829,7 @@ export function formAssociated( await page.waitForChanges(); const component = await page.find(tag); - const submitButton = await page.find("calcite-button"); + const submitButton = await page.find("#submitButton"); const spyEvent = await page.spyOnEvent(getClearValidationEventName(tag)); await assertPreventsFormSubmission(page, component, submitButton, requiredValidationMessage); @@ -1055,14 +1074,12 @@ export function formAssociated( await component.callMethod("setFocus"); await page.waitForChanges(); - if (tag === "calcite-combobox") { - await page.keyboard.press("Space"); - await page.keyboard.press("Enter"); - } else if (tag === "calcite-select") { - await page.keyboard.press("ArrowDown"); + if (options?.changeValueKeys) { + for (const key of options?.changeValueKeys) { + await page.keyboard.press(key); + } } else { - // time picker needs to know whether it's AM or PM before it will commit the value - await page.keyboard.type(`${options.testValue}${tag === "calcite-input-time-picker" ? "PM" : ""}`); + await page.keyboard.type(options?.validUserInputTestValue || options.testValue); await page.keyboard.press("Tab"); } From bb75ce420faa95448e6fb2b41a4075efda17528c Mon Sep 17 00:00:00 2001 From: Ben Elan Date: Mon, 5 Feb 2024 11:15:22 -0800 Subject: [PATCH 4/5] chore: cleanup --- .../calcite-components/src/tests/commonTests.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/calcite-components/src/tests/commonTests.ts b/packages/calcite-components/src/tests/commonTests.ts index ef975771871..a2b66218f5e 100644 --- a/packages/calcite-components/src/tests/commonTests.ts +++ b/packages/calcite-components/src/tests/commonTests.ts @@ -686,7 +686,8 @@ interface FormAssociatedOptions { testValue: any; /** - * Set this if the expected submit value **is different** from stringifying `testValue`. For example, a component may transform an object to a serializable string. + * Set this if the expected submit value **is different** from stringifying `testValue`. + * For example, a component may transform an object to a serializable string. */ expectedSubmitValue?: any; @@ -697,7 +698,7 @@ interface FormAssociatedOptions { * * This option is only relevant when the `validation` option is enabled. */ - validUserInputTestValue?: any; + validUserInputTestValue?: string; /* * Set this if emitting an input/change event requires key presses. Each array item will be passed @@ -724,7 +725,7 @@ interface FormAssociatedOptions { clearable?: boolean; /** - * Specifies whether the component supports preventing submission and displaying validation messages. + * Specifies if the component supports preventing submission and displaying validation messages. */ validation?: boolean; } @@ -741,13 +742,13 @@ export function formAssociated( componentTagOrHtml: TagOrHTML | TagOrHTMLWithBeforeContent, options: FormAssociatedOptions, ): void { - const inputTypeContext = options.inputType ? `(input type="${options.inputType}")` : ""; + const inputTypeContext = options.inputType ? ` (input type="${options.inputType}")` : ""; - it(`supports association via ancestry ${inputTypeContext}`, () => testAncestorFormAssociated()); - it(`supports association via form ID ${inputTypeContext}`, () => testIdFormAssociated()); + it(`supports association via ancestry${inputTypeContext}`, () => testAncestorFormAssociated()); + it(`supports association via form ID${inputTypeContext}`, () => testIdFormAssociated()); if (options.validation && !["color", "month", "time"].includes(options.inputType)) { - it(`supports required property validation ${inputTypeContext}`, () => testRequiredPropertyValidation()); + it(`supports required property validation${inputTypeContext}`, () => testRequiredPropertyValidation()); } async function testAncestorFormAssociated(): Promise { From 222b80411404af9a941080073c3712842de0482f Mon Sep 17 00:00:00 2001 From: Ben Elan Date: Mon, 5 Feb 2024 11:34:23 -0800 Subject: [PATCH 5/5] chore: more cleanup --- .../calcite-components/src/tests/commonTests.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/calcite-components/src/tests/commonTests.ts b/packages/calcite-components/src/tests/commonTests.ts index a2b66218f5e..02e658744f7 100644 --- a/packages/calcite-components/src/tests/commonTests.ts +++ b/packages/calcite-components/src/tests/commonTests.ts @@ -742,12 +742,12 @@ export function formAssociated( componentTagOrHtml: TagOrHTML | TagOrHTMLWithBeforeContent, options: FormAssociatedOptions, ): void { - const inputTypeContext = options.inputType ? ` (input type="${options.inputType}")` : ""; + const inputTypeContext = options?.inputType ? ` (input type="${options.inputType}")` : ""; it(`supports association via ancestry${inputTypeContext}`, () => testAncestorFormAssociated()); it(`supports association via form ID${inputTypeContext}`, () => testIdFormAssociated()); - if (options.validation && !["color", "month", "time"].includes(options.inputType)) { + if (options?.validation && !["color", "month", "time"].includes(options?.inputType)) { it(`supports required property validation${inputTypeContext}`, () => testRequiredPropertyValidation()); } @@ -1072,22 +1072,18 @@ export function formAssociated( event: EventSpy, tag: string, ) { - await component.callMethod("setFocus"); - await page.waitForChanges(); - if (options?.changeValueKeys) { - for (const key of options?.changeValueKeys) { + for (const key of options.changeValueKeys) { await page.keyboard.press(key); } } else { - await page.keyboard.type(options?.validUserInputTestValue || options.testValue); + await page.keyboard.type(options?.validUserInputTestValue ?? options.testValue); await page.keyboard.press("Tab"); } await page.waitForChanges(); - // the components with Input events will emit multiple times, depending on - // the length of the test value + // components with an Input event will emit multiple times depending on the length of testValue if (componentsWithInputEvent.includes(tag)) { expect(event.length).toBeGreaterThanOrEqual(1); } else {