Expose isIconButton publicly #11226
Expose isIconButton publicly #11226
Conversation
googlebot
added
the
cla: yes
|
@DavidSouther could you talk about your use-case for needing these APIs? |
jelbourn
added
the
needs: discussion
|
@jelbourn We have a "Progress Button" which replaces the content with an ng-spinner while awaiting an observable. It seems silly to have both @ContentChild(MatButton) as well as @ContentChild('button[mat-icon-button]') when we could just ask the button itself whether it's an icon button. And since FAB is right there, I just figured I'd expose that as well. |
src/lib/button/button.ts
Outdated
| @@ -77,9 +77,15 @@ export class MatButton extends _MatButtonMixinBase | |||
|
|
|||
| /** Whether the button is round. */ | |||
| _isRoundButton: boolean = this._hasHostAttributes('mat-fab', 'mat-mini-fab'); | |||
| get isRoundButton(): boolean { | |||
There was a problem hiding this comment.
Instead of making these getters, could you just make the existing properties public and readonly? TypeScript generates a fair amount more code for getters, so it's better to avoid when you don't also need a setter.
There was a problem hiding this comment.
More than happy to, wanted to avoid what might be perceived as a breaking change.
There was a problem hiding this comment.
We consider anything prefixed with an underscore to be a private API, so it should be okay
There was a problem hiding this comment.
Done. PTAL. I only found the one if block, and the few .html references in this repo.
Update button.html with public property names.
jelbourn
added
pr: lgtm
action: merge
|
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. |
No description provided.