Skip to content

feat(material-experimental/mdc-checkbox): add default options #17578

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 4, 2019
Merged

feat(material-experimental/mdc-checkbox): add default options #17578

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5dbbe86
feat(material-experimental/mdc-checkbox): add default options
andrewseguin Nov 1, 2019
6045b0a
add breaking changes comment
andrewseguin Nov 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 84 additions & 2 deletions src/material-experimental/mdc-checkbox/checkbox.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
MatCheckboxChange,
MatCheckboxModule
} from './index';
import {MAT_CHECKBOX_DEFAULT_OPTIONS} from '@angular/material/checkbox';


describe('MatCheckbox', () => {
Expand Down Expand Up @@ -468,13 +469,48 @@ describe('MatCheckbox', () => {
}));
});

describe(`when MAT_CHECKBOX_CLICK_ACTION is set`, () => {
beforeEach(() => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [MatCheckboxModule, FormsModule, ReactiveFormsModule],
declarations: [SingleCheckbox],
providers: [
{provide: MAT_CHECKBOX_CLICK_ACTION, useValue: 'check'},
{provide: MAT_CHECKBOX_DEFAULT_OPTIONS, useValue: {clickAction: 'noop'}}
]
});

fixture = createComponent(SingleCheckbox);
fixture.detectChanges();

checkboxDebugElement = fixture.debugElement.query(By.directive(MatCheckbox))!;
checkboxNativeElement = checkboxDebugElement.nativeElement;
testComponent = fixture.debugElement.componentInstance;

inputElement = checkboxNativeElement.querySelector('input') as HTMLInputElement;
});

it('should override the value set in the default options', fakeAsync(() => {
testComponent.isIndeterminate = true;
inputElement.click();
fixture.detectChanges();
flush();

expect(inputElement.checked).toBe(true);
expect(inputElement.indeterminate).toBe(true);
}));
});

describe(`when MAT_CHECKBOX_CLICK_ACTION is 'check'`, () => {
beforeEach(() => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [MatCheckboxModule, FormsModule, ReactiveFormsModule],
declarations: [SingleCheckbox],
providers: [{provide: MAT_CHECKBOX_CLICK_ACTION, useValue: 'check'}]
providers: [
{provide: MAT_CHECKBOX_DEFAULT_OPTIONS, useValue: {clickAction: 'check'}}
]
});

fixture = createComponent(SingleCheckbox);
Expand Down Expand Up @@ -506,7 +542,9 @@ describe('MatCheckbox', () => {
TestBed.configureTestingModule({
imports: [MatCheckboxModule, FormsModule, ReactiveFormsModule],
declarations: [SingleCheckbox],
providers: [{provide: MAT_CHECKBOX_CLICK_ACTION, useValue: 'noop'}]
providers: [
{provide: MAT_CHECKBOX_DEFAULT_OPTIONS, useValue: {clickAction: 'noop'}}
]
});

fixture = createComponent(SingleCheckbox);
Expand Down Expand Up @@ -933,6 +971,50 @@ describe('MatCheckbox', () => {
});
});

describe('MatCheckboxDefaultOptions', () => {
describe('when MAT_CHECKBOX_DEFAULT_OPTIONS overridden', () => {
beforeEach(() => {
TestBed.configureTestingModule({
imports: [MatCheckboxModule, FormsModule],
declarations: [SingleCheckbox, SingleCheckbox],
providers: [{
provide: MAT_CHECKBOX_DEFAULT_OPTIONS,
useValue: {color: 'primary'},
}],
});

TestBed.compileComponents();
});

it('should override default color in component', () => {
const fixture: ComponentFixture<SingleCheckbox> =
TestBed.createComponent(SingleCheckbox);
fixture.detectChanges();
const checkboxDebugElement: DebugElement =
fixture.debugElement.query(By.directive(MatCheckbox))!;
expect(
checkboxDebugElement.nativeElement.classList
).toContain('mat-primary');
});

it('should not override explicit input bindings', () => {
const fixture: ComponentFixture<SingleCheckbox> =
TestBed.createComponent(SingleCheckbox);
fixture.componentInstance.checkboxColor = 'warn';
fixture.detectChanges();
const checkboxDebugElement: DebugElement =
fixture.debugElement.query(By.directive(MatCheckbox))!;
expect(
checkboxDebugElement.nativeElement.classList
).not.toContain('mat-primary');
expect(
checkboxDebugElement.nativeElement.classList
).toContain('mat-warn');
expect(checkboxDebugElement.nativeElement.classList).toContain('mat-warn');
});
});
});

/** Simple component for testing a single checkbox. */
@Component({
template: `
Expand Down
28 changes: 24 additions & 4 deletions src/material-experimental/mdc-checkbox/checkbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ import {
ViewEncapsulation
} from '@angular/core';
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
import {MAT_CHECKBOX_CLICK_ACTION, MatCheckboxClickAction} from '@angular/material/checkbox';
import {
MAT_CHECKBOX_CLICK_ACTION,
MAT_CHECKBOX_DEFAULT_OPTIONS,
MatCheckboxClickAction, MatCheckboxDefaultOptions
} from '@angular/material/checkbox';
import {ThemePalette} from '@angular/material/core';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
import {MDCCheckboxAdapter, MDCCheckboxFoundation} from '@material/checkbox';
Expand Down Expand Up @@ -228,12 +232,28 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
private _changeDetectorRef: ChangeDetectorRef,
private _platform: Platform,
@Attribute('tabindex') tabIndex: string,
/**
* @deprecated `_clickAction` parameter to be removed, use
* `MAT_CHECKBOX_DEFAULT_OPTIONS`
* @breaking-change 10.0.0
*/
@Optional() @Inject(MAT_CHECKBOX_CLICK_ACTION) private _clickAction: MatCheckboxClickAction,
@Optional() @Inject(ANIMATION_MODULE_TYPE) public _animationMode?: string) {
this.tabIndex = parseInt(tabIndex) || 0;
this._checkboxFoundation = new MDCCheckboxFoundation(this._checkboxAdapter);
@Optional() @Inject(ANIMATION_MODULE_TYPE) public _animationMode?: string,
@Optional() @Inject(MAT_CHECKBOX_DEFAULT_OPTIONS)
private _options?: MatCheckboxDefaultOptions) {
// Note: We don't need to set up the MDCFormFieldFoundation. Its only purpose is to manage the
// ripple, which we do ourselves instead.
this.tabIndex = parseInt(tabIndex) || 0;
this._checkboxFoundation = new MDCCheckboxFoundation(this._checkboxAdapter);

this._options = this._options || {};
Copy link
Member

@crisbeto crisbeto Nov 2, 2019

Choose a reason for hiding this comment

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

You're only using the options in the constructor so you don't need to create a property for them.

Copy link
Contributor Author

@andrewseguin andrewseguin Nov 4, 2019

Choose a reason for hiding this comment

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

True, though it does make it a little nicer for the following lines where I directly access this._options instead of adding an if(this._options) { ... } check before them.

Copy link
Contributor

@mmalerba mmalerba Nov 4, 2019

Choose a reason for hiding this comment

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

Can we use default parameters on constructors that get DI'd? i.e.:
@Optional() @Inject(MAT_CHECKBOX_DEFAULT_OPTIONS) private _options: MatCheckboxDefaultOptions = {}

Copy link
Contributor Author

@andrewseguin andrewseguin Nov 4, 2019

Choose a reason for hiding this comment

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

Unfortunately not, Angular will inject null if it's not provided: https://stackblitz.com/edit/angular-euxmcx?file=src/app/app.component.ts


if (this._options.color) {
this.color = this._options.color;
}

// TODO: Remove this after the `_clickAction` parameter is removed as an injection parameter.
Copy link
Member

@crisbeto crisbeto Nov 2, 2019

Choose a reason for hiding this comment

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

This should have a @breaking-change 10.0.0 so we can catch it in the breaking changes script while doing the cleanup.

this._clickAction = this._clickAction || this._options.clickAction;
}

ngAfterViewInit() {
Expand Down