md-icon #281
md-icon #281
Conversation
googlebot
added
the
cla: yes
| } | ||
|
|
||
| @Injectable() | ||
| export class MdIconProvider { |
There was a problem hiding this comment.
I'm trying to come up with a better name for this class, since it would be good to move away from the Angular 1 "provider" concept. What do you think about MdIconRegistry ?
| * Sets the CSS class name to be used for icon fonts when an <md-icon> component does not | ||
| * have a fontSet input value, and is not loading an icon by name or URL. | ||
| */ | ||
| setDefaultFontSetClass(className: string) { |
There was a problem hiding this comment.
Missing return value type (this)
| return this._inProgressUrlFetches.get(url); | ||
| } | ||
| const req = this._http.get(url) | ||
| .map((response) => response.text()) |
There was a problem hiding this comment.
Forget the whole thing. Apparently HTTP doesn't have a way to do that.
Note that a flatMap might be a good idea if the server returns multiple elements.
|
|
||
| @Component({ | ||
| selector: 'test-app', | ||
| template: `<md-icon aria-label="{{ariaLabel}}" alt="{{altText}}">{{iconName}}</md-icon>`, |
There was a problem hiding this comment.
One last comment:
for all these tests where you're using the {{ }} inside the attribute value, they should instead use the [ ] syntax, e.g.:
<md-icon [attr.aria-label]="ariaLabel" [alt]="altText">{{iconName}}</md-icon>
There was a problem hiding this comment.
Done. Incidentally the documentation slightly leans toward interpolation: https://angular.io/docs/ts/latest/guide/template-syntax.html#!#property-binding
There is no technical reason to prefer one form to the other. We lean toward readability, which tends to favor interpolation.
|
LGTM aside from those last two minor comments. |
|
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. |
Preview PR for md-icon. Doc at https://docs.google.com/a/google.com/document/d/1zWTjnSLFPV4II9IxpIehyeSFtnzw_efdy7kEerMRIuo/edit?usp=sharing