Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Added required input for single file attachment #78

Merged
merged 10 commits into from
Nov 18, 2019
31 changes: 6 additions & 25 deletions src/app/public/modules/checkbox/checkbox.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ import {
} from '@angular/core';

import {
AbstractControl,
ControlValueAccessor,
NgControl
} from '@angular/forms';

import {
SkyFormsUtility
} from '../shared/forms-utility';

/**
Blackbaud-AlexKingman marked this conversation as resolved.
Show resolved Hide resolved
* Monotonically increasing integer used to auto-generate unique ids for checkbox components.
*/
Expand Down Expand Up @@ -101,7 +104,7 @@ export class SkyCheckboxComponent implements ControlValueAccessor, OnInit {
*/
@Input()
set required(value: boolean) {
this._required = this.coerceBooleanProperty(value);
this._required = SkyFormsUtility.coerceBooleanProperty(value);
}

get required(): boolean {
Expand All @@ -127,7 +130,7 @@ export class SkyCheckboxComponent implements ControlValueAccessor, OnInit {
public ngOnInit(): void {
if (this.ngControl) {
// Backwards compatibility support for anyone still using Validators.Required.
this.required = this.required || this.hasRequiredValidation(this.ngControl);
this.required = this.required || SkyFormsUtility.hasRequiredValidation(this.ngControl);
}
}

Expand Down Expand Up @@ -201,26 +204,4 @@ export class SkyCheckboxComponent implements ControlValueAccessor, OnInit {
this.checked = !this.checked;
}

/**
* Gets the required state of the checkbox.
* Currently, Angular doesn't offer a way to get this easily, so we have to create an empty
* control using the current validator to see if it throws a `required` validation error.
* https://github.com/angular/angular/issues/13461#issuecomment-340368046
*/
private hasRequiredValidation(ngControl: NgControl): boolean {
const abstractControl = ngControl.control as AbstractControl;
if (abstractControl && abstractControl.validator) {
const validator = abstractControl.validator({} as AbstractControl);
if (validator && validator.required) {
return true;
}
}
return false;
}

/** Coerces a data-bound value (typically a string) to a boolean. */
private coerceBooleanProperty(value: any): boolean {
return value !== undefined && `${value}` !== 'false';
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<div
class="sky-file-attachment-label-wrapper"
[ngClass]="{ 'sky-control-label-required': required && hasLabelComponent }"
[attr.aria-required]="(required) ? true : null"
[attr.id]="labelElementId"
>
<ng-container *ngTemplateOutlet="labelContent"
Expand All @@ -22,6 +23,7 @@
tabindex="-1"
type="file"
[attr.accept]="acceptedTypes || null"
[attr.required]="(required) ? '' : null"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your previous pull request, you used required, but here you're using attr.required. Is there a specific difference between the two?
https://github.com/blackbaud/skyux-forms/pull/77/files#diff-38b1c0ce82272f881b231303a0ea9867R15

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that - it needs to be consistent. I've changed it.

(change)="fileChangeEvent($event)"
#fileInput
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('File attachment', () => {
fileAttachmentInstance = fixture.componentInstance.fileAttachmentComponent;
});

//#region helpers
function getInputDebugEl(): DebugElement {
return fixture.debugElement.query(By.css('input'));
}
Expand Down Expand Up @@ -304,18 +305,55 @@ describe('File attachment', () => {
}
//#endregion

it('should allow the user to specify if the file is required', fakeAsync(() => {
it('should not have required class and aria-reqiured attribute when not required', fakeAsync(() => {
fileAttachmentInstance.ngAfterViewInit();
tick();
fixture.detectChanges();
const labelWrapper = getLabelWrapper();
const input = getInputDebugEl();

expect(input.nativeElement.getAttribute('required')).toBeNull();
expect(labelWrapper.classList.contains('sky-control-label-required')).toBe(false);
expect(labelWrapper.getAttribute('aria-required')).toBeNull();
}));

let labelWrapper = getLabelWrapper();
it('should have appropriate classes when file is required', fakeAsync(() => {
fixture.componentInstance.required = true;
fileAttachmentInstance.ngAfterViewInit();
tick();
fixture.detectChanges();
const labelWrapper = getLabelWrapper();
const input = getInputDebugEl();

expect(input.nativeElement.getAttribute('required')).not.toBeNull();
expect(labelWrapper.classList.contains('sky-control-label-required')).toBe(true);
expect(labelWrapper.getAttribute('aria-required')).toBe('true');
}));

it('should have appropriate classes when file is required and initialized with file', fakeAsync(() => {
fixture.componentInstance.required = true;
const testFile = <SkyFileItem> {
file: {
name: 'myFile',
type: '',
size: 1
},
url: 'myFile'
};
fileAttachmentInstance.value = testFile;
fileAttachmentInstance.ngAfterViewInit();
tick();
fixture.detectChanges();
const labelWrapper = getLabelWrapper();
const input = getInputDebugEl();

expect(fileAttachmentInstance.required).toBe(true);
expect(input.nativeElement.getAttribute('required')).not.toBeNull();
expect(labelWrapper.classList.contains('sky-control-label-required')).toBe(true);
expect(labelWrapper.getAttribute('aria-required')).toBe('true');
}));

it('should handle removing the label', fakeAsync(() => {
fixture.componentInstance.required = true;
fileAttachmentInstance.ngAfterViewInit();
fileAttachmentInstance.ngAfterContentInit();
tick();
Expand Down
78 changes: 34 additions & 44 deletions src/app/public/modules/file-attachment/file-attachment.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,27 @@ import {
ContentChildren,
ElementRef,
EventEmitter,
forwardRef,
Input,
OnDestroy,
Optional,
Output,
ViewChild,
QueryList
QueryList,
Self,
ViewChild
} from '@angular/core';

import {
ControlValueAccessor,
AbstractControl,
NG_VALUE_ACCESSOR,
NG_VALIDATORS
NgControl
} from '@angular/forms';

import {
Subject
} from 'rxjs';

import {
SkyFormsUtility
} from '../shared/forms-utility';

import {
SkyFileAttachmentChange
} from './types/file-attachment-change';
Expand All @@ -50,31 +52,16 @@ import {
SkyFileItemService
} from './file-item.service';

// tslint:disable:no-forward-ref no-use-before-declare
const SKY_FILE_ATTACHMENT_VALUE_ACCESSOR = {
provide: NG_VALUE_ACCESSOR,
useExisting: forwardRef(() => SkyFileAttachmentComponent),
multi: true
};

const SKY_FILE_ATTACHMENT_VALIDATOR = {
provide: NG_VALIDATORS,
useExisting: forwardRef(() => SkyFileAttachmentComponent),
multi: true
};

Blackbaud-SteveBrush marked this conversation as resolved.
Show resolved Hide resolved
let uniqueId = 0;

@Component({
selector: 'sky-file-attachment',
templateUrl: './file-attachment.component.html',
styleUrls: ['./file-attachment.component.scss'],
providers: [
SKY_FILE_ATTACHMENT_VALUE_ACCESSOR,
SKY_FILE_ATTACHMENT_VALIDATOR
],
changeDetection: ChangeDetectionStrategy.OnPush
})
export class SkyFileAttachmentComponent implements ControlValueAccessor, AfterViewInit, AfterContentInit, OnDestroy {
export class SkyFileAttachmentComponent implements AfterViewInit, AfterContentInit, OnDestroy {

@Input()
public acceptedTypes: string;

Expand Down Expand Up @@ -105,6 +92,13 @@ export class SkyFileAttachmentComponent implements ControlValueAccessor, AfterVi

public rejectedOver: boolean = false;

/**
* Indicates whether the input is required for form validation.
* When you set this property to `true`, the component adds `aria-required` and `required`
* attributes to the input element so that forms display an invalid state until the input element
* is complete. This property accepts a `boolean` value.
*/
@Input()
public required: boolean = false;

public set value(value: SkyFileItem) {
Expand All @@ -130,17 +124,24 @@ export class SkyFileAttachmentComponent implements ControlValueAccessor, AfterVi
@ContentChildren(SkyFileAttachmentLabelComponent)
private labelComponents: QueryList<SkyFileAttachmentLabelComponent>;

private control: AbstractControl;
private enterEventTarget: any;

private fileAttachmentId = uniqueId++;

private ngUnsubscribe = new Subject<void>();

private _value: any;

constructor(
private changeDetector: ChangeDetectorRef,
private fileAttachmentService: SkyFileAttachmentService,
private fileItemService: SkyFileItemService
) { }
private fileItemService: SkyFileItemService,
@Self() @Optional() private ngControl: NgControl
) {
if (this.ngControl) {
this.ngControl.valueAccessor = this;
}
}

public ngAfterViewInit(): void {
// This is needed to address a bug in Angular 7.
Expand All @@ -149,19 +150,16 @@ export class SkyFileAttachmentComponent implements ControlValueAccessor, AfterVi
// Of note is the parent check which allows us to determine if the form is reactive.
// Without this check there is a changed before checked error
/* istanbul ignore else */
if (this.control) {
if (this.ngControl) {
setTimeout(() => {
this.control.setValue(this.value, {
this.ngControl.control.setValue(this.value, {
emitEvent: false
});

// Set required to apply required state to label
if (this.control.errors && this.control.errors.required) {
this.required = true;
}

this.changeDetector.markForCheck();
});

// Backwards compatibility support for anyone still using Validators.Required.
this.required = this.required || SkyFormsUtility.hasRequiredValidation(this.ngControl);
}
}

Expand Down Expand Up @@ -286,14 +284,6 @@ export class SkyFileAttachmentComponent implements ControlValueAccessor, AfterVi
this.changeDetector.markForCheck();
}

public validate(control: AbstractControl): { [key: string]: any } {
if (!this.control) {
this.control = control;
}

return undefined;
}

Blackbaud-SteveBrush marked this conversation as resolved.
Show resolved Hide resolved
public emitClick(): void {
this.fileClick.emit({
file: this.value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
>
<sky-file-attachment
formControlName="attachment"
[required]="required"
>
<sky-file-attachment-label
*ngIf="showLabel"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,31 @@
import {
Component,
ViewChild,
OnInit
OnInit,
ViewChild
} from '@angular/core';

import {
FormBuilder,
FormControl,
FormGroup
} from '@angular/forms';

import {
SkyFileAttachmentComponent
} from '../file-attachment.component';

import { FormGroup, FormControl, Validators, FormBuilder } from '@angular/forms';

@Component({
selector: 'file-attachment-test',
templateUrl: './file-attachment.component.fixture.html'
})
export class FileAttachmentTestComponent implements OnInit {

public attachment: FormControl;

public fileForm: FormGroup;

public required: boolean = false;

public showLabel: boolean = true;

@ViewChild(SkyFileAttachmentComponent)
Expand All @@ -29,7 +36,7 @@ export class FileAttachmentTestComponent implements OnInit {
) { }

public ngOnInit(): void {
this.attachment = new FormControl(undefined, Validators.required);
this.attachment = new FormControl(undefined);
this.fileForm = this.formBuilder.group({
attachment: this.attachment
});
Expand Down
32 changes: 32 additions & 0 deletions src/app/public/modules/shared/forms-utility.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {
AbstractControl,
NgControl
} from '@angular/forms';

// Need to add the following to classes which contain static methods.
// See: https://github.com/ng-packagr/ng-packagr/issues/641
// @dynamic
export class SkyFormsUtility {

/** Coerces a data-bound value (typically a string) to a boolean. */
public static coerceBooleanProperty(value: any): boolean {
return value !== undefined && `${value}` !== 'false';
}

/**
* Gets the required state of the checkbox.
* Currently, Angular doesn't offer a way to get this easily, so we have to create an empty
* control using the current validator to see if it throws a `required` validation error.
* https://github.com/angular/angular/issues/13461#issuecomment-340368046
*/
public static hasRequiredValidation(ngControl: NgControl): boolean {
const abstractControl = ngControl.control as AbstractControl;
if (abstractControl && abstractControl.validator) {
const validator = abstractControl.validator({} as AbstractControl);
if (validator && validator.required) {
return true;
}
}
return false;
}
}