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

Validation Flaws in IxInput: Immediate Error Display and Documentation Workaround Problems #1638

Open
2 tasks done
Aiderlei opened this issue Jan 9, 2025 · 1 comment
Open
2 tasks done
Labels
triage We discuss this topic in our internal weekly

Comments

@Aiderlei
Copy link

Aiderlei commented Jan 9, 2025

Prerequisites

  • I have read the Contributing Guidelines.
  • I have not leaked any internal/restricted information like screenshots, videos, code snippets, links etc.

Bug Report: Validation Flaws in IxInput: Immediate Error Display and Documentation Workaround Problems

Description

I have tested the new IxInput elements and discovered that the required form validation is displayed immediately upon the first render.

This behavior is problematic because it does not allow for customization of validation behavior in forms, which is possible in pure Angular or other libraries. Typically, developers can customize when validation messages appear, such as by rendering them only after a field is touched or dirty.

Comparison with Angular Material

For reference, Angular Material provides an errorStateMatcher to tweak validation behavior. By default, Angular Material only displays validation messages when a field is dirty or touched. This provides better control and user experience.

Problems with Suggested Workaround

The suggested workaround in the documentation introduces additional issues:

  1. Incorrect Validation State:

    • A form with only empty fields, when using the custom required validator from the documentation, will incorrectly display the form as valid, making it submittable even though it is invalid.
  2. Misleading Feedback Messages:

    • When using the "valid" feedback message, the form displays valid feedback for an empty field on initial render. After touching the field and leaving it empty, it then shows the invalid feedback message.
  3. Buggy Behavior in Example Code:

    • The example in the documentation is flawed because the condition !control.untouched leads to unexpected behavior. For example:
      • Typing "xxxx" into the field, deleting it, and tabbing out of the field does not show a validation error message. This leaves the field incorrectly marked as valid.

Adjusting the CustomRequiredValidator to rely on !pristine is also insufficient, and conditionally setting validation messages does not resolve the issue, as it still displays incorrect valid feedback (refer to my repository for more details).

Expected Behavior

The required validation messages for IxInput should:

  • Only display after the field has been touched or dirty, similar to how Angular Material handles this behavior.
  • Allow customization of the validation display logic, such as with an equivalent to Angular Material's errorStateMatcher.

Suggested Solution

  • Introduce good validation behavior per default (as in Angular Material)
  • Introduce a global config mechanism to control when validation messages are displayed (e.g., after the field becomes touched or dirty).
  • Provide a customizable validation behavior similar to Angular Material's errorStateMatcher.

What type of frontend framework are you seeing the problem on?

Angular

Which version of iX do you use?

v2.6.1

Code to produce this issue.

I have created a github repository linked to stackblitz in the README to demonstrate these behaviors.

The demo shows four different examples of required fields, these are:

  1. First Form: Demonstrates how the required field validation is supposed to work.
  2. Second Form: Uses Angular's RequiredValidator with the required HTML attribute. This demonstrates that IxInput is broken for the RequiredValidator.
  3. Third Form: Implements the exact example from the documentation using the CustomRequiredValidator. This highlights that the suggested solution from the documentation is flawed, as it allows submission of an empty form and shows incorrect valid feedback.
  4. Fourth Form: Uses a slightly modified CustomRequiredValidator from the documentation. This shows that adjusting the CustomRequiredValidator to rely on !pristine is insufficient and that setting validation messages conditionally does not resolve the issue.
@Aiderlei Aiderlei added the triage We discuss this topic in our internal weekly label Jan 9, 2025
@Aiderlei Aiderlei changed the title Initial Required Form Validation Displays on Render and Doc's Custom Validator Allows Empty Submission Validation Flaws in IxInput: Immediate Error Display and Documentation Workaround Problems Jan 9, 2025
@lucasluizss
Copy link

I'd like to confirm the issue and propose a solution.

The current implementation of the ValueAccessor directive in /packages/angular/src/control-value-accessors/value-accessor.ts might trigger early validation due to the way class names are mapped based on the form control state in the ngAfterViewInit lifecycle hook.

I propose the following solution:

  • Modify writeValue: The writeValue method should only call mapNgToIxClassNames if the provided value is different from the lastValue. This ensures that class names are updated only when the actual value of the input element changes.

Here's the modified code:

...

@Directive()
export class ValueAccessor
  implements ControlValueAccessor, AfterViewInit, OnDestroy
{
  public static readonly ANGULAR_CLASS_PREFIX = 'ng-';

  private onChange: (value: any) => void = () => {
    /**/
  };
  private onTouched: () => void = () => {
    /**/
  };
  protected lastValue: any;
  private statusChanges?: Subscription;

  constructor(protected injector: Injector, protected elementRef: ElementRef) {}

  writeValue(value: any): void {
    this.elementRef.nativeElement.value = this.lastValue = value;
    // Call mapNgToIxClassNames only when the value actually changes
    if (value !== this.lastValue) {
      mapNgToIxClassNames(this.elementRef);
    }
  }

  handleValueChange(el: HTMLElement, value: any): void {
    if (el === this.elementRef.nativeElement) {
      if (value !== this.lastValue) {
        this.lastValue = value;
        this.onChange(value);
        mapNgToIxClassNames(this.elementRef);
      }
    }
  }

...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage We discuss this topic in our internal weekly
Projects
None yet
Development

No branches or pull requests

2 participants