Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: avoid internal validation before interacting with input #1682

Merged
merged 17 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/six-wombats-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@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
<ix-input label="Name:" formControlName="name" [suppressClassMapping]="true" [class.ix-invalid]="!form.get('name')!.valid && form.get('name')!.touched" required> </ix-input>
```

`value-accessor` ignores NgControls which are untouched but has `required=true` errors
nuke-ellington marked this conversation as resolved.
Show resolved Hide resolved

Fixes #1638 #1680
9 changes: 9 additions & 0 deletions .changeset/stale-ladybugs-wave.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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.
-->
<form class="form-validation-example" [formGroup]="exampleForm" (ngSubmit)="submit()">
<form
class="form-validation-example"
[formGroup]="exampleForm"
(ngSubmit)="submit()"
>
<ix-input
label="Name:"
helperText="Write down your name"
invalidText="Value is required"
formControlName="name"
required
danielleroux marked this conversation as resolved.
Show resolved Hide resolved
>
</ix-input>
<ix-button type="submit">Submit</ix-button>
Expand Down
33 changes: 33 additions & 0 deletions packages/angular-test-app/src/test/input/form-ix-input-required.ts
Original file line number Diff line number Diff line change
@@ -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: `
<form class="form-validation-example" [formGroup]="form">
<ix-input
label="Name:"
helperText="Write down your name"
invalidText="Value is required"
formControlName="name"
required
danielleroux marked this conversation as resolved.
Show resolved Hide resolved
>
</ix-input>
</form>
`,
})
export class TestInputFormRequired {
public form = new FormGroup({
name: new FormControl('', Validators.compose([Validators.required])),
danielleroux marked this conversation as resolved.
Show resolved Hide resolved
});
}
Original file line number Diff line number Diff line change
@@ -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: `
<form [formGroup]="form">
<ix-input formControlName="name" [required]="true"> </ix-input>
danielleroux marked this conversation as resolved.
Show resolved Hide resolved
</form>
`,
})
export class TestInputFormWithValidators {
public form = new FormGroup({
name: new FormControl('', [Validators.required, Validators.minLength(4)]),
});
}
130 changes: 130 additions & 0 deletions packages/angular-test-app/src/test/input/input-required-check.spec.ts
Original file line number Diff line number Diff line change
@@ -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<TestInputFormRequired>;

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<TestInputFormRequired>;

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');
}));
});
21 changes: 9 additions & 12 deletions packages/angular-test-app/src/test/utils/wait-for-hydration.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
export function waitForHydration(
element: HTMLElement,
timeout = 5000
timeout = 1000
): Promise<void> {
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);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -22,12 +22,12 @@ import { ValueAccessor, mapNgToIxClassNames } 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 {
this.elementRef.nativeElement.checked = this.lastValue = value;
mapNgToIxClassNames(this.elementRef);
super.mapNgToIxClassNames(this.elementRef);
}

@HostListener('checkedChange', ['$event.target'])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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'])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand Down
Loading
Loading