Improved required fields

Ensures required field inputs show an asterisk and have correct ARIA markup. Also removes custom CSS from specific components. Fixes AB#553, AB#602, AB#607, AB#777, AB#875, AB#916, AB#986
Azure · Jan 27, 2023 · 07a4b1d · 07a4b1d
1 parent aa42afe
commit 07a4b1d
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 19 deletions.
diff --git a/src/@batch-flask/ui/form/form-field/form-field.html b/src/@batch-flask/ui/form/form-field/form-field.html
@@ -5,7 +5,7 @@
         [attr.for]="control.id"
         [attr.aria-owns]="control.id">
         {{control.placeholder}}
-        <sup *ngIf="control.required">
+        <sup class="required" *ngIf="control.required">
             <i class="fa fa-asterisk" aria-hidden="true"></i>
         </sup>
     </label>

diff --git a/src/@batch-flask/ui/form/form-field/form-field.scss b/src/@batch-flask/ui/form/form-field/form-field.scss
@@ -5,6 +5,13 @@ bl-form-field {
     .label-container {
         display: block;
         color: $primary-text;
+        sup.required {
+            line-height: 1em;
+            .fa-asterisk {
+                color: var(--color-danger);
+                font-size: 8px;
+            }
+        }
     }
 
     .input-container {
@@ -28,11 +35,6 @@ bl-form-field {
         // Having this opacity make contrast ratio too loos
         // opacity: 0.5;
     }
-
-    .fa-asterisk {
-        color: var(--color-danger);
-        font-size: 8px;
-    }
 }
 
 bl-form-field {

diff --git a/src/@batch-flask/ui/form/input/input.directive.spec.ts b/src/@batch-flask/ui/form/input/input.directive.spec.ts
@@ -1,6 +1,6 @@
 import { Component, DebugElement } from "@angular/core";
 import { ComponentFixture, TestBed } from "@angular/core/testing";
-import { FormControl, FormsModule, ReactiveFormsModule } from "@angular/forms";
+import { FormControl, FormsModule, ReactiveFormsModule, Validators } from "@angular/forms";
 import { By } from "@angular/platform-browser";
 import { InputDirective } from "./input.directive";
 
@@ -13,7 +13,7 @@ describe("InputDirective", () => {
 
     function createComponent(comp) {
         TestBed.configureTestingModule({
-            imports: [ FormsModule, ReactiveFormsModule],
+            imports: [FormsModule, ReactiveFormsModule],
             declarations: [InputDirective, comp],
         });
         fixture = TestBed.createComponent(comp);
@@ -64,6 +64,45 @@ describe("InputDirective", () => {
         });
     });
 
+    describe("when setting required", () => {
+        it("should mark as required based on form control validation", () => {
+            createComponent(BlInputWithFormControl);
+            const testComponent = fixture.componentInstance;
+
+            fixture.detectChanges();
+            expect(de.nativeElement.required).toBeFalsy();
+
+            testComponent.formControl.setValidators(Validators.required);
+            testComponent.formControl.updateValueAndValidity();
+            fixture.detectChanges();
+            expect(de.nativeElement.required).toBe(true);
+            expect(de.nativeElement.getAttribute("aria-required")).toBe("true");
+
+            testComponent.formControl.clearValidators();
+            fixture.detectChanges();
+            expect(de.nativeElement.required).toBe(false);
+            expect(de.nativeElement.getAttribute("aria-required"))
+                .toBeFalsy();
+        });
+        it("should mark as required based on attribute", () => {
+            createComponent(BlInputWithRequired);
+            const testComponent = fixture.componentInstance;
+
+            testComponent.required = true;
+            fixture.detectChanges();
+            expect(de.nativeElement.required).toBe(true);
+
+            // Attribute overrides form control validation
+            testComponent.required = false;
+            testComponent.formControl.setValidators(Validators.required);
+            testComponent.formControl.updateValueAndValidity();
+            fixture.detectChanges();
+            expect(de.nativeElement.required).toBe(false);
+            expect(de.nativeElement.getAttribute("aria-required"))
+                .toBeFalsy();
+        });
+    });
+
     describe("when using a form control", () => {
         let testComponent: BlInputWithFormControl;
 
@@ -99,6 +138,14 @@ class BlInputWithDisabled {
     public disabled: boolean;
 }
 
+@Component({
+    template: `<input blInput [required]="required">`,
+})
+class BlInputWithRequired {
+    public required: boolean;
+    public formControl = new FormControl();
+}
+
 @Component({
     template: `<input blInput [formControl]="formControl">`,
 })

diff --git a/src/@batch-flask/ui/form/input/input.directive.ts b/src/@batch-flask/ui/form/input/input.directive.ts
@@ -48,6 +48,8 @@ export class InputDirective implements FormFieldControl<any>, OnChanges, OnDestr
 
     public readonly stateChanges = new Subject<void>();
     public readonly controlType: string = "bl-input";
+    private _required: boolean;
+    private _requiredAttribute = false;
 
     @Input()
     @HostBinding("disabled")
@@ -70,7 +72,34 @@ export class InputDirective implements FormFieldControl<any>, OnChanges, OnDestr
     @HostBinding("attr.placeholder")
     public placeholder: string;
 
-    @Input() @FlagInput() @HostBinding("required") public required = false;
+    @Input() @FlagInput()
+    @HostBinding("required")
+    public get required(): boolean {
+        if (this._requiredAttribute) {
+            return this._required;
+        }
+        if (this.ngControl?.control?.validator) {
+            return this.ngControl.control
+                .validator({} as FormControl)?.required;
+        }
+        return false;
+    }
+
+    /* By default, the `blInput` directive uses the validator on the attached
+     * control to determine if the input element is required. However, if a
+     * template specifies a `required` attribute directly, e.g.
+     *
+     *      <input blInput type="text" required>,
+     *
+     * the directive will use it instead.
+     */
+    public set required(value: boolean) {
+        this._required = coerceBooleanProperty(value);
+        this._requiredAttribute = true;
+    }
+
+    @HostBinding("attr.aria-required")
+    public get ariaRequired(): string { return this.required ? "true" : null; }
 
     /** Input type of the element. */
     @Input()

diff --git a/src/app/components/job/action/add/add-job-form.scss b/src/app/components/job/action/add/add-job-form.scss
@@ -6,8 +6,3 @@
     margin-bottom: 5px;
     vertical-align: top;
 }
-
-.required label:after {
-    content: "*";
-    color: $validation-error-color;
-}
diff --git a/src/app/components/pool/action/add/pool-create-basic-dialog.scss b/src/app/components/pool/action/add/pool-create-basic-dialog.scss
@@ -26,8 +26,3 @@ bl-pool-create-basic-dialog {
         color: #1d54af;
     }
 }
-
-.required label:after {
-    content: "*";
-    color: $validation-error-color;
-}