From 6de8534c5d36bfc90548a4bace1fa8ab81515bf9 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Fri, 9 Feb 2024 20:57:14 -0800 Subject: [PATCH] fix(input, input-number, input-text, text-area): ensure all applicable props are considered in form validation (#8655) **Related Issue:** #8647 ## Summary This ensures `pattern`, `minlength`, `maxlength`, `min`, `max`, `step` are set on the internal form input based on the matching type. **Note**: `minlength` and `maxlength` won't trigger constraint validation unless a user interacts with the input (see [`minlength` spec](https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#setting-minimum-input-length-requirements:-the-minlength-attribute:~:text=Constraint%20validation%3A%20If%20an%20element%20has%20a%20minimum,element%20is%20suffering%20from%20being%20too%20short.) and [`maxlength` spec](https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#setting-minimum-input-length-requirements:-the-minlength-attribute:~:text=Constraint%20validation%3A%20If%20an%20element%20has%20a%20maximum,element%20is%20suffering%20from%20being%20too%20long.)). This will have to be a known limitation until #8126 is completed. --- .../calcite-components/src/components.d.ts | 10 +++ .../input-number/input-number.e2e.ts | 4 +- .../components/input-number/input-number.tsx | 11 ++- .../components/input-text/input-text.e2e.ts | 4 +- .../src/components/input-text/input-text.tsx | 10 +-- .../src/components/input/common/input.spec.ts | 38 +++++++++ .../src/components/input/common/input.ts | 80 +++++++++++++++++++ .../src/components/input/common/tests.ts | 69 ++++++++++++++++ .../src/components/input/input.e2e.ts | 4 +- .../src/components/input/input.tsx | 22 ++--- .../src/components/text-area/text-area.tsx | 17 +++- 11 files changed, 236 insertions(+), 33 deletions(-) create mode 100644 packages/calcite-components/src/components/input/common/input.spec.ts create mode 100644 packages/calcite-components/src/components/input/common/input.ts diff --git a/packages/calcite-components/src/components.d.ts b/packages/calcite-components/src/components.d.ts index 273ff7865fd..9b069281ec3 100644 --- a/packages/calcite-components/src/components.d.ts +++ b/packages/calcite-components/src/components.d.ts @@ -4862,6 +4862,11 @@ export namespace Components { * Made into a prop for testing purposes only */ "messages": TextAreaMessages; + /** + * Specifies the minimum number of characters allowed. + * @mdn [minlength](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea#attr-minlength) + */ + "minLength": number; /** * Specifies the name of the component. * @mdn [name](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea#attr-name) @@ -12358,6 +12363,11 @@ declare namespace LocalJSX { * Made into a prop for testing purposes only */ "messages"?: TextAreaMessages; + /** + * Specifies the minimum number of characters allowed. + * @mdn [minlength](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea#attr-minlength) + */ + "minLength"?: number; /** * Specifies the name of the component. * @mdn [name](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea#attr-name) 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 70d7c939e21..7656be8e1e1 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 @@ -15,7 +15,7 @@ import { import { getElementRect, getElementXY, selectText } from "../../tests/utils"; import { letterKeys, numberKeys } from "../../utils/key"; import { locales, numberStringFormatter } from "../../utils/locale"; -import { testPostValidationFocusing } from "../input/common/tests"; +import { testHiddenInputSyncing, testPostValidationFocusing } from "../input/common/tests"; describe("calcite-input-number", () => { const delayFor2UpdatesInMs = 200; @@ -1753,6 +1753,8 @@ describe("calcite-input-number", () => { }); testPostValidationFocusing("calcite-input-number"); + + testHiddenInputSyncing("calcite-input-number"); }); describe("translation support", () => { diff --git a/packages/calcite-components/src/components/input-number/input-number.tsx b/packages/calcite-components/src/components/input-number/input-number.tsx index 4e11ad71e05..6e0ba4f55cf 100644 --- a/packages/calcite-components/src/components/input-number/input-number.tsx +++ b/packages/calcite-components/src/components/input-number/input-number.tsx @@ -70,6 +70,11 @@ import { InputNumberMessages } from "./assets/input-number/t9n"; import { CSS, SLOTS } from "./resources"; import { getIconScale } from "../../utils/component"; import { Validation } from "../functional/Validation"; +import { + NumericInputComponent, + syncHiddenFormInput, + TextualInputComponent, +} from "../input/common/input"; /** * @slot action - A slot for positioning a button next to the component. @@ -86,7 +91,9 @@ export class InputNumber FormComponent, InteractiveComponent, LocalizedComponent, + NumericInputComponent, T9nComponent, + TextualInputComponent, LoadableComponent { //-------------------------------------------------------------------------- @@ -798,9 +805,7 @@ export class InputNumber }; syncHiddenFormInput(input: HTMLInputElement): void { - input.type = "number"; - input.min = this.min?.toString(10) ?? ""; - input.max = this.max?.toString(10) ?? ""; + syncHiddenFormInput("number", this, input); } private onHiddenFormInputInput = (event: Event): void => { 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 0211ca26fb4..876b7b01ed7 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 @@ -12,7 +12,7 @@ import { t9n, } from "../../tests/commonTests"; import { selectText } from "../../tests/utils"; -import { testPostValidationFocusing } from "../input/common/tests"; +import { testHiddenInputSyncing, testPostValidationFocusing } from "../input/common/tests"; describe("calcite-input-text", () => { describe("labelable", () => { @@ -465,6 +465,8 @@ describe("calcite-input-text", () => { formAssociated("calcite-input-text", { testValue: "test", submitsOnEnter: true, validation: true }); testPostValidationFocusing("calcite-input-text"); + + testHiddenInputSyncing("calcite-input-text"); }); describe("translation support", () => { diff --git a/packages/calcite-components/src/components/input-text/input-text.tsx b/packages/calcite-components/src/components/input-text/input-text.tsx index c711293efdf..384d0bc9797 100644 --- a/packages/calcite-components/src/components/input-text/input-text.tsx +++ b/packages/calcite-components/src/components/input-text/input-text.tsx @@ -50,6 +50,7 @@ import { InputTextMessages } from "./assets/input-text/t9n"; import { CSS, SLOTS } from "./resources"; import { getIconScale } from "../../utils/component"; import { Validation } from "../functional/Validation"; +import { syncHiddenFormInput, TextualInputComponent } from "../input/common/input"; /** * @slot action - A slot for positioning a button next to the component. @@ -67,6 +68,7 @@ export class InputText InteractiveComponent, LoadableComponent, LocalizedComponent, + TextualInputComponent, T9nComponent { //-------------------------------------------------------------------------- @@ -494,13 +496,7 @@ export class InputText }; syncHiddenFormInput(input: HTMLInputElement): void { - if (this.minLength != null) { - input.minLength = this.minLength; - } - - if (this.maxLength != null) { - input.maxLength = this.maxLength; - } + syncHiddenFormInput("text", this, input); } private onHiddenFormInputInput = (event: Event): void => { diff --git a/packages/calcite-components/src/components/input/common/input.spec.ts b/packages/calcite-components/src/components/input/common/input.spec.ts new file mode 100644 index 00000000000..c69069f4277 --- /dev/null +++ b/packages/calcite-components/src/components/input/common/input.spec.ts @@ -0,0 +1,38 @@ +/* eslint-disable jest/no-conditional-expect -- Using conditional logic in a confined test helper to handle specific scenarios, reducing duplication, balancing test readability and maintainability. **/ +import { minMaxLengthTypes, minMaxStepTypes, patternTypes, syncHiddenFormInput } from "./input"; + +describe("common input utils", () => { + it("syncHiddenFormInput", async () => { + const minMaxLengthTestValues = { minLength: 0, maxLength: 10 }; + const patternTestValue = { pattern: "test" }; + const minMaxStepTestValues = { min: 0, max: 10, step: 1 }; + + const allTypes = Array.from(new Set([...minMaxLengthTypes, ...patternTypes, ...minMaxStepTypes])); + const allValueFakeInputComponent = { ...minMaxLengthTestValues, ...minMaxStepTestValues, ...patternTestValue }; + + const hiddenFormInput = document.createElement("input"); + + allTypes.forEach((type) => { + syncHiddenFormInput(type, allValueFakeInputComponent, hiddenFormInput); + + const expectedType = type === "textarea" ? "text" : type; + + expect(hiddenFormInput.type).toBe(expectedType); + + if (minMaxStepTypes.includes(type)) { + expect(hiddenFormInput.min).toBe("0"); + expect(hiddenFormInput.max).toBe("10"); + expect(hiddenFormInput.step).toBe("1"); + } + + if (minMaxLengthTypes.includes(type)) { + expect(hiddenFormInput.minLength).toBe(0); + expect(hiddenFormInput.maxLength).toBe(10); + } + + if (patternTypes.includes(type)) { + expect(hiddenFormInput.pattern).toBe("test"); + } + }); + }); +}); diff --git a/packages/calcite-components/src/components/input/common/input.ts b/packages/calcite-components/src/components/input/common/input.ts new file mode 100644 index 00000000000..c964f151721 --- /dev/null +++ b/packages/calcite-components/src/components/input/common/input.ts @@ -0,0 +1,80 @@ +export type InputComponent = NumericInputComponent | TextualInputComponent; + +export interface NumericInputComponent { + min: number; + max: number; + step: number | "any"; +} + +export interface TextualInputComponent { + pattern?: string; + minLength: number; + maxLength: number; +} + +/** + * Exported for testing purposes only + */ +export const minMaxStepTypes = ["date", "datetime-local", "month", "number", "range", "time", "week"]; + +/** + * Exported for testing purposes only + */ +export const patternTypes = ["email", "password", "search", "tel", "text", "url"]; + +/** + * Exported for testing purposes only + */ +export const minMaxLengthTypes = ["email", "password", "search", "tel", "text", "textarea", "url"]; + +function updateConstraintValidation( + inputComponent: InputComponent, + input: HTMLInputElement, + propName: string, + matchesType: boolean, +): void { + const attributeName = propName.toLowerCase(); + const value = inputComponent[propName]; + + if (matchesType && value != null) { + input.setAttribute(attributeName, `${value}`); + } else { + // we remove the attribute to ensure validation-constraints are properly reset + input.removeAttribute(attributeName); + } +} + +/** + * Synchronizes the hidden form input with the validation-related input properties. + * + * Note: loss of precision is expected due to the hidden input's value and validation-constraint props being strings. + * + * @param type - The input type. + * @param inputComponent + * @param hiddenFormInput + */ +export function syncHiddenFormInput( + type: HTMLInputElement["type"] | "textarea", + inputComponent: InputComponent, + hiddenFormInput: HTMLInputElement, +): void { + hiddenFormInput.type = type === "textarea" ? "text" : type; + + const isMinMaxStepType = minMaxStepTypes.includes(type); + const numericInputComponent = inputComponent as NumericInputComponent; + + updateConstraintValidation(numericInputComponent, hiddenFormInput, "min", isMinMaxStepType); + updateConstraintValidation(numericInputComponent, hiddenFormInput, "max", isMinMaxStepType); + updateConstraintValidation(numericInputComponent, hiddenFormInput, "step", isMinMaxStepType); + + const isMinMaxLengthType = minMaxLengthTypes.includes(type); + + const textualInputComponent = inputComponent as TextualInputComponent; + + updateConstraintValidation(textualInputComponent, hiddenFormInput, "minLength", isMinMaxLengthType); + updateConstraintValidation(textualInputComponent, hiddenFormInput, "maxLength", isMinMaxLengthType); + + const isPatternType = patternTypes.includes(type); + + updateConstraintValidation(textualInputComponent, hiddenFormInput, "pattern", isPatternType); +} diff --git a/packages/calcite-components/src/components/input/common/tests.ts b/packages/calcite-components/src/components/input/common/tests.ts index d0200d46f57..94360f0535a 100644 --- a/packages/calcite-components/src/components/input/common/tests.ts +++ b/packages/calcite-components/src/components/input/common/tests.ts @@ -1,3 +1,5 @@ +/* eslint-disable jest/no-conditional-expect -- Using conditional logic in a confined test helper to handle specific scenarios, reducing duplication, balancing test readability and maintainability. **/ + import { newE2EPage } from "@stencil/core/testing"; import { isElementFocused } from "../../../tests/utils"; import { hiddenFormInputSlotName } from "../../../utils/form"; @@ -46,3 +48,70 @@ export function testPostValidationFocusing( expect(await input.getProperty("value")).toBe(expectedValue); }); } + +export function testHiddenInputSyncing( + inputTag: Extract< + keyof JSX.IntrinsicElements, + "calcite-input" | "calcite-input-text" | "calcite-input-number" | "calcite-text-area" + >, +): void { + it("syncs hidden input with the input component", async () => { + const page = await newE2EPage(); + await page.setContent(html` +
+ <${inputTag} name="form-name"> +
+ `); + const input = await page.find(inputTag); + const hiddenInput = await page.find(`input[slot=${hiddenFormInputSlotName}]`); + + // intentionally setting all props regardless of type for testing purposes + input.setProperty("min", 0); + input.setProperty("max", 10); + input.setProperty("step", 1); + input.setProperty("pattern", "test"); + input.setProperty("minLength", 0); + input.setProperty("maxLength", 10); + await page.waitForChanges(); + + async function assertTextProps(): Promise { + expect(await hiddenInput.getProperty("type")).toBe("text"); + expect(await hiddenInput.getProperty("min")).toBe(""); + expect(await hiddenInput.getProperty("max")).toBe(""); + expect(await hiddenInput.getProperty("pattern")).toBe("test"); + expect(await hiddenInput.getProperty("minLength")).toBe(0); + expect(await hiddenInput.getProperty("maxLength")).toBe(10); + } + + async function assertNumericProps(): Promise { + expect(await hiddenInput.getProperty("type")).toBe("number"); + expect(await hiddenInput.getProperty("min")).toBe("0"); + expect(await hiddenInput.getProperty("max")).toBe("10"); + expect(await hiddenInput.getProperty("pattern")).toBe(""); + expect(await hiddenInput.getProperty("minLength")).toBe(-1); + expect(await hiddenInput.getProperty("maxLength")).toBe(-1); + } + + if (inputTag === "calcite-input") { + // testing subset of types + + await input.setProperty("type", "text"); + await page.waitForChanges(); + + await assertTextProps(); + + await input.setProperty("type", "number"); + await page.waitForChanges(); + + await assertNumericProps(); + return; + } + + if (inputTag === "calcite-input-text" || inputTag === "calcite-text-area") { + await assertTextProps(); + return; + } + + await assertNumericProps(); + }); +} diff --git a/packages/calcite-components/src/components/input/input.e2e.ts b/packages/calcite-components/src/components/input/input.e2e.ts index 22ab160dcd2..8eadc7bbf6e 100644 --- a/packages/calcite-components/src/components/input/input.e2e.ts +++ b/packages/calcite-components/src/components/input/input.e2e.ts @@ -15,7 +15,7 @@ import { letterKeys, numberKeys } from "../../utils/key"; import { locales, numberStringFormatter } from "../../utils/locale"; import { getElementRect, getElementXY, selectText } from "../../tests/utils"; import { KeyInput } from "puppeteer"; -import { testPostValidationFocusing } from "./common/tests"; +import { testHiddenInputSyncing, testPostValidationFocusing } from "./common/tests"; describe("calcite-input", () => { const delayFor2UpdatesInMs = 200; @@ -2058,6 +2058,8 @@ describe("calcite-input", () => { } testPostValidationFocusing("calcite-input"); + + testHiddenInputSyncing("calcite-input"); }); describe("translation support", () => { diff --git a/packages/calcite-components/src/components/input/input.tsx b/packages/calcite-components/src/components/input/input.tsx index b80c5762e42..1a6e8754d89 100644 --- a/packages/calcite-components/src/components/input/input.tsx +++ b/packages/calcite-components/src/components/input/input.tsx @@ -71,6 +71,7 @@ import { InputPlacement, NumberNudgeDirection, SetValueOrigin } from "./interfac import { CSS, INPUT_TYPE_ICONS, SLOTS } from "./resources"; import { getIconScale } from "../../utils/component"; import { Validation } from "../functional/Validation"; +import { NumericInputComponent, syncHiddenFormInput, TextualInputComponent } from "./common/input"; /** * @slot action - A slot for positioning a `calcite-button` next to the component. @@ -88,7 +89,9 @@ export class Input InteractiveComponent, T9nComponent, LocalizedComponent, - LoadableComponent + LoadableComponent, + NumericInputComponent, + TextualInputComponent { //-------------------------------------------------------------------------- // @@ -884,22 +887,7 @@ export class Input }; syncHiddenFormInput(input: HTMLInputElement): void { - const { type } = this; - - input.type = type; - - if (type === "number") { - input.min = this.min?.toString(10) ?? ""; - input.max = this.max?.toString(10) ?? ""; - } else if (type === "text") { - if (this.minLength != null) { - input.minLength = this.minLength; - } - - if (this.maxLength != null) { - input.maxLength = this.maxLength; - } - } + syncHiddenFormInput(this.type, this, input); } private onHiddenFormInputInput = (event: Event): void => { diff --git a/packages/calcite-components/src/components/text-area/text-area.tsx b/packages/calcite-components/src/components/text-area/text-area.tsx index 86f1d9f0cb2..226f934da08 100644 --- a/packages/calcite-components/src/components/text-area/text-area.tsx +++ b/packages/calcite-components/src/components/text-area/text-area.tsx @@ -50,6 +50,7 @@ import { CharacterLengthObj } from "./interfaces"; import { guid } from "../../utils/guid"; import { Status } from "../interfaces"; import { Validation } from "../functional/Validation"; +import { syncHiddenFormInput, TextualInputComponent } from "../input/common/input"; /** * @slot - A slot for adding text. @@ -70,7 +71,8 @@ export class TextArea LocalizedComponent, LoadableComponent, T9nComponent, - InteractiveComponent + InteractiveComponent, + Omit { //-------------------------------------------------------------------------- // @@ -117,6 +119,13 @@ export class TextArea */ @Prop() label: string; + /** + * Specifies the minimum number of characters allowed. + * + * @mdn [minlength](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea#attr-minlength) + */ + @Prop({ reflect: true }) minLength: number; + /** * Specifies the maximum number of characters allowed. * @@ -189,7 +198,7 @@ export class TextArea @Prop({ reflect: true }) status: Status = "idle"; /** The component's value. */ - @Prop({ mutable: true }) value: string; + @Prop({ mutable: true }) value = ""; /** * Specifies the wrapping mechanism for the text. @@ -479,6 +488,8 @@ export class TextArea if (this.isCharacterLimitExceeded()) { input.setCustomValidity(this.replacePlaceHoldersInMessages()); } + + syncHiddenFormInput("textarea", this, input); } setTextAreaEl = (el: HTMLTextAreaElement): void => { @@ -534,7 +545,7 @@ export class TextArea } }, RESIZE_TIMEOUT, - { leading: false } + { leading: false }, ); private isCharacterLimitExceeded(): boolean {