Add logic to allow warnings for input fields

[Exitare]

Does address but does not close #3236. Tests are failing, because I dont want to fix them before this idea is either accepted or denied. Right now, this is only working for the creature template max level field.

Feedback is welcome

[FrancescoBorzi]

Build failing

[Exitare]

@FrancescoBorzi Works on my end. Can you please provide more information ?

[Helias]

We have a lint rule that notifies if a component is not using the OnPush strategy, it's complaining about it

This is the issue reported by the pipeline

[Exitare]

Thank you, but the failed test is intentional. I would love to hear some feedback on my proposed solution before I fix any tests that will break due to the change. If the solution is rejected I didn't waste too much time :) Hope that makes sense!

[Helias]

ok, I will test this PR locally and let you know ;-)

[Exitare]

Thank you :)

[Exitare]

I also would like to mention, that this will NOT address any editors. The example I added for the maxLevel in creature template is just for showcasing purposes of the proposed solution. The PR aims to provide an easy framework to add form validation, not to add form validation everywhere it is needed. This needs to happen gradually, otherwise this will be a gigantic PR, considering Item Template and Creature Template have to change along with a lot of other forms.

[Helias]

I don't dislike the approach but I am wondering if can be done only with:

              <input
                keiraInputValidation
                [formControlName]="'maxlevel'"
                id="maxlevel"
                type="number"
                class="form-control form-control-sm"
              />

Without requiring any other information or adding components, because may you can find a way to get the formcontrol from the input and you could spawn dynamically a component next to the without already writing a placeholder component <keira-validation-feedback>. If this is not feasible without using the <keira-validation-feedback> component and the input parameter [keiraInputValidation]="editorService.form.get('maxlevel')" is mandatory, then I can approve this approach.

Thanks by the way for implementing this proof of concept of warnings, it's a good job!

[Exitare]

Thank you for providing feedback. I will try to make it work. I also had a solution in mind that minimizes the changes required to all editors as this can introduce quite the overhead in work. I will explore possible solutions and hopefully something will work :)

[Exitare]

In theory I could do something like this:

import { Directive, ViewContainerRef } from '@angular/core';

@Directive({
  selector: '[dynamicComponentHost]',
})
export class DynamicComponentHostDirective {
  constructor(public viewContainerRef: ViewContainerRef) {}
}

import { Component, ComponentFactoryResolver, OnInit } from '@angular/core';
import { KeiraValidationFeedbackComponent } from './keira-validation-feedback.component';
import { DynamicComponentHostDirective } from './dynamic-component-host.directive';

@Component({
  selector: 'app-dynamic-form',
  template: `
    <div class="form-group col-12 col-sm-6 col-md-4 col-lg-3 col-xl-2">
      <label for="maxLevel">Max Level</label>
      <input
        [keiraInputValidation]="editorService.form.get('maxlevel')"
        [formControlName]="'maxlevel'"
        id="maxlevel"
        type="number"
        class="form-control form-control-sm"
        (blur)="showValidationFeedback()"
      />
      <ng-template dynamicComponentHost></ng-template>
    </div>
  `,
})
export class DynamicFormComponent implements OnInit {
  @ViewChild(DynamicComponentHostDirective, { static: true })
  dynamicHost!: DynamicComponentHostDirective;

  constructor(
    private resolver: ComponentFactoryResolver,
    public editorService: EditorService
  ) {}

  ngOnInit(): void {}

  showValidationFeedback(): void {
    const control = this.editorService.form.get('maxlevel');
    if (control?.invalid && control.touched) {
      const viewContainerRef = this.dynamicHost.viewContainerRef;
      viewContainerRef.clear(); // Clear any existing components
      const componentFactory =
        this.resolver.resolveComponentFactory(KeiraValidationFeedbackComponent);
      const componentRef = viewContainerRef.createComponent(componentFactory);
      componentRef.instance.control = control; // Pass the control to the component
    }
  }
}

Used like this:

<div class="form-group col-12 col-sm-6 col-md-4 col-lg-3 col-xl-2">
  <label for="maxLevel">Max Level</label>
  <input
    [keiraInputValidation]="editorService.form.get('maxlevel')"
    [formControlName]="'maxlevel'"
    id="maxlevel"
    type="number"
    class="form-control form-control-sm"
    (blur)="showValidationFeedback()"
  />
  <ng-template dynamicComponentHost></ng-template>
</div>

But personally, I think this is a lot of boilerplate code and super convoluted too, for a worse outcome in terms of readability and most likely maintainability too.

If you have a better solution or something I should check out, please let me know :)

[Helias]

I was convinced that everything can be implemented inside the directive attribute making it agnostic and generic for all inputs without requiring any input information.

import { Directive, ElementRef, inject, OnInit, Renderer2 } from '@angular/core';
import { AbstractControl, NgControl } from '@angular/forms';
import { SubscriptionHandler } from '@keira/shared/utils';

@Directive({
  selector: '[keiraInputValidation]',
  standalone: true,
})
export class InputValidationDirective extends SubscriptionHandler implements OnInit {
  private readonly el: ElementRef = inject(ElementRef);
  private readonly renderer: Renderer2 = inject(Renderer2);
  private readonly ngControl: NgControl = inject(NgControl);

  private errorDiv: HTMLElement | null = null;

  ngOnInit(): void {
    const control = this.ngControl.control;

    if (!control) {
      return;
    }

    this.subscriptions.push(
      control.statusChanges?.subscribe(() => {
        this.updateErrorMessage(control);
      }),
    );
  }

  private updateErrorMessage(control: AbstractControl): void {
    if (this.errorDiv) {
      this.renderer.removeChild(this.el.nativeElement.parentNode, this.errorDiv);
      this.errorDiv = null;
    }

    if (control?.touched && control?.invalid) {
      const errorMessage = control?.errors?.['required'] ? 'This field is required' : 'Invalid field';

      this.errorDiv = this.renderer.createElement('div');

      const text = this.renderer.createText(errorMessage);
      this.renderer.appendChild(this.errorDiv, text);

      const parent = this.el.nativeElement.parentNode;
      this.renderer.appendChild(parent, this.errorDiv);
    }
  }
}

<div class="form-group col-12 col-sm-6 col-md-4 col-lg-3 col-xl-2">
  <label for="maxLevel">Max Level</label>
  <input keiraInputValidation [formControlName]="'maxlevel'" id="maxlevel" type="number" class="form-control form-control-sm" />
</div>

This will allow us to add the directive keiraInputValidation to any input without further boilerplate or implementation, moreover, now the directive will be self-contained and it does not require any other helper component like the keira3-validation-feedback. I am not a fan of the Renderer2 service but if we need to manipulate the DOM it's the solution for Angular.

You can find the commit here https://github.com/azerothcore/Keira3/commit/7021a3a5f677c21d962817227c202b9ea517fe6a The branch name is warnings-directive

EDIT: this code is a draft based on your code, you can re-use it improving it if possible, we could continue the implementation in this branch/PR

[Exitare]

@Helias Wow! thank you. I wasn't aware of this possibility. That looks great. I will copy it over :)

[Exitare]

@Helias I added the directive and it works, with a twist. If the creature template is opened up the first time one can clear the field and put another field in focus (e.g. click on minlevel), the error message will not show up. One has to enter a value again and then delete it, to actually trigger the error message. I will look into this.

Additionally, if we add the validation, we are just one step shy of adding a subject that the keira edit button can subscribe to, to disable them if validation fails. What are your thoughts on that?

[Helias]

I noticed that, if we remove the control.touched contidion it should work on the first time too, I am not sure if it's 100% correct but we could remove it and then think if we really need that condition or not.

About the validation, that's a good idea actually 🚀

[Exitare]

I added a workaround, by adding markedAsTouched() when the field is invalid. But maybe thats not as good as a solution as it can be?

[Helias]

I added a workaround, by adding markedAsTocuched() when the field is invalid. But maybe thats not as good as a solution as it can be?

it could be in this case, let's keep it

[FrancescoBorzi]

@Exitare are you planning to complete this?

[Exitare]

Yes, I plan to. I will be able to spend more time on the project again in around 2 months. If thats too long, I am fine with closing the PR.

[FrancescoBorzi]

Yes, I plan to. I will be able to spend more time on the project again in around 2 months. If thats too long, I am fine with closing the PR.

no worries, we will wait for you!