Skip to content

Commit

Permalink
fix(icon): handle icons as <symbol> nodes
Browse files Browse the repository at this point in the history
Fixes icons not being rendered if they're defined as a `<symbol>` inside the source file.

Fixes angular#4680.
  • Loading branch information
crisbeto committed May 20, 2017
1 parent c946631 commit 95198d1
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 8 deletions.
8 changes: 8 additions & 0 deletions src/lib/icon/fake-svgs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ const FAKE_SVGS = (() => {
</svg>
`);

svgs.set('farm-set-3.svg', `
<svg>
<symbol id="duck">
<path id="quack"></path>
</symbol>
</svg>
`);

svgs.set('arrow-set.svg', `
<svg>
<defs>
Expand Down
26 changes: 25 additions & 1 deletion src/lib/icon/icon-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,14 +327,24 @@ export class MdIconRegistry {
*/
private _extractSvgIconFromSet(iconSet: SVGElement, iconName: string): SVGElement {
const iconNode = iconSet.querySelector('#' + iconName);

if (!iconNode) {
return null;
}

// If the icon node is itself an <svg> node, clone and return it directly. If not, set it as
// the content of a new <svg> node.
if (iconNode.tagName.toLowerCase() == 'svg') {
if (iconNode.tagName.toLowerCase() === 'svg') {
return this._setSvgAttributes(iconNode.cloneNode(true) as SVGElement);
}

// If the node is a <symbol>, it won't be rendered so we have to convert it into <svg>. Note
// that the same could be achieved by referring to it via <use href="#id">, however the <use>
// tag is problematic on Firefox, because it needs to include the current page path.
if (iconNode.nodeName.toLowerCase() === 'symbol') {
return this._setSvgAttributes(this._toSvgElement(iconNode));
}

// 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.
Expand All @@ -343,6 +353,7 @@ export class MdIconRegistry {
const svg = this._svgElementFromString('<svg></svg>');
// Clone the node so we don't remove it from the parent icon set element.
svg.appendChild(iconNode.cloneNode(true));

return this._setSvgAttributes(svg);
}

Expand All @@ -361,6 +372,19 @@ export class MdIconRegistry {
return svg;
}

/**
* Converts an element into an SVG node by cloning all of its children.
*/
private _toSvgElement(element: Element): SVGElement {
let svg = this._svgElementFromString('<svg></svg>');

for (let i = 0; i < element.children.length; i++) {
svg.appendChild(element.children[i].cloneNode(true));
}

return svg;
}

/**
* Sets the default attributes for an SVG element to be used as an icon.
*/
Expand Down
36 changes: 29 additions & 7 deletions src/lib/icon/icon.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,31 @@ import {wrappedErrorMessage} from '../core/testing/wrapped-error-message';


/** Returns the CSS classes assigned to an element as a sorted array. */
const sortedClassNames = (elem: Element) => elem.className.split(' ').sort();
function sortedClassNames(element: Element): string[] {
return element.className.split(' ').sort();
}

/**
* Verifies that an element contains a single <svg> child element, and returns that child.
*/
const verifyAndGetSingleSvgChild = (element: SVGElement): any => {
function verifyAndGetSingleSvgChild(element: SVGElement): SVGElement {
expect(element.childNodes.length).toBe(1);
const svgChild = <Element>element.childNodes[0];
const svgChild = element.childNodes[0] as SVGElement;
expect(svgChild.tagName.toLowerCase()).toBe('svg');
return svgChild;
};
}

/**
* Verifies that an element contains a single <path> child element whose "id" attribute has
* the specified value.
*/
const verifyPathChildElement = (element: Element, attributeValue: string) => {
function verifyPathChildElement(element: Element, attributeValue: string): void {
expect(element.childNodes.length).toBe(1);
const pathElement = <Element>element.childNodes[0];
const pathElement = element.childNodes[0] as SVGPathElement;
expect(pathElement.tagName.toLowerCase()).toBe('path');
expect(pathElement.getAttribute('id')).toBe(attributeValue);
};
}


describe('MdIcon', () => {

Expand Down Expand Up @@ -240,6 +243,25 @@ describe('MdIcon', () => {
expect(httpRequestUrls.sort()).toEqual(['farm-set-1.svg', 'farm-set-2.svg']);
});

it('should unwrap <symbol> nodes', () => {
mdIconRegistry.addSvgIconSetInNamespace('farm', trust('farm-set-3.svg'));

const fixture = TestBed.createComponent(MdIconFromSvgNameTestApp);
const testComponent = fixture.componentInstance;
const mdIconElement = fixture.debugElement.nativeElement.querySelector('md-icon');

testComponent.iconName = 'farm:duck';
fixture.detectChanges();

const svgElement = verifyAndGetSingleSvgChild(mdIconElement);
const firstChild = svgElement.children[0];

expect(svgElement.querySelector('symbol')).toBeFalsy();
expect(svgElement.children.length).toBe(1);
expect(firstChild.nodeName.toLowerCase()).toBe('path');
expect(firstChild.getAttribute('id')).toBe('quack');
});

it('should not wrap <svg> elements in icon sets in another svg tag', () => {
mdIconRegistry.addSvgIconSet(trust('arrow-set.svg'));

Expand Down

0 comments on commit 95198d1

Please sign in to comment.