Skip to content
This repository has been archived by the owner on Mar 27, 2023. It is now read-only.

Commit

Permalink
[NG] clrInput and form control validation components (#2295)
Browse files Browse the repository at this point in the history
* Renamed FormControlService and WrappedFormControl
* Added ClrInput, ClrInputContainer for tracking input control states
* Added ClrControlHelper and ClrControlError for validation
* Added ClrIfError directive to conditionally display error messages

Signed-off-by: Jeremy Wilken <gnomation@gnomeontherun.com>
  • Loading branch information
gnomeontherun committed Jun 6, 2018
1 parent 9a729b9 commit 4863786
Show file tree
Hide file tree
Showing 52 changed files with 1,046 additions and 83 deletions.
1 change: 1 addition & 0 deletions CODING_GUIDELINES.md
Expand Up @@ -150,6 +150,7 @@ ClrFormsModule
* Have related components (like parent / child) communicate by default through services.
* Never use explicitly named template reference variables as part of the API, to query them through `@ContentChild`. Simply receive structural directives or inputs.
* Never make HTTP calls, and never impose formatting constraints on their backend API.
* In cases where a directive cannot be used unless it is a child of another directive, it is usually best to throw a new Error in the constructor to alert developers that it is not supported. [Example](src/clr-angular/popover/dropdown/dropdown-menu.ts)

Finally, and this is harder to put in black-or-white terms, keep the API as simple and natural as possible, even if it means more work on Clarity's side. Implementing multiple directives and components and making them communicate through services will lead to a much more pleasant integration than forcing your consumer to pass data manually from one to the other. Here are a couple examples of what to do and not to do (taken from real-life carousel components):

Expand Down
Binary file modified gemini/screens/forms/form-input-group/default/chrome.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 5 additions & 6 deletions src/clr-angular/forms/checkbox/checkbox-container.spec.ts
Expand Up @@ -7,8 +7,7 @@ import { Component } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';

import { FormControlService } from '../common/form-control.service';

import { ControlIdService } from '../common/providers/control-id.service';
import { ClrCheckboxContainer } from './checkbox-container';

@Component({
Expand All @@ -23,7 +22,7 @@ class SimpleTest {}

interface TestContext {
fixture: ComponentFixture<SimpleTest>;
formControlService: FormControlService;
controlIdService: ControlIdService;
checkboxContainer: ClrCheckboxContainer;
checkboxContainerEl: any;
}
Expand All @@ -35,13 +34,13 @@ export default function(): void {
this.fixture = TestBed.createComponent(SimpleTest);
this.fixture.detectChanges();
const checkboxContainerDE = this.fixture.debugElement.query(By.directive(ClrCheckboxContainer));
this.formControlService = checkboxContainerDE.injector.get(FormControlService, null);
this.controlIdService = checkboxContainerDE.injector.get(ControlIdService, null);
this.checkboxContainer = checkboxContainerDE.componentInstance;
this.checkboxContainerEl = checkboxContainerDE.nativeElement;
});

it('declares a FormControlService provider', function(this: TestContext) {
expect(this.formControlService).toBeTruthy();
it('declares a ControlIdService provider', function(this: TestContext) {
expect(this.controlIdService).toBeTruthy();
});

it('implements DynamicWrapper', function(this: TestContext) {
Expand Down
4 changes: 2 additions & 2 deletions src/clr-angular/forms/checkbox/checkbox-container.ts
Expand Up @@ -7,7 +7,7 @@
import { Component } from '@angular/core';

import { DynamicWrapper } from '../../utils/host-wrapping/dynamic-wrapper';
import { FormControlService } from '../common/form-control.service';
import { ControlIdService } from '../common/providers/control-id.service';

@Component({
selector: 'clr-checkbox-container',
Expand All @@ -18,7 +18,7 @@ import { FormControlService } from '../common/form-control.service';
<label *ngIf="_dynamic"></label>
`,
host: { '[class.checkbox]': 'true' },
providers: [FormControlService],
providers: [ControlIdService],
})
export class ClrCheckboxContainer implements DynamicWrapper {
// Indicates whether the container is dynamically created by the checkbox input itself
Expand Down
6 changes: 3 additions & 3 deletions src/clr-angular/forms/checkbox/checkbox.spec.ts
Expand Up @@ -6,8 +6,8 @@
import { Component } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { FormControlService } from '../common/form-control.service';
import { WrappedFormControl } from '../common/wrapped-form-control';
import { ControlIdService } from '../common/providers/control-id.service';
import { WrappedFormControl } from '../common/wrapped-control';
import { ClrCheckboxNext } from './checkbox';
import { ClrCheckboxContainer } from './checkbox-container';

Expand All @@ -22,7 +22,7 @@ export default function(): void {
describe('Checkbox directive', () => {
it('correctly extends WrappedFormControl<ClrCheckboxContainer>', function() {
spyOn(WrappedFormControl.prototype, 'ngOnInit');
TestBed.configureTestingModule({ declarations: [ClrCheckboxNext, SimpleTest], providers: [FormControlService] });
TestBed.configureTestingModule({ declarations: [ClrCheckboxNext, SimpleTest], providers: [ControlIdService] });
this.fixture = TestBed.createComponent(SimpleTest);
this.fixture.detectChanges();
expect(
Expand Down
2 changes: 1 addition & 1 deletion src/clr-angular/forms/checkbox/checkbox.ts
Expand Up @@ -6,7 +6,7 @@

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

import { WrappedFormControl } from '../common/wrapped-form-control';
import { WrappedFormControl } from '../common/wrapped-control';

import { ClrCheckboxContainer } from './checkbox-container';

Expand Down
18 changes: 14 additions & 4 deletions src/clr-angular/forms/common/all.spec.ts
Expand Up @@ -11,13 +11,23 @@
*/

import CommonSpecs from './common.spec';
import FormControlServiceSpecs from './form-control.service.spec';
import ErrorSpecs from './error.spec';
import HelperSpecs from './helper.spec';
import ControlStatusServiceSpecs from './if-error/if-error.service.spec';
import IfErrorSpecs from './if-error/if-error.spec';
import LabelSpecs from './label.spec';
import WrappedFormControlSpecs from './wrapped-form-control.spec';
import ControlIdServiceSpecs from './providers/control-id.service.spec';
import NgControlServiceSpecs from './providers/ng-control.service.spec';
import WrappedControlSpecs from './wrapped-control.spec';

describe('Forms common utilities', function() {
FormControlServiceSpecs();
ControlIdServiceSpecs();
ControlStatusServiceSpecs();
NgControlServiceSpecs();
LabelSpecs();
WrappedFormControlSpecs();
IfErrorSpecs();
WrappedControlSpecs();
CommonSpecs();
ErrorSpecs();
HelperSpecs();
});
9 changes: 8 additions & 1 deletion src/clr-angular/forms/common/common.module.ts
Expand Up @@ -7,7 +7,14 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';

import { ClrControlError } from './error';
import { ClrControlHelper } from './helper';
import { ClrIfError } from './if-error/if-error';
import { ClrLabel } from './label';

@NgModule({ imports: [CommonModule], declarations: [ClrLabel], exports: [ClrLabel] })
@NgModule({
imports: [CommonModule],
declarations: [ClrLabel, ClrControlError, ClrControlHelper, ClrIfError],
exports: [ClrLabel, ClrControlError, ClrControlHelper, ClrIfError],
})
export class ClrCommonFormsModule {}
9 changes: 4 additions & 5 deletions src/clr-angular/forms/common/common.spec.ts
Expand Up @@ -11,8 +11,8 @@ import { DynamicWrapper } from '../../utils/host-wrapping/dynamic-wrapper';
import { ClrHostWrappingModule } from '../../utils/host-wrapping/host-wrapping.module';

import { ClrCommonFormsModule } from './common.module';
import { FormControlService } from './form-control.service';
import { WrappedFormControl } from './wrapped-form-control';
import { ControlIdService } from './providers/control-id.service';
import { WrappedFormControl } from './wrapped-control';

/*
* Components representing generic form controls.
Expand All @@ -24,7 +24,7 @@ import { WrappedFormControl } from './wrapped-form-control';
<ng-content></ng-content>
<label id="container-view-label-after"></label>
`,
providers: [FormControlService],
providers: [ControlIdService],
})
class GenericWrapper implements DynamicWrapper {
_dynamic = false;
Expand Down Expand Up @@ -61,7 +61,6 @@ class NoWrapperWithId {}
<input genericControl />
<label id="test-view-label-after"></label>
</generic-wrapper>
`,
})
class WithWrapperNoId {}
Expand All @@ -87,7 +86,7 @@ export default function(): void {
fixture.detectChanges();
if (!expectedId) {
const wrapperDebug = fixture.debugElement.query(By.directive(GenericWrapper));
expectedId = wrapperDebug.injector.get(FormControlService).id;
expectedId = wrapperDebug.injector.get(ControlIdService).id;
}
const input = fixture.nativeElement.querySelector('input');
expect(input.getAttribute('id')).toBe(expectedId, 'input has the wrong id attribute');
Expand Down
35 changes: 35 additions & 0 deletions src/clr-angular/forms/common/error.spec.ts
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2016-2018 VMware, Inc. All Rights Reserved.
* This software is released under MIT license.
* The full license information can be found in LICENSE in the root directory of this project.
*/
import { Component } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';

import { ClrControlError } from './error';

@Component({ template: `<clr-control-error>Test error</clr-control-error>` })
class SimpleTest {}

export default function(): void {
describe('ClrControlError', () => {
let fixture;

beforeEach(function() {
TestBed.configureTestingModule({ declarations: [ClrControlError, SimpleTest] });
fixture = TestBed.createComponent(SimpleTest);
fixture.detectChanges();
});

it('projects content', function() {
expect(fixture.debugElement.query(By.directive(ClrControlError)).nativeElement.innerText).toContain('Test error');
});

it('adds the .clr-subtext class to host', function() {
expect(
fixture.debugElement.query(By.directive(ClrControlError)).nativeElement.classList.contains('clr-subtext')
).toBeTrue();
});
});
}
16 changes: 16 additions & 0 deletions src/clr-angular/forms/common/error.ts
@@ -0,0 +1,16 @@
/**
* Copyright (c) 2016-2018 VMware, Inc. All Rights Reserved.
* This software is released under MIT license.
* The full license information can be found in LICENSE in the root directory of this project.
*/

import { Component } from '@angular/core';

@Component({
selector: 'clr-control-error',
template: `
<ng-content></ng-content>
`,
host: { '[class.clr-subtext]': 'true' },
})
export class ClrControlError {}
37 changes: 37 additions & 0 deletions src/clr-angular/forms/common/helper.spec.ts
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2016-2018 VMware, Inc. All Rights Reserved.
* This software is released under MIT license.
* The full license information can be found in LICENSE in the root directory of this project.
*/
import { Component } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';

import { ClrControlHelper } from './helper';

@Component({ template: `<clr-control-helper>Test helper</clr-control-helper>` })
class SimpleTest {}

export default function(): void {
describe('ClrControlHelper', () => {
let fixture;

beforeEach(function() {
TestBed.configureTestingModule({ declarations: [ClrControlHelper, SimpleTest] });
fixture = TestBed.createComponent(SimpleTest);
fixture.detectChanges();
});

it('projects content', function() {
expect(fixture.debugElement.query(By.directive(ClrControlHelper)).nativeElement.innerText).toContain(
'Test helper'
);
});

it('adds the .clr-subtext class to host', function() {
expect(
fixture.debugElement.query(By.directive(ClrControlHelper)).nativeElement.classList.contains('clr-subtext')
).toBeTrue();
});
});
}
16 changes: 16 additions & 0 deletions src/clr-angular/forms/common/helper.ts
@@ -0,0 +1,16 @@
/**
* Copyright (c) 2016-2018 VMware, Inc. All Rights Reserved.
* This software is released under MIT license.
* The full license information can be found in LICENSE in the root directory of this project.
*/

import { Component } from '@angular/core';

@Component({
selector: 'clr-control-helper',
template: `
<ng-content></ng-content>
`,
host: { '[class.clr-subtext]': 'true' },
})
export class ClrControlHelper {}
62 changes: 62 additions & 0 deletions src/clr-angular/forms/common/if-error/if-error.service.spec.ts
@@ -0,0 +1,62 @@
/*
* Copyright (c) 2016-2018 VMware, Inc. All Rights Reserved.
* This software is released under MIT license.
* The full license information can be found in LICENSE in the root directory of this project.
*/
import { FormControl } from '@angular/forms';

import { NgControlService } from '../providers/ng-control.service';

import { IfErrorService } from './if-error.service';

export default function(): void {
describe('IfErrorService', function() {
let service, ngControlService, testControl;

beforeEach(() => {
testControl = new FormControl(true);
ngControlService = new NgControlService();
service = new IfErrorService(ngControlService);
});

it('subscribes to the statusChanges when the control is emitted', () => {
spyOn(testControl.statusChanges, 'subscribe').and.callThrough();
ngControlService.setControl(testControl);
expect(testControl.statusChanges.subscribe).toHaveBeenCalled();
});

it('provides observable for statusChanges, passing the control', () => {
const cb = jasmine.createSpy('cb');
const sub = service.statusChanges.subscribe(control => cb(control));
ngControlService.setControl(testControl);
// Change the state of the input to trigger statusChange
testControl.markAsTouched();
testControl.updateValueAndValidity();
expect(cb).toHaveBeenCalled();
expect(cb).toHaveBeenCalledWith(testControl);
sub.unsubscribe();
});

it('should allow a manual trigger of status observable', () => {
const cb = jasmine.createSpy('cb');
const sub = service.statusChanges.subscribe(control => cb(control));
ngControlService.setControl(testControl);
// Manually trigger status check
service.triggerStatusChange();
expect(cb).toHaveBeenCalled();
expect(cb).toHaveBeenCalledWith(testControl);
sub.unsubscribe();
});

it('should not fire status changes if control is untouched', () => {
const cb = jasmine.createSpy('cb');
const sub = service.statusChanges.subscribe(control => cb(control));
ngControlService.setControl(testControl);
// Change state of the input, should only fire when touched
testControl.markAsDirty();
testControl.updateValueAndValidity();
expect(cb).not.toHaveBeenCalled();
sub.unsubscribe();
});
});
}

0 comments on commit 4863786

Please sign in to comment.