fix(icon): remove IDs from source icon set from rendered output (#8266)

This commit is contained in:
Jeremy Elbourn
2017-11-10 11:08:16 -08:00
committed by Joey Perrott
parent ba36d3ab50
commit 76806e3fef
3 changed files with 33 additions and 25 deletions
+11 -10
View File
@@ -8,39 +8,40 @@
/**
* Fake URLs and associated SVG documents used by tests.
* The ID attribute is used to load the icons, the name attribute is only used for testing.
* @docs-private
*/
export const FAKE_SVGS = {
cat: '<svg><path id="meow"></path></svg>',
dog: '<svg><path id="woof"></path></svg>',
cat: '<svg><path id="meow" name="meow"></path></svg>',
dog: '<svg><path id="woof" name="woof"></path></svg>',
farmSet1: `
<svg>
<defs>
<g id="pig"><path id="oink"></path></g>
<g id="cow"><path id="moo"></path></g>
<g id="pig" name="pig"><path name="oink"></path></g>
<g id="cow" name="cow"><path name="moo"></path></g>
</defs>
</svg>
`,
farmSet2: `
<svg>
<defs>
<g id="cow"><path id="moo moo"></path></g>
<g id="sheep"><path id="baa"></path></g>
<g id="cow" name="cow"><path name="moo moo"></path></g>
<g id="sheep" name="sheep"><path name="baa"></path></g>
</defs>
</svg>
`,
farmSet3: `
<svg>
<symbol id="duck">
<path id="quack"></path>
<symbol id="duck" name="duck">
<path id="quack" name="quack"></path>
</symbol>
</svg>
`,
arrows: `
<svg>
<defs>
<svg id="left-arrow"><path id="left"></path></svg>
<svg id="right-arrow"><path id="right"></path></svg>
<svg id="left-arrow" name="left-arrow"><path name="left"></path></svg>
<svg id="right-arrow" name="right-arrow"><path name="right"></path></svg>
</defs>
</svg> `
};
+12 -9
View File
@@ -365,23 +365,28 @@ export class MatIconRegistry {
* returns it. Returns null if no matching element is found.
*/
private _extractSvgIconFromSet(iconSet: SVGElement, iconName: string): SVGElement | null {
const iconNode = iconSet.querySelector('#' + iconName);
const iconSource = iconSet.querySelector('#' + iconName);
if (!iconNode) {
if (!iconSource) {
return null;
}
// Clone the element and remove the ID to prevent multiple elements from being added
// to the page with the same ID.
const iconElement = iconSource.cloneNode(true) as Element;
iconElement.id = '';
// 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') {
return this._setSvgAttributes(iconNode.cloneNode(true) as SVGElement);
if (iconElement.nodeName.toLowerCase() === 'svg') {
return this._setSvgAttributes(iconElement 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));
if (iconElement.nodeName.toLowerCase() === 'symbol') {
return this._setSvgAttributes(this._toSvgElement(iconElement));
}
// createElement('SVG') doesn't work as expected; the DOM ends up with
@@ -391,7 +396,7 @@ export class MatIconRegistry {
// http://stackoverflow.com/questions/23003278/svg-innerhtml-in-firefox-can-not-display
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));
svg.appendChild(iconElement);
return this._setSvgAttributes(svg);
}
@@ -400,8 +405,6 @@ export class MatIconRegistry {
* Creates a DOM element from the given SVG string.
*/
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 = div.querySelector('svg') as SVGElement;
+10 -6
View File
@@ -17,6 +17,7 @@ function sortedClassNames(element: Element): string[] {
* Verifies that an element contains a single <svg> child element, and returns that child.
*/
function verifyAndGetSingleSvgChild(element: SVGElement): SVGElement {
expect(element.id).toBeFalsy();
expect(element.childNodes.length).toBe(1);
const svgChild = element.childNodes[0] as SVGElement;
expect(svgChild.tagName.toLowerCase()).toBe('svg');
@@ -31,7 +32,9 @@ function verifyPathChildElement(element: Element, attributeValue: string): void
expect(element.childNodes.length).toBe(1);
const pathElement = element.childNodes[0] as SVGPathElement;
expect(pathElement.tagName.toLowerCase()).toBe('path');
expect(pathElement.getAttribute('id')).toBe(attributeValue);
// The testing data SVGs have the name attribute set for verification.
expect(pathElement.getAttribute('name')).toBe(attributeValue);
}
@@ -190,7 +193,7 @@ describe('MatIcon', () => {
svgChild = svgElement.childNodes[0];
// The first <svg> child should be the <g id="pig"> element.
expect(svgChild.tagName.toLowerCase()).toBe('g');
expect(svgChild.getAttribute('id')).toBe('pig');
expect(svgChild.getAttribute('name')).toBe('pig');
verifyPathChildElement(svgChild, 'oink');
// Change the icon, and the SVG element should be replaced.
@@ -200,7 +203,7 @@ describe('MatIcon', () => {
svgChild = svgElement.childNodes[0];
// The first <svg> child should be the <g id="cow"> element.
expect(svgChild.tagName.toLowerCase()).toBe('g');
expect(svgChild.getAttribute('id')).toBe('cow');
expect(svgChild.getAttribute('name')).toBe('cow');
verifyPathChildElement(svgChild, 'moo');
});
@@ -224,7 +227,8 @@ describe('MatIcon', () => {
svgChild = svgElement.childNodes[0];
// The <svg> child should be the <g id="pig"> element.
expect(svgChild.tagName.toLowerCase()).toBe('g');
expect(svgChild.getAttribute('id')).toBe('pig');
expect(svgChild.getAttribute('name')).toBe('pig');
expect(svgChild.getAttribute('id')).toBe('');
expect(svgChild.childNodes.length).toBe(1);
verifyPathChildElement(svgChild, 'oink');
@@ -237,7 +241,7 @@ describe('MatIcon', () => {
svgChild = svgElement.childNodes[0];
// The first <svg> child should be the <g id="cow"> element.
expect(svgChild.tagName.toLowerCase()).toBe('g');
expect(svgChild.getAttribute('id')).toBe('cow');
expect(svgChild.getAttribute('name')).toBe('cow');
expect(svgChild.childNodes.length).toBe(1);
verifyPathChildElement(svgChild, 'moo moo');
});
@@ -259,7 +263,7 @@ describe('MatIcon', () => {
expect(svgElement.querySelector('symbol')).toBeFalsy();
expect(svgElement.childNodes.length).toBe(1);
expect(firstChild.nodeName.toLowerCase()).toBe('path');
expect((firstChild as HTMLElement).getAttribute('id')).toBe('quack');
expect((firstChild as HTMLElement).getAttribute('name')).toBe('quack');
});
it('should not wrap <svg> elements in icon sets in another svg tag', () => {