Skip to content

md-icon #281

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.

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 45 commits into from
Apr 23, 2016
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
62e82fe
Start of md-icon directive and service.
Mar 23, 2016
8a188e4
Return SVG element instead of string, set default styles.
Mar 23, 2016
8b6f5ff
API and caching cleanup.
Mar 24, 2016
9518ca2
var->const
Mar 24, 2016
ec761da
Make MdIconProvider available at top level so it's shared by all comp…
Mar 24, 2016
776b755
Icon set support.
Mar 24, 2016
d88144e
Font support, various refactorings.
dozingcat Mar 27, 2016
159d53d
Merge remote-tracking branch 'origin/master' into mdicon
Mar 28, 2016
e423582
Rename directive bindings.
Mar 29, 2016
3ce1334
Merge branch 'master' into mdicon
dozingcat Apr 3, 2016
22a92e1
test
dozingcat Apr 3, 2016
0e6f396
Injecting test HTTP classes.
dozingcat Apr 4, 2016
014be77
start of tests
dozingcat Apr 4, 2016
ea3beea
HTTP tests.
dozingcat Apr 5, 2016
72d167f
Improved change detection and added more tests.
dozingcat Apr 7, 2016
53aeed2
Icon namespaces, addIcon and addIconSet are now additive.
dozingcat Apr 8, 2016
7390015
cleanup
Apr 8, 2016
86d86aa
Merge branch 'master' into mdicon
Apr 8, 2016
17f40c5
fix tests
Apr 8, 2016
87dd4ac
Remove local changes.
Apr 8, 2016
82bd25e
remove unused entry
Apr 8, 2016
e4d568f
more tests
dozingcat Apr 10, 2016
af2f94b
MdIconProvider->MdIconRegistry
dozingcat Apr 10, 2016
154b5e0
Fix bug copying icons out of icon sets, and use Observable.share() co…
dozingcat Apr 13, 2016
cef1277
Remove unused imports.
dozingcat Apr 13, 2016
30f8cce
Remove unnecessary cloneNode.
dozingcat Apr 14, 2016
73a9a3f
Remove custom viewBox size.
Apr 15, 2016
03bcbed
Merge remote-tracking branch 'upstream/master' into mdicon
Apr 15, 2016
e87af2e
Updated for PR comments.
dozingcat Apr 18, 2016
062307e
formatting fixes
dozingcat Apr 18, 2016
d3682b7
Fix tslint errors.
dozingcat Apr 18, 2016
8e2ff62
Factor out common test assertions, and add tests for <svg> elements i…
dozingcat Apr 18, 2016
4b02d27
Fix tests for Edge and IE11 (element.children bad, element.childNodes…
dozingcat Apr 19, 2016
195acd3
Merge remote-tracking branch 'upstream/master' into mdicon
dozingcat Apr 19, 2016
6391cdb
Merge remote-tracking branch 'upstream/master' into mdicon
Apr 19, 2016
437ab0c
In-progress updates for PR comments.
Apr 19, 2016
364391a
More PR comments.
Apr 19, 2016
6dc1af2
Remove Renderer.detachView call since it doesn't work in IE11.
dozingcat Apr 20, 2016
796d37f
comments
dozingcat Apr 20, 2016
a4ea23e
Move test SVGs to separate file.
dozingcat Apr 20, 2016
df6415b
Use <md-icon> for menu icon.
Apr 20, 2016
04b23fa
Use watched:false in karma.config.ts.
Apr 20, 2016
37d8362
Merge remote-tracking branch 'upstream/master' into mdicon
Apr 22, 2016
6927d52
Merge remote-tracking branch 'upstream/master' into mdicon
Apr 22, 2016
08e8cbd
PR comments for test component bindings and demo styles.
dozingcat Apr 23, 2016
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
},
"devDependencies": {
"add-stream": "^1.0.0",
"angular-cli": "^0.0.28",
"angular-cli": "^0.0.29",
"angular-cli-github-pages": "^0.2.0",
"broccoli-autoprefixer": "^4.1.0",
"broccoli-caching-writer": "^2.2.1",
Expand Down
255 changes: 255 additions & 0 deletions src/components/icon/icon-provider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,255 @@
import {Injectable, Renderer} from 'angular2/core';
import {Http, Response, HTTP_PROVIDERS} from 'angular2/http';
import {AsyncSubject, Observer, Observable} from 'rxjs/Rx';

/**
* Configuration for an icon, possibly including the cached SVG element.
*/
class IconConfig {
svgElement: SVGElement = null;
constructor(public url: string, public viewBoxSize: number) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you know why we do anything with viewBox? I know material1 does the same thing, but I'm iffy on the background.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it seems like an obscure feature. You can use it to have a version of the icon that's zoomed in or out, but only from one corner because the top and left coordinates are always 0. I don't know of any especially compelling use cases, happy to remove it.

}
}

@Injectable()
export class MdIconProvider {
Copy link
Member

Choose a reason for hiding this comment

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

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SG

// IconConfig objects and cached SVG elements for individual icons.
// First level of cache is icon set (which is the empty string for the default set).
// Second level is the icon name within the set.
private _iconConfigs = new Map<string, Map<string, IconConfig>>();
Copy link
Member

Choose a reason for hiding this comment

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

Rather than doing a Map of Map, why not just treat the key as namespace:icon-name ? For the default namespace, it's just the empty string. It's okay to disallow : in both namespaces and icon names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed slightly cleaner to use nested maps rather than encoding the nesting in the key. No strong opinion either way though.


// IconConfig objects and cached SVG elements for icon sets.
// These are stored only by set name, but multiple URLs can be registered under the same name.
private _iconSetConfigs = new Map<string, [IconConfig]>();
Copy link
Member

Choose a reason for hiding this comment

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

Does the format Map<string, IconConfig[]> work here?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see now that this is a tuple type, not an array (which isn't something I had seen used before). Why is it a tuple?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes IconConfig[] works. It is just supposed to be an array.


// Cache for icons loaded by direct URLs.
private _cachedIconsByUrl = new Map<string, SVGElement>();

private _fontClassNamesByAlias = new Map<string, string>();

private _inProgressUrlFetches = new Map<string, Observable<string>>();

private _defaultViewBoxSize = 24;
private _defaultFontSetClass = 'material-icons';

constructor(private _http: Http) {
}

public addIcon(iconName: string, url: string, viewBoxSize:number=0): MdIconProvider {
Copy link
Member

Choose a reason for hiding this comment

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

Cool TypeScript trick: you can make your return type this

public addIcon(name: string, url: string, viewBoxSize: number = 0): this { ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Neat.

return this.addIconInSet('', iconName, url, viewBoxSize);
}

public addIconInSet(
Copy link
Member

Choose a reason for hiding this comment

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

I think we can separate the concept of icon namespaces and icon sets. We can treat an icon-set is a bundle of icons all in one file, which can be added to any namespace. Perhaps it makes sense to just use the term "icon bundle".

addIcon(name, url, opt_namespace)

addIconBundle(url, opt_namespace)

Then again, maybe it does make more sense to always treat a "bundle" as a namespace. Let me run this idea by the rest of the team.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's basically what it does now, I think. You can do addIconSet('animals', 'animals.svg') and addIconInSet('animals', 'cat', 'cat1.svg') and then svgIcon="animals:cat" will use the individual icon while "animals:dog" will search the set/bundle. And you can pass an empty namespace to addIconSet to add it to the default namespace.

I like your names and API, assuming we can get rid of the current optional viewBoxSize parameter.

setName: string, iconName: string, url: string, viewBoxSize:number=0): MdIconProvider {
let iconSetMap = this._iconConfigs.get(setName);
if (!iconSetMap) {
iconSetMap = new Map<string, IconConfig>();
this._iconConfigs.set(setName, iconSetMap);
}
iconSetMap.set(iconName, new IconConfig(url, viewBoxSize || this._defaultViewBoxSize));
return this;
}

public addIconSet(setName: string, url: string, viewBoxSize=0): MdIconProvider {
const config = new IconConfig(url, viewBoxSize || this._defaultViewBoxSize);
if (this._iconSetConfigs.has(setName)) {
this._iconSetConfigs.get(setName).push(config);
} else {
this._iconSetConfigs.set(setName, [config]);
}
return this;
}

public registerFontSet(alias: string, className?: string): MdIconProvider {
this._fontClassNamesByAlias.set(alias, className || alias);
return this;
}

public setDefaultViewBoxSize(size: number) {
this._defaultViewBoxSize = size;
return this;
}

public getDefaultViewBoxSize(): number {
return this._defaultViewBoxSize;
}

public setDefaultFontSetClass(className: string) {
this._defaultFontSetClass = className;
return this;
}

public getDefaultFontSetClass(): string {
return this._defaultFontSetClass;
}

loadIconFromSetByName(setName: string, iconName: string): Observable<SVGElement> {
// Return (copy of) cached icon if possible.
if (this._iconConfigs.has(setName) && this._iconConfigs.get(setName).has(iconName)) {
const config = this._iconConfigs.get(setName).get(iconName);
if (config.svgElement) {
// We already have the SVG element for this icon, return a copy.
return Observable.of(config.svgElement.cloneNode(true));
} else {
// Fetch the icon from the config's URL, cache it, and return a copy.
return this._loadIconFromConfig(config)
.do((svg: SVGElement) => config.svgElement = svg)
.map((svg: SVGElement) => svg.cloneNode(true));
}
}
// See if we have any icon sets registered for the set name.
const iconSetConfigs = this._iconSetConfigs.get(setName);
if (iconSetConfigs) {
// For all the icon set SVG elements we've fetched, see if any contain an icon with the
// requested name.
const namedIcon = this._extractIconWithNameFromAnySet(iconName, iconSetConfigs);
if (namedIcon) {
// We could cache namedSvg in _iconConfigs, but since we have to make a copy every
// time anyway, there's probably not much advantage compared to just always extracting
// it from the icon set.
return Observable.of(namedIcon.cloneNode(true));
}
// Not found in any cached icon sets. If there are icon sets with URLs that we haven't
// fetched, fetch them now and look for iconName in the results.
const iconSetFetchRequests = <[Observable<SVGElement>]>[];
iconSetConfigs.forEach((setConfig) => {
if (!setConfig.svgElement) {
iconSetFetchRequests.push(
this._loadIconSetFromConfig(setConfig)
.catch((err: any, source: any, caught: any): Observable<SVGElement> => {
// Swallow errors fetching individual URLs so the combined Observable won't
// necessarily fail.
console.log(`Loading icon set URL: ${setConfig.url} failed with error: ${err}`);
return Observable.of(null);
})
.do((svg: SVGElement) => {
// Cache SVG element.
if (svg) {
setConfig.svgElement = svg;
}
})
);
}
});
// Fetch all the icon set URLs. When the requests complete, every IconSet should have a
// cached SVG element (unless the request failed), and we can check again for the icon.
return Observable.forkJoin(iconSetFetchRequests)
.map((ignoredResults: any) => {
const foundIcon = this._extractIconWithNameFromAnySet(iconName, iconSetConfigs);
if (!foundIcon) {
throw Error(`Failed to find icon name: ${iconName} in set: ${setName}`);
}
return foundIcon;
});
}
return Observable.throw(Error(`Unknown icon name: ${iconName} in set: ${setName}`));
}

private _extractIconWithNameFromAnySet(iconName: string, setConfigs: [IconConfig]): SVGElement {
// Iterate backwards, so icon sets added later have precedence.
for (let i = setConfigs.length - 1; i >= 0; i--) {
const config = setConfigs[i];
if (config.svgElement) {
const foundIcon = this._extractSvgIconFromSet(config.svgElement, iconName, config);
if (foundIcon) {
return foundIcon;
}
}
}
return null;
}

loadIconFromUrl(url: string): Observable<SVGElement> {
if (this._cachedIconsByUrl.has(url)) {
return Observable.of(this._cachedIconsByUrl.get(url).cloneNode(true));
}
return this._loadIconFromConfig(new IconConfig(url, this._defaultViewBoxSize))
.do((svg: SVGElement) => this._cachedIconsByUrl.set(url, svg));
}

classNameForFontAlias(alias: string): string {
if (!this._fontClassNamesByAlias.has(alias)) {
throw Error('Unknown font alias: ' + alias);
}
return this._fontClassNamesByAlias.get(alias);
}

private _fetchUrl(url: string): Observable<string> {
// FIXME: This is trying to avoid sending a duplicate request for a URL when there is already
// a request in progress for that URL. But it's not working; even though we return the cached
// Observable, a second request is still sent.
console.log('*** fetchUrl: ' + url);
if (this._inProgressUrlFetches.has(url)) {
console.log("*** Using existing request");
return this._inProgressUrlFetches.get(url);
}
console.log(`*** Sending request for ${url}`);
const req = this._http.get(url)
.do((response) => {
console.log(`*** Got response for ${url}`);
console.log('*** Removing request: ' + url);
this._inProgressUrlFetches.delete(url);
})
.map((response) => response.text());
this._inProgressUrlFetches.set(url, req);
return req;
}

private _loadIconFromConfig(config: IconConfig): Observable<SVGElement> {
return this._fetchUrl(config.url)
.map(svgText => this._createSvgElementForSingleIcon(svgText, config));
}

private _loadIconSetFromConfig(config: IconConfig): Observable<SVGElement> {
return this._fetchUrl(config.url)
.map((svgText) => this._svgElementFromString(svgText));
}

private _createSvgElementForSingleIcon(responseText: string, config: IconConfig): SVGElement {
const svg = this._svgElementFromString(responseText);
this._setSvgAttributes(svg, config);
return svg;
}

private _setSvgAttributes(svg: SVGElement, config: IconConfig) {
const viewBoxSize = config.viewBoxSize || this._defaultViewBoxSize;
if (!svg.getAttribute('xmlns')) {
svg.setAttribute('xmlns', 'http://www.w3.org/2000/svg');
}
svg.setAttribute('fit', '');
svg.setAttribute('height', '100%');
svg.setAttribute('width', '100%');
svg.setAttribute('preserveAspectRatio', 'xMidYMid meet');
svg.setAttribute('viewBox',
svg.getAttribute('viewBox') || ('0 0 ' + viewBoxSize + ' ' + viewBoxSize));
svg.setAttribute('focusable', 'false'); // Disable IE11 default behavior to make SVGs focusable.
}

private _extractSvgIconFromSet(
iconSet: SVGElement, iconName: string, config: IconConfig): SVGElement {
const iconNode = iconSet.querySelector('#' + iconName);
if (!iconNode) {
return null;
}
// createElement('SVG') doesn't work as expected; the DOM ends up with
// the correct nodes, but the SVG content doesn't render. Instead we
// have to create an empty SVG node using innerHTML and append its content.
// http://stackoverflow.com/questions/23003278/svg-innerhtml-in-firefox-can-not-display
const svg = this._svgElementFromString('<svg></svg>');
svg.appendChild(iconNode);
this._setSvgAttributes(svg, config);
return svg;
}

private _svgElementFromString(str: string): SVGElement {
// TODO: Is there a better way than innerHTML? Renderer doesn't appear to have a method for
// creating an element from an HTML string.
const div = document.createElement('DIV');
div.innerHTML = str;
const svg = <SVGElement>div.querySelector('svg');
if (!svg) {
throw Error('<svg> tag not found');
}
return svg;
}
}
19 changes: 19 additions & 0 deletions src/components/icon/icon.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
@import "variables";
@import "default-theme";

/** The width/height of the icon element. */
$md-icon-size: 24px !default;

/**
This works because we're using ViewEncapsulation.None. If we used the default
encapsulation, the selector would need to be ":host".
*/
md-icon {
background-repeat: no-repeat;
display: inline-block;
fill: currentColor;
height: $md-icon-size;
margin: auto;
vertical-align: middle;
width: $md-icon-size;
Copy link
Member

Choose a reason for hiding this comment

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

Too much indent
(should only be +2)

}
Loading