From 07e2d2f7e7f94b5c7f988d53e80da51aec228585 Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Fri, 7 Feb 2025 13:01:00 +0100 Subject: [PATCH 01/10] feat: avoid internal validation before interacting with input --- .changeset/six-wombats-grow.md | 20 ++++ .changeset/stale-ladybugs-wave.md | 9 ++ .../boolean-value-accessor.ts | 4 +- .../radio-value-accessor.ts | 4 +- .../control-value-accessors/value-accessor.ts | 111 +++++++++--------- packages/core/src/components.d.ts | 20 ++++ .../src/components/date-input/date-input.tsx | 15 ++- packages/core/src/components/input/input.tsx | 16 ++- .../core/src/components/input/input.util.ts | 4 +- .../src/components/input/number-input.tsx | 15 ++- .../components/input/tests/validation.ct.ts | 29 +++++ .../core/src/components/input/textarea.tsx | 15 ++- .../core/src/components/select/select.tsx | 12 ++ .../core/src/components/utils/input/index.ts | 7 +- .../src/components/utils/input/validation.ts | 21 +++- .../src/stories/input-date.stories.ts | 37 ++++++ .../src/stories/input-number.stories.ts | 11 +- .../src/stories/input.stories.ts | 37 ++++++ .../src/stories/select.stories.ts | 7 ++ 19 files changed, 318 insertions(+), 76 deletions(-) create mode 100644 .changeset/six-wombats-grow.md create mode 100644 .changeset/stale-ladybugs-wave.md create mode 100644 packages/storybook-docs/src/stories/input-date.stories.ts create mode 100644 packages/storybook-docs/src/stories/input.stories.ts diff --git a/.changeset/six-wombats-grow.md b/.changeset/six-wombats-grow.md new file mode 100644 index 00000000000..c2479468edb --- /dev/null +++ b/.changeset/six-wombats-grow.md @@ -0,0 +1,20 @@ +--- +'@siemens/ix-angular': minor +--- + +Add `suppressClassMapping` to value-accessors to prevent that the accessors automatically map `ng-`-classes to `ix-`-classes. + +If `[suppressClassMapping]="true"` you need to control the `ix-`-classes on your own. + +```html + + +``` + +Fixes #1638 #1680 diff --git a/.changeset/stale-ladybugs-wave.md b/.changeset/stale-ladybugs-wave.md new file mode 100644 index 00000000000..4aa31e6771d --- /dev/null +++ b/.changeset/stale-ladybugs-wave.md @@ -0,0 +1,9 @@ +--- +'@siemens/ix': patch +--- + +Prevent input elements like (`ix-input`, `ix-number-input`, `ix-date-input`, `ix-select`, `ix-textarea`) to show `required` validation error without any user interaction. + +If the class `ix-invalid` is applied programmatically an error message is still shown even without a user interaction. + +Fixes #1638, #1680 diff --git a/packages/angular/src/control-value-accessors/boolean-value-accessor.ts b/packages/angular/src/control-value-accessors/boolean-value-accessor.ts index d8bf75e1be7..c19033bf0c5 100644 --- a/packages/angular/src/control-value-accessors/boolean-value-accessor.ts +++ b/packages/angular/src/control-value-accessors/boolean-value-accessor.ts @@ -8,7 +8,7 @@ */ import { Directive, HostListener, ElementRef, Injector } from '@angular/core'; import { NG_VALUE_ACCESSOR } from '@angular/forms'; -import { ValueAccessor, mapNgToIxClassNames } from './value-accessor'; +import { ValueAccessor } from './value-accessor'; @Directive({ selector: 'ix-checkbox,ix-toggle', @@ -27,7 +27,7 @@ export class BooleanValueAccessorDirective extends ValueAccessor { override writeValue(value: boolean): void { this.elementRef.nativeElement.checked = this.lastValue = value; - mapNgToIxClassNames(this.elementRef); + super.mapNgToIxClassNames(this.elementRef); } @HostListener('checkedChange', ['$event.target']) diff --git a/packages/angular/src/control-value-accessors/radio-value-accessor.ts b/packages/angular/src/control-value-accessors/radio-value-accessor.ts index 2f3c296277b..98e6a018672 100644 --- a/packages/angular/src/control-value-accessors/radio-value-accessor.ts +++ b/packages/angular/src/control-value-accessors/radio-value-accessor.ts @@ -8,7 +8,7 @@ */ import { Directive, HostListener, ElementRef, Injector } from '@angular/core'; import { NG_VALUE_ACCESSOR } from '@angular/forms'; -import { ValueAccessor, mapNgToIxClassNames } from './value-accessor'; +import { ValueAccessor } from './value-accessor'; @Directive({ selector: 'ix-radio', @@ -29,7 +29,7 @@ export class RadioValueAccessorDirective extends ValueAccessor { this.lastValue = value; this.elementRef.nativeElement.checked = this.elementRef.nativeElement.value === value; - mapNgToIxClassNames(this.elementRef); + super.mapNgToIxClassNames(this.elementRef); } @HostListener('checkedChange', ['$event.target']) diff --git a/packages/angular/src/control-value-accessors/value-accessor.ts b/packages/angular/src/control-value-accessors/value-accessor.ts index 005c01af630..1d0fa6871a0 100644 --- a/packages/angular/src/control-value-accessors/value-accessor.ts +++ b/packages/angular/src/control-value-accessors/value-accessor.ts @@ -13,6 +13,7 @@ import { OnDestroy, Directive, HostListener, + Input, } from '@angular/core'; import { ControlValueAccessor, NgControl } from '@angular/forms'; import { Subscription } from 'rxjs'; @@ -32,6 +33,8 @@ export class ValueAccessor protected lastValue: any; private statusChanges?: Subscription; + @Input() suppressClassMapping = false; + constructor(protected injector: Injector, protected elementRef: ElementRef) {} writeValue(value: any): void { @@ -88,7 +91,7 @@ export class ValueAccessor }); } - detourFormControlMethods(ngControl, this.elementRef); + this.detourFormControlMethods(ngControl, this.elementRef); } getAssignedNgControl(): NgControl | null { @@ -107,63 +110,61 @@ export class ValueAccessor if (!ngControl) { return; } - mapNgToIxClassNames(this.elementRef); + this.mapNgToIxClassNames(this.elementRef); } -} -const detourFormControlMethods = ( - ngControl: NgControl, - elementRef: ElementRef -) => { - const formControl = ngControl.control as any; - if (formControl) { - const methodsToPatch = [ - 'markAsTouched', - 'markAllAsTouched', - 'markAsUntouched', - 'markAsDirty', - 'markAsPristine', - ]; - methodsToPatch.forEach((method) => { - if (typeof formControl[method] !== 'undefined') { - const oldFn = formControl[method].bind(formControl); - formControl[method] = (...params: any[]) => { - oldFn(...params); - mapNgToIxClassNames(elementRef); - }; - } + detourFormControlMethods(ngControl: NgControl, elementRef: ElementRef) { + const formControl = ngControl.control as any; + if (formControl) { + const methodsToPatch = [ + 'markAsTouched', + 'markAllAsTouched', + 'markAsUntouched', + 'markAsDirty', + 'markAsPristine', + ]; + methodsToPatch.forEach((method) => { + if (typeof formControl[method] !== 'undefined') { + const oldFn = formControl[method].bind(formControl); + formControl[method] = (...params: any[]) => { + oldFn(...params); + this.mapNgToIxClassNames(elementRef); + }; + } + }); + } + } + + async mapNgToIxClassNames(element: ElementRef): Promise { + if (this.suppressClassMapping) { + return; + } + setTimeout(async () => { + const input = element.nativeElement; + + const classes = this.getClasses(input); + const classList = input.classList; + classList.remove( + 'ix-valid', + 'ix-invalid', + 'ix-touched', + 'ix-untouched', + 'ix-dirty', + 'ix-pristine' + ); + classList.add(...classes); }); } -}; - -export const mapNgToIxClassNames = async ( - element: ElementRef -): Promise => { - setTimeout(async () => { - const input = element.nativeElement; - - const classes = getClasses(input); - const classList = input.classList; - classList.remove( - 'ix-valid', - 'ix-invalid', - 'ix-touched', - 'ix-untouched', - 'ix-dirty', - 'ix-pristine' - ); - classList.add(...classes); - }); -}; - -const getClasses = (element: HTMLElement) => { - const classList = element.classList; - const classes: string[] = []; - for (let i = 0; i < classList.length; i++) { - const item = classList.item(i); - if (item?.startsWith(ValueAccessor.ANGULAR_CLASS_PREFIX)) { - classes.push(`ix-${item.substring(3)}`); + + getClasses(element: HTMLElement) { + const classList = element.classList; + const classes: string[] = []; + for (let i = 0; i < classList.length; i++) { + const item = classList.item(i); + if (item?.startsWith(ValueAccessor.ANGULAR_CLASS_PREFIX)) { + classes.push(`ix-${item.substring(3)}`); + } } + return classes; } - return classes; -}; +} diff --git a/packages/core/src/components.d.ts b/packages/core/src/components.d.ts index 1f7b1d41d93..abee13fe5d1 100644 --- a/packages/core/src/components.d.ts +++ b/packages/core/src/components.d.ts @@ -777,6 +777,10 @@ export namespace Components { * error text below the input field */ "invalidText"?: string; + /** + * Returns whether the text field has been touched. + */ + "isTouched": () => Promise; /** * label of the input field */ @@ -1585,6 +1589,10 @@ export namespace Components { * The error text for the text field. */ "invalidText"?: string; + /** + * Returns whether the text field has been touched. + */ + "isTouched": () => Promise; /** * The label for the text field. */ @@ -2153,6 +2161,10 @@ export namespace Components { * The error text for the input field */ "invalidText"?: string; + /** + * Returns true if the input field has been touched + */ + "isTouched": () => Promise; /** * The label for the input field */ @@ -2522,6 +2534,10 @@ export namespace Components { * @since 2.6.0 */ "invalidText"?: string; + /** + * Check if the input field has been touched. + */ + "isTouched": () => Promise; /** * Label for the select component * @since 2.6.0 @@ -2785,6 +2801,10 @@ export namespace Components { * The error text for the textarea field. */ "invalidText"?: string; + /** + * Check if the textarea field has been touched. + */ + "isTouched": () => Promise; /** * The label for the textarea field. */ diff --git a/packages/core/src/components/date-input/date-input.tsx b/packages/core/src/components/date-input/date-input.tsx index e84807bcc70..751643f3c2a 100644 --- a/packages/core/src/components/date-input/date-input.tsx +++ b/packages/core/src/components/date-input/date-input.tsx @@ -173,6 +173,7 @@ export class DateInput implements IxInputFieldComponent { private readonly dropdownElementRef = makeRef(); private classObserver?: ClassMutationObserver; private invalidReason?: string; + private touched = false; updateFormInternalValue(value: string): void { this.formInternals.setFormValue(value); @@ -337,7 +338,10 @@ export class DateInput implements IxInputFieldComponent { this.openDropdown(); this.ixFocus.emit(); }} - onBlur={() => this.ixBlur.emit()} + onBlur={() => { + this.ixBlur.emit(); + this.touched = true; + }} > { return (await this.getNativeInputElement()).focus(); } + /** + * Returns whether the text field has been touched. + * @internal + */ + @Method() + isTouched(): Promise { + return Promise.resolve(this.touched); + } + render() { const invalidText = this.isInputInvalid ? this.i18nErrorDateUnparsable diff --git a/packages/core/src/components/input/input.tsx b/packages/core/src/components/input/input.tsx index f2ab6dd7746..6aeb1e0f055 100644 --- a/packages/core/src/components/input/input.tsx +++ b/packages/core/src/components/input/input.tsx @@ -171,8 +171,8 @@ export class Input implements IxInputFieldComponent { private readonly inputRef = makeRef(); private readonly slotEndRef = makeRef(); private readonly slotStartRef = makeRef(); - private readonly inputId = `input-${inputIds++}`; + private touched = false; @HookValidationLifecycle() updateClassMappings(result: ValidationResults) { @@ -234,6 +234,15 @@ export class Input implements IxInputFieldComponent { return (await this.getNativeInputElement()).focus(); } + /** + * Returns whether the text field has been touched. + * @internal + */ + @Method() + isTouched(): Promise { + return Promise.resolve(this.touched); + } + render() { const inputAria: A11yAttributes = getAriaAttributesForInput(this); return ( @@ -282,7 +291,10 @@ export class Input implements IxInputFieldComponent { updateFormInternalValue={(value) => this.updateFormInternalValue(value) } - onBlur={() => onInputBlur(this, this.inputRef.current)} + onBlur={() => { + onInputBlur(this, this.inputRef.current); + this.touched = true; + }} ariaAttributes={inputAria} > ( } export function onInputBlur( - comp: IxInputFieldComponent, + comp: IxFormComponent, input?: HTMLInputElement | HTMLTextAreaElement | null ) { comp.ixBlur.emit(); @@ -84,6 +84,8 @@ export function onInputBlur( if (!input) { throw new Error('Input element is not available'); } + + input.setAttribute('data-ix-touched', 'true'); checkInternalValidity(comp, input); } diff --git a/packages/core/src/components/input/number-input.tsx b/packages/core/src/components/input/number-input.tsx index 4573e4b6925..e8f45ee7cf4 100644 --- a/packages/core/src/components/input/number-input.tsx +++ b/packages/core/src/components/input/number-input.tsx @@ -168,6 +168,7 @@ export class NumberInput implements IxInputFieldComponent { private readonly slotEndRef = makeRef(); private readonly slotStartRef = makeRef(); private readonly numberInputId = `number-input-${numberInputIds++}`; + private touched = false; @HookValidationLifecycle() updateClassMappings(result: ValidationResults) { @@ -223,6 +224,15 @@ export class NumberInput implements IxInputFieldComponent { return (await this.getNativeInputElement()).focus(); } + /** + * Returns true if the input field has been touched + * @internal + */ + @Method() + isTouched(): Promise { + return Promise.resolve(this.touched); + } + render() { const showStepperButtons = this.showStepperButtons && (this.disabled || this.readonly) === false; @@ -278,7 +288,10 @@ export class NumberInput implements IxInputFieldComponent { updateFormInternalValue={(value) => this.updateFormInternalValue(Number(value)) } - onBlur={() => onInputBlur(this, this.inputRef.current)} + onBlur={() => { + onInputBlur(this, this.inputRef.current); + this.touched = true; + }} > { await expect(input).toHaveClass(/ix-invalid/); }); }); + +test.describe.configure({ + mode: 'parallel', +}); +test.describe('prevent initial require validation', async () => { + [ + 'ix-input', + 'ix-number-input', + 'ix-date-input', + 'ix-select', + 'ix-textarea', + ].forEach((selector) => { + test(selector, async ({ mount, page }) => { + await mount(`<${selector} required>`); + + const inputComponent = page.locator(selector); + const input = inputComponent.locator( + selector !== 'ix-textarea' ? 'input' : 'textarea' + ); + await expect(inputComponent).toBeVisible(); + await expect(inputComponent).not.toHaveClass(/ix-invalid/); + + await input.focus(); + await input.blur(); + + await expect(inputComponent).toHaveClass(/ix-invalid/); + }); + }); +}); diff --git a/packages/core/src/components/input/textarea.tsx b/packages/core/src/components/input/textarea.tsx index c2bbff8e91b..a7341d1f3b0 100644 --- a/packages/core/src/components/input/textarea.tsx +++ b/packages/core/src/components/input/textarea.tsx @@ -171,6 +171,7 @@ export class Textarea implements IxInputFieldComponent { @State() isInvalidByRequired = false; private readonly textAreaRef = makeRef(); + private touched = false; @HookValidationLifecycle() updateClassMappings(result: ValidationResults) { @@ -214,6 +215,15 @@ export class Textarea implements IxInputFieldComponent { return (await this.getNativeInputElement()).focus(); } + /** + * Check if the textarea field has been touched. + * @internal + * */ + @Method() + isTouched(): Promise { + return Promise.resolve(this.touched); + } + render() { return ( { updateFormInternalValue={(value) => this.updateFormInternalValue(value) } - onBlur={() => onInputBlur(this, this.textAreaRef.current)} + onBlur={() => { + onInputBlur(this, this.textAreaRef.current); + this.touched = true; + }} > diff --git a/packages/core/src/components/select/select.tsx b/packages/core/src/components/select/select.tsx index 69caa9f8930..3d5ccea4514 100644 --- a/packages/core/src/components/select/select.tsx +++ b/packages/core/src/components/select/select.tsx @@ -246,6 +246,8 @@ export class Select implements IxInputFieldComponent { private addItemElement?: HTMLIxDropdownItemElement; private arrowFocusController?: ArrowFocusController; + private touched = false; + private readonly itemObserver = createMutationObserver(() => { if (!this.arrowFocusController) { return; @@ -765,6 +767,7 @@ export class Select implements IxInputFieldComponent { private onInputBlur(event: Event) { this.ixBlur.emit(); + this.touched = true; if (this.editable) { return; @@ -848,6 +851,15 @@ export class Select implements IxInputFieldComponent { } } + /** + * Check if the input field has been touched. + * @internal + * */ + @Method() + isTouched(): Promise { + return Promise.resolve(this.touched); + } + render() { return ( extends IxComponent { hasValidValue(): Promise; getValidityState?(): Promise; getAssociatedFormElement(): Promise; + isTouched?(): Promise; } export interface IxInputFieldComponent @@ -95,9 +96,9 @@ export interface IxInputFieldComponent focusInput(): void; } -export function isIxInputFieldComponent( - obj: HTMLElement -): obj is HTMLIxInputFieldComponentElement { +export function isIxInputFieldComponent( + obj: HTMLElement | IxFormComponent +): obj is HTMLIxInputFieldComponentElement { return ( obj && 'getAssociatedFormElement' in obj && diff --git a/packages/core/src/components/utils/input/validation.ts b/packages/core/src/components/utils/input/validation.ts index 8549de2fdd7..f7b5b19dd6f 100644 --- a/packages/core/src/components/utils/input/validation.ts +++ b/packages/core/src/components/utils/input/validation.ts @@ -14,6 +14,12 @@ export type ClassMutationObserver = { destroy: () => void; }; +export async function isTouched(host: IxFormComponent) { + if (typeof host.isTouched === 'function') { + return host.isTouched(); + } +} + export async function shouldSuppressInternalValidation( host: IxFormComponent ) { @@ -106,20 +112,21 @@ export function HookValidationLifecycle(options?: { ) as unknown as HTMLIxFormComponentElement; checkIfRequiredFunction = async () => { - const skipValidation = await shouldSuppressInternalValidation(host); - if (skipValidation) { - return; - } - if (host.hasValidValue && typeof host.hasValidValue === 'function') { const hasValue = await host.hasValidValue(); + const touched = await isTouched(host); if (host.required) { - host.classList.toggle('ix-invalid--required', !hasValue); + host.classList.toggle('ix-invalid--required', !hasValue && touched); } else { host.classList.remove('ix-invalid--required'); } } + const skipValidation = await shouldSuppressInternalValidation(host); + if (skipValidation) { + return; + } + if ( host.getValidityState && typeof host.getValidityState === 'function' @@ -134,6 +141,7 @@ export function HookValidationLifecycle(options?: { }; host.addEventListener('valueChange', checkIfRequiredFunction); + host.addEventListener('ixBlur', checkIfRequiredFunction); setTimeout(checkIfRequiredFunction); return connectedCallback?.call(this); }; @@ -165,6 +173,7 @@ export function HookValidationLifecycle(options?: { if (host && checkIfRequiredFunction) { host.removeEventListener('valueChange', checkIfRequiredFunction); + host.removeEventListener('ixBlur', checkIfRequiredFunction); } return disconnectedCallback?.call(this); diff --git a/packages/storybook-docs/src/stories/input-date.stories.ts b/packages/storybook-docs/src/stories/input-date.stories.ts new file mode 100644 index 00000000000..9e4bea7d9b4 --- /dev/null +++ b/packages/storybook-docs/src/stories/input-date.stories.ts @@ -0,0 +1,37 @@ +/* + * SPDX-FileCopyrightText: 2024 Siemens AG + * + * SPDX-License-Identifier: MIT + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +import type { Components } from '@siemens/ix/components'; +import type { ArgTypes, Meta, StoryObj } from '@storybook/web-components'; +import { genericRender, makeArgTypes } from './utils/generic-render'; + +type Element = Components.IxDateInput; + +const meta = { + title: 'Example/Input/Date', + tags: [], + render: (args) => genericRender('ix-date-input', args), + argTypes: makeArgTypes>>('ix-date-input', {}), + parameters: { + design: { + type: 'figma', + url: 'https://www.figma.com/design/r2nqdNNXXZtPmWuVjIlM1Q/iX-Components---Brand-Dark?node-id=225-5535&m=dev', + }, + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Required: Story = { + args: { + label: 'Required', + value: '', + required: true, + }, +}; diff --git a/packages/storybook-docs/src/stories/input-number.stories.ts b/packages/storybook-docs/src/stories/input-number.stories.ts index 12fc8fc0782..5448ea45c0e 100644 --- a/packages/storybook-docs/src/stories/input-number.stories.ts +++ b/packages/storybook-docs/src/stories/input-number.stories.ts @@ -13,7 +13,7 @@ import { genericRender, makeArgTypes } from './utils/generic-render'; type Element = Components.IxNumberInput; const meta = { - title: 'Example/Input', + title: 'Example/Input/Number', tags: [], render: (args) => genericRender('ix-number-input', args), argTypes: makeArgTypes>>('ix-number-input', {}), @@ -28,9 +28,16 @@ const meta = { export default meta; type Story = StoryObj; -export const Number: Story = { +export const Default: Story = { args: { value: 1337, showStepperButtons: true, }, }; + +export const Required: Story = { + args: { + required: true, + label: 'Required', + }, +}; diff --git a/packages/storybook-docs/src/stories/input.stories.ts b/packages/storybook-docs/src/stories/input.stories.ts new file mode 100644 index 00000000000..e82685a8621 --- /dev/null +++ b/packages/storybook-docs/src/stories/input.stories.ts @@ -0,0 +1,37 @@ +/* + * SPDX-FileCopyrightText: 2024 Siemens AG + * + * SPDX-License-Identifier: MIT + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +import type { Components } from '@siemens/ix/components'; +import type { ArgTypes, Meta, StoryObj } from '@storybook/web-components'; +import { genericRender, makeArgTypes } from './utils/generic-render'; + +type Element = Components.IxInput; + +const meta = { + title: 'Example/Input/Text', + tags: [], + render: (args) => genericRender('ix-input', args), + argTypes: makeArgTypes>>('ix-input', {}), + parameters: { + design: { + type: 'figma', + url: 'https://www.figma.com/design/r2nqdNNXXZtPmWuVjIlM1Q/iX-Components---Brand-Dark?node-id=225-5535&m=dev', + }, + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Required: Story = { + args: { + label: 'Required', + value: '', + required: true, + }, +}; diff --git a/packages/storybook-docs/src/stories/select.stories.ts b/packages/storybook-docs/src/stories/select.stories.ts index 2ec24ec80e7..3ed44b5db2a 100644 --- a/packages/storybook-docs/src/stories/select.stories.ts +++ b/packages/storybook-docs/src/stories/select.stories.ts @@ -79,3 +79,10 @@ export const editable_with_dropdown_width: Story = { dropdownMaxWidth: '25rem', }, }; + +export const Required: Story = { + args: { + required: true, + label: 'Required', + }, +}; From e3ca01b6f148da88df80d234287343d6d5b9c372 Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Fri, 7 Feb 2025 14:23:50 +0100 Subject: [PATCH 02/10] test: adapt test --- .../core/src/components/date-input/tests/date-input.ct.ts | 4 ++++ .../src/components/field-label/tests/field-label.ct.ts | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/packages/core/src/components/date-input/tests/date-input.ct.ts b/packages/core/src/components/date-input/tests/date-input.ct.ts index e6654c51bd2..48454f6abff 100644 --- a/packages/core/src/components/date-input/tests/date-input.ct.ts +++ b/packages/core/src/components/date-input/tests/date-input.ct.ts @@ -128,12 +128,16 @@ test('select date by input with invalid date - i18n', async ({ test('required', async ({ mount, page }) => { await mount(``); const dateInputElement = page.locator('ix-date-input'); + const input = dateInputElement.locator('input'); await expect(dateInputElement).toHaveAttribute('required'); await expect(dateInputElement.locator('ix-field-label')).toHaveText( 'MyLabel *' ); + await input.focus(); + await input.blur(); + await expect(dateInputElement).toHaveClass(/ix-invalid--required/); }); diff --git a/packages/core/src/components/field-label/tests/field-label.ct.ts b/packages/core/src/components/field-label/tests/field-label.ct.ts index c15513935eb..f5204d977ce 100644 --- a/packages/core/src/components/field-label/tests/field-label.ct.ts +++ b/packages/core/src/components/field-label/tests/field-label.ct.ts @@ -135,8 +135,11 @@ test('invalid color with invalid text field', async ({ mount, page }) => { `); const fieldElement = page.locator('ix-input'); + const inputElement = fieldElement.locator('input'); const labelElement = page.locator('ix-field-label'); + await inputElement.focus(); + await inputElement.blur(); await expect(fieldElement).toHaveClass(/ix-invalid--required/); await expect(labelElement.locator('ix-typography')).toHaveAttribute( @@ -167,8 +170,12 @@ test('invalid color with invalid textarea field', async ({ mount, page }) => { `); const fieldElement = page.locator('ix-textarea'); + const textareaElement = fieldElement.locator('textarea'); const labelElement = page.locator('ix-field-label'); + await textareaElement.focus(); + await textareaElement.blur(); + await expect(fieldElement).toHaveClass(/ix-invalid--required/); await expect(labelElement.locator('ix-typography')).toHaveAttribute( From 3fac62087283f09a958e3d4f5589bad2429b0b0c Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Fri, 7 Feb 2025 14:45:44 +0100 Subject: [PATCH 03/10] test: reduce flaky test --- .../core/src/components/field-label/tests/field-label.ct.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/field-label/tests/field-label.ct.ts b/packages/core/src/components/field-label/tests/field-label.ct.ts index f5204d977ce..6dbe97d0bd4 100644 --- a/packages/core/src/components/field-label/tests/field-label.ct.ts +++ b/packages/core/src/components/field-label/tests/field-label.ct.ts @@ -136,12 +136,14 @@ test('invalid color with invalid text field', async ({ mount, page }) => { const fieldElement = page.locator('ix-input'); const inputElement = fieldElement.locator('input'); + await expect(inputElement).toBeVisible(); + const labelElement = page.locator('ix-field-label'); await inputElement.focus(); await inputElement.blur(); - await expect(fieldElement).toHaveClass(/ix-invalid--required/); + await expect(fieldElement).toHaveClass(/ix-invalid--required/); await expect(labelElement.locator('ix-typography')).toHaveAttribute( 'style', 'color: var(--theme-color-alarm-text);' @@ -171,6 +173,8 @@ test('invalid color with invalid textarea field', async ({ mount, page }) => { const fieldElement = page.locator('ix-textarea'); const textareaElement = fieldElement.locator('textarea'); + await expect(textareaElement).toBeVisible(); + const labelElement = page.locator('ix-field-label'); await textareaElement.focus(); From 80db45893632bfb4e91fe8dbb7dc1bfc8950a4bc Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Wed, 12 Feb 2025 17:32:40 +0100 Subject: [PATCH 04/10] fix: avoid any cast --- .../angular/src/control-value-accessors/value-accessor.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/angular/src/control-value-accessors/value-accessor.ts b/packages/angular/src/control-value-accessors/value-accessor.ts index 1d0fa6871a0..0973087229e 100644 --- a/packages/angular/src/control-value-accessors/value-accessor.ts +++ b/packages/angular/src/control-value-accessors/value-accessor.ts @@ -114,7 +114,7 @@ export class ValueAccessor } detourFormControlMethods(ngControl: NgControl, elementRef: ElementRef) { - const formControl = ngControl.control as any; + const formControl = ngControl.control; if (formControl) { const methodsToPatch = [ 'markAsTouched', @@ -122,7 +122,7 @@ export class ValueAccessor 'markAsUntouched', 'markAsDirty', 'markAsPristine', - ]; + ] as const; methodsToPatch.forEach((method) => { if (typeof formControl[method] !== 'undefined') { const oldFn = formControl[method].bind(formControl); From 4041baffc03335fb73cb0ad90645d22f20d1f3e2 Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Wed, 12 Feb 2025 17:34:04 +0100 Subject: [PATCH 05/10] fix: place skip validation check before required check --- packages/core/src/components/utils/input/validation.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/components/utils/input/validation.ts b/packages/core/src/components/utils/input/validation.ts index f7b5b19dd6f..c3b85cd9709 100644 --- a/packages/core/src/components/utils/input/validation.ts +++ b/packages/core/src/components/utils/input/validation.ts @@ -112,6 +112,11 @@ export function HookValidationLifecycle(options?: { ) as unknown as HTMLIxFormComponentElement; checkIfRequiredFunction = async () => { + const skipValidation = await shouldSuppressInternalValidation(host); + if (skipValidation) { + return; + } + if (host.hasValidValue && typeof host.hasValidValue === 'function') { const hasValue = await host.hasValidValue(); const touched = await isTouched(host); @@ -122,11 +127,6 @@ export function HookValidationLifecycle(options?: { } } - const skipValidation = await shouldSuppressInternalValidation(host); - if (skipValidation) { - return; - } - if ( host.getValidityState && typeof host.getValidityState === 'function' From 218f7cb1f0401df1126e07991144903e84f0ba0a Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Thu, 13 Feb 2025 08:07:19 +0100 Subject: [PATCH 06/10] docs(storybook): add story --- packages/storybook-docs/src/stories/textarea.stories.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/storybook-docs/src/stories/textarea.stories.ts b/packages/storybook-docs/src/stories/textarea.stories.ts index 97deda6bbe8..1cae5949e12 100644 --- a/packages/storybook-docs/src/stories/textarea.stories.ts +++ b/packages/storybook-docs/src/stories/textarea.stories.ts @@ -28,8 +28,14 @@ const meta = { export default meta; type Story = StoryObj; -// More on writing stories with args: https://storybook.js.org/docs/writing-stories/args export const Default: Story = { + args: {}, +}; + +export const Required: Story = { args: { + label: 'Required', + value: '', + required: true, }, }; From 1465f949496b70cc09874c934c9c5b76bc7c5bd7 Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Thu, 13 Feb 2025 12:16:25 +0100 Subject: [PATCH 07/10] fix: implement default behaviour of inital required validation --- .../input-form-validation.html | 7 +++++- .../preview-examples/input-form-validation.ts | 2 +- .../boolean-value-accessor.ts | 2 +- .../date-value-accessor.ts | 2 +- .../select-value-accessor.ts | 2 +- .../text-value-accessor.ts | 2 +- .../control-value-accessors/value-accessor.ts | 23 ++++++++++++++++--- 7 files changed, 31 insertions(+), 9 deletions(-) diff --git a/packages/angular-test-app/src/preview-examples/input-form-validation.html b/packages/angular-test-app/src/preview-examples/input-form-validation.html index 8e4002b66ac..2132589be6a 100644 --- a/packages/angular-test-app/src/preview-examples/input-form-validation.html +++ b/packages/angular-test-app/src/preview-examples/input-form-validation.html @@ -6,12 +6,17 @@ This source code is licensed under the MIT license found in the LICENSE file in the root directory of this source tree. --> -
+ Submit diff --git a/packages/angular-test-app/src/preview-examples/input-form-validation.ts b/packages/angular-test-app/src/preview-examples/input-form-validation.ts index b35590f05de..fb28e61955a 100644 --- a/packages/angular-test-app/src/preview-examples/input-form-validation.ts +++ b/packages/angular-test-app/src/preview-examples/input-form-validation.ts @@ -16,7 +16,7 @@ import { FormControl, FormGroup, Validators } from '@angular/forms'; }) export default class InputFormValidation { exampleForm = new FormGroup({ - name: new FormControl('', [Validators.required]), + name: new FormControl('', [Validators.required, Validators.minLength(4)]), }); submit() { diff --git a/packages/angular/src/control-value-accessors/boolean-value-accessor.ts b/packages/angular/src/control-value-accessors/boolean-value-accessor.ts index c19033bf0c5..dfdce7c4d2c 100644 --- a/packages/angular/src/control-value-accessors/boolean-value-accessor.ts +++ b/packages/angular/src/control-value-accessors/boolean-value-accessor.ts @@ -22,7 +22,7 @@ import { ValueAccessor } from './value-accessor'; }) export class BooleanValueAccessorDirective extends ValueAccessor { constructor(injector: Injector, el: ElementRef) { - super(injector, el); + super(injector, el, true); } override writeValue(value: boolean): void { diff --git a/packages/angular/src/control-value-accessors/date-value-accessor.ts b/packages/angular/src/control-value-accessors/date-value-accessor.ts index 5e8b961bb11..9d4fc0d7863 100644 --- a/packages/angular/src/control-value-accessors/date-value-accessor.ts +++ b/packages/angular/src/control-value-accessors/date-value-accessor.ts @@ -22,7 +22,7 @@ import { ValueAccessor } from './value-accessor'; }) export class DateValueAccessorDirective extends ValueAccessor { constructor(injector: Injector, el: ElementRef) { - super(injector, el); + super(injector, el, true); } @HostListener('valueChange', ['$event.target']) diff --git a/packages/angular/src/control-value-accessors/select-value-accessor.ts b/packages/angular/src/control-value-accessors/select-value-accessor.ts index 0378eed4986..8aff4043359 100644 --- a/packages/angular/src/control-value-accessors/select-value-accessor.ts +++ b/packages/angular/src/control-value-accessors/select-value-accessor.ts @@ -22,7 +22,7 @@ import { ValueAccessor } from './value-accessor'; }) export class SelectValueAccessorDirective extends ValueAccessor { constructor(injector: Injector, el: ElementRef) { - super(injector, el); + super(injector, el, true); } @HostListener('valueChange', ['$event.target']) diff --git a/packages/angular/src/control-value-accessors/text-value-accessor.ts b/packages/angular/src/control-value-accessors/text-value-accessor.ts index a403da590a4..5e7c1bf4b31 100644 --- a/packages/angular/src/control-value-accessors/text-value-accessor.ts +++ b/packages/angular/src/control-value-accessors/text-value-accessor.ts @@ -22,7 +22,7 @@ import { ValueAccessor } from './value-accessor'; }) export class TextValueAccessorDirective extends ValueAccessor { constructor(injector: Injector, el: ElementRef) { - super(injector, el); + super(injector, el, true); } @HostListener('input', ['$event.target']) diff --git a/packages/angular/src/control-value-accessors/value-accessor.ts b/packages/angular/src/control-value-accessors/value-accessor.ts index 0973087229e..11f092b73c7 100644 --- a/packages/angular/src/control-value-accessors/value-accessor.ts +++ b/packages/angular/src/control-value-accessors/value-accessor.ts @@ -35,7 +35,11 @@ export class ValueAccessor @Input() suppressClassMapping = false; - constructor(protected injector: Injector, protected elementRef: ElementRef) {} + constructor( + protected injector: Injector, + protected elementRef: ElementRef, + private checkRequiredValidator = false + ) {} writeValue(value: any): void { this.elementRef.nativeElement.value = this.lastValue = value; @@ -139,10 +143,11 @@ export class ValueAccessor if (this.suppressClassMapping) { return; } - setTimeout(async () => { - const input = element.nativeElement; + const input = element.nativeElement; + setTimeout(async () => { const classes = this.getClasses(input); + const classList = input.classList; classList.remove( 'ix-valid', @@ -153,6 +158,18 @@ export class ValueAccessor 'ix-pristine' ); classList.add(...classes); + + const ngControl = this.getAssignedNgControl(); + if (ngControl && this.checkRequiredValidator) { + const { errors, touched } = ngControl; + + const hasOtherErrors = errors && Object.keys(errors).length > 1; + const isRequiredButUntouched = errors?.required && !touched; + + if (hasOtherErrors === false && isRequiredButUntouched) { + input.classList.remove('ix-invalid'); + } + } }); } From f60cbde307ea72a82f4cd2278314f576cdcb1fef Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Thu, 13 Feb 2025 14:52:18 +0100 Subject: [PATCH 08/10] fix: add tests --- .../src/test/input/form-ix-input-required.ts | 33 +++++ .../input/form-ix-input-with-validators.ts | 26 ++++ .../test/input/input-required-check.spec.ts | 130 ++++++++++++++++++ .../src/test/utils/wait-for-hydration.ts | 21 ++- 4 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 packages/angular-test-app/src/test/input/form-ix-input-required.ts create mode 100644 packages/angular-test-app/src/test/input/form-ix-input-with-validators.ts create mode 100644 packages/angular-test-app/src/test/input/input-required-check.spec.ts diff --git a/packages/angular-test-app/src/test/input/form-ix-input-required.ts b/packages/angular-test-app/src/test/input/form-ix-input-required.ts new file mode 100644 index 00000000000..0b603b4583d --- /dev/null +++ b/packages/angular-test-app/src/test/input/form-ix-input-required.ts @@ -0,0 +1,33 @@ +/* + * SPDX-FileCopyrightText: 2025 Siemens AG + * + * SPDX-License-Identifier: MIT + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { FormControl, FormGroup, Validators } from '@angular/forms'; + +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-input-form-required', + template: ` + + + + + `, +}) +export class TestInputFormRequired { + public form = new FormGroup({ + name: new FormControl('', Validators.compose([Validators.required])), + }); +} diff --git a/packages/angular-test-app/src/test/input/form-ix-input-with-validators.ts b/packages/angular-test-app/src/test/input/form-ix-input-with-validators.ts new file mode 100644 index 00000000000..3fc9b107889 --- /dev/null +++ b/packages/angular-test-app/src/test/input/form-ix-input-with-validators.ts @@ -0,0 +1,26 @@ +/* + * SPDX-FileCopyrightText: 2025 Siemens AG + * + * SPDX-License-Identifier: MIT + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { FormControl, FormGroup, Validators } from '@angular/forms'; + +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-input-form-with-validators', + template: ` +
+ +
+ `, +}) +export class TestInputFormWithValidators { + public form = new FormGroup({ + name: new FormControl('', [Validators.required, Validators.minLength(4)]), + }); +} diff --git a/packages/angular-test-app/src/test/input/input-required-check.spec.ts b/packages/angular-test-app/src/test/input/input-required-check.spec.ts new file mode 100644 index 00000000000..6848cb415f5 --- /dev/null +++ b/packages/angular-test-app/src/test/input/input-required-check.spec.ts @@ -0,0 +1,130 @@ +import { FormsModule, ReactiveFormsModule } from '@angular/forms'; + +import { ApplicationInitStatus, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; +import { + ComponentFixture, + fakeAsync, + TestBed, + tick, + flush, +} from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; +import { IxModule } from '@siemens/ix-angular'; +import { waitForHydration } from '../utils/wait-for-hydration'; +import { TestInputFormRequired } from './form-ix-input-required'; +import { TestInputFormWithValidators } from './form-ix-input-with-validators'; + +describe('ix-input required', () => { + let component: TestInputFormRequired; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [TestInputFormRequired, TestInputFormWithValidators], + schemas: [], + imports: [FormsModule, ReactiveFormsModule, IxModule.forRoot()], + }).compileComponents(); + + fixture = TestBed.createComponent(TestInputFormRequired); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + beforeEach(async () => { + // until https://github.com/angular/angular/issues/24218 is fixed + await TestBed.inject(ApplicationInitStatus).donePromise; + }); + + it('show no invalid state after first validation', fakeAsync(() => { + const input = fixture.debugElement.query( + By.css('ix-input[formControlName="name"]') + ); + + fixture.detectChanges(); + tick(100); + flush(); + + fixture.detectChanges(); + + expect(component).toBeDefined(); + expect(input.nativeElement.classList).not.toContain('ix-invalid'); + + input.nativeElement.dispatchEvent(new Event('ixBlur')); + fixture.detectChanges(); + + tick(100); + flush(); + expect(input.nativeElement.classList).toContain('ix-invalid'); + + component.form.get('name')!.setValue('test'); + input.nativeElement.dispatchEvent(new Event('ixBlur')); + fixture.detectChanges(); + + tick(100); + flush(); + expect(input.nativeElement.classList).not.toContain('ix-invalid'); + })); +}); + +describe('ix-input required + additional validators', () => { + let component: TestInputFormWithValidators; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [TestInputFormRequired, TestInputFormWithValidators], + schemas: [], + imports: [FormsModule, ReactiveFormsModule, IxModule.forRoot()], + }).compileComponents(); + + fixture = TestBed.createComponent(TestInputFormWithValidators); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + beforeEach(async () => { + // until https://github.com/angular/angular/issues/24218 is fixed + await TestBed.inject(ApplicationInitStatus).donePromise; + }); + + it('should show invalid state if another validator is added', fakeAsync(() => { + const input = fixture.debugElement.query( + By.css('ix-input[formControlName="name"]') + ); + + fixture.detectChanges(); + tick(100); + flush(); + + fixture.detectChanges(); + + expect(component).toBeDefined(); + // Validators.required = true, but not showing invalid state, because the input is not touched + expect(input.nativeElement.classList).not.toContain('ix-invalid'); + + input.nativeElement.dispatchEvent(new Event('ixBlur')); + fixture.detectChanges(); + + tick(100); + flush(); + // Validators.required = true + expect(input.nativeElement.classList).toContain('ix-invalid'); + + component.form.get('name')!.setValue('1'); + input.nativeElement.dispatchEvent(new Event('ixBlur')); + fixture.detectChanges(); + + tick(100); + flush(); + // Validators.minLength = true + expect(input.nativeElement.classList).toContain('ix-invalid'); + + component.form.get('name')!.setValue('1234'); + input.nativeElement.dispatchEvent(new Event('ixBlur')); + fixture.detectChanges(); + + tick(100); + flush(); + expect(input.nativeElement.classList).not.toContain('ix-invalid'); + })); +}); diff --git a/packages/angular-test-app/src/test/utils/wait-for-hydration.ts b/packages/angular-test-app/src/test/utils/wait-for-hydration.ts index 92fa7c480fd..47b4ebe015a 100644 --- a/packages/angular-test-app/src/test/utils/wait-for-hydration.ts +++ b/packages/angular-test-app/src/test/utils/wait-for-hydration.ts @@ -1,22 +1,19 @@ export function waitForHydration( element: HTMLElement, - timeout = 5000 + timeout = 1000 ): Promise { return new Promise((resolve, reject) => { - const interval = 50; - let elapsedTime = 0; - - const checkClass = () => { + const interval = setInterval(() => { if (element.classList.contains('hydrated')) { + clearInterval(interval); + clearTimeout(timeoutId); resolve(); - } else if (elapsedTime >= timeout) { - reject(new Error(`Timeout waiting for hydration`)); - } else { - elapsedTime += interval; - setTimeout(checkClass, interval); } - }; + }, 50); - checkClass(); + const timeoutId = setTimeout(() => { + clearInterval(interval); + reject(new Error('Timeout waiting for hydration')); + }, timeout); }); } From 75d0f8bf302a7a03940466467d10c606c5e6f1c8 Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Thu, 13 Feb 2025 14:59:05 +0100 Subject: [PATCH 09/10] add changeset section --- .changeset/six-wombats-grow.md | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/.changeset/six-wombats-grow.md b/.changeset/six-wombats-grow.md index c2479468edb..ab38a7ccfb1 100644 --- a/.changeset/six-wombats-grow.md +++ b/.changeset/six-wombats-grow.md @@ -7,14 +7,9 @@ Add `suppressClassMapping` to value-accessors to prevent that the accessors auto If `[suppressClassMapping]="true"` you need to control the `ix-`-classes on your own. ```html - - + ``` +`value-accessor` ignores NgControls which are untouched but has `required=true` errors + Fixes #1638 #1680 From 9db50b0c7293c96e1dc7bce3d073ee593147ab9b Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Thu, 13 Feb 2025 15:00:21 +0100 Subject: [PATCH 10/10] remove default example --- .../src/preview-examples/input-form-validation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/angular-test-app/src/preview-examples/input-form-validation.ts b/packages/angular-test-app/src/preview-examples/input-form-validation.ts index fb28e61955a..b35590f05de 100644 --- a/packages/angular-test-app/src/preview-examples/input-form-validation.ts +++ b/packages/angular-test-app/src/preview-examples/input-form-validation.ts @@ -16,7 +16,7 @@ import { FormControl, FormGroup, Validators } from '@angular/forms'; }) export default class InputFormValidation { exampleForm = new FormGroup({ - name: new FormControl('', [Validators.required, Validators.minLength(4)]), + name: new FormControl('', [Validators.required]), }); submit() {