feat(material-experimental/mdc-checkbox): add default options #17578
Conversation
googlebot
added
the
cla: yes
andrewseguin
added
pr: merge safe
target: patch
| this.tabIndex = parseInt(tabIndex) || 0; | ||
| this._checkboxFoundation = new MDCCheckboxFoundation(this._checkboxAdapter); | ||
|
|
||
| this._options = this._options || {}; |
There was a problem hiding this comment.
You're only using the options in the constructor so you don't need to create a property for them.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Can we use default parameters on constructors that get DI'd? i.e.:
@Optional() @Inject(MAT_CHECKBOX_DEFAULT_OPTIONS) private _options: MatCheckboxDefaultOptions = {}
There was a problem hiding this comment.
Unfortunately not, Angular will inject null if it's not provided: https://stackblitz.com/edit/angular-euxmcx?file=src/app/app.component.ts
| this.color = this._options.color; | ||
| } | ||
|
|
||
| // TODO: Remove this after the `_clickAction` parameter is removed as an injection parameter. |
There was a problem hiding this comment.
This should have a @breaking-change 10.0.0 so we can catch it in the breaking changes script while doing the cleanup.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Follow-up to legacy checkbox's options implementation, PR #17473 - changes and tests are essentially copy/paste to maintain parity between the two