Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Commit 841e8b2

Browse files
Splaktarmmalerba
authored andcommitted
fix(icon): stop breaking id references when caching icon ids (#11342)
<!-- Filling out this template is required! Do not delete it when submitting a Pull Request! Without this information, your Pull Request may be auto-closed. --> ## PR Checklist Please check that your PR fulfills the following requirements: - [x] The commit message follows [our guidelines](https://github.com/angular/material/blob/master/.github/CONTRIBUTING.md#-commit-message-format) - [x] Tests for the changes have been added or this is not a bug fix / enhancement - [x] Docs have been added, updated, or were not required ## PR Type What kind of change does this PR introduce? <!-- Please check the one that applies to this PR using "x". --> ``` [x] Bugfix [ ] Enhancement [x] Documentation content changes [ ] Code style update (formatting, local variables) [ ] Refactoring (no functional changes, no api changes) [ ] Build related changes [ ] CI related changes [ ] Infrastructure changes [ ] Other... Please describe: ``` ## What is the current behavior? SVGs with embedded `id`s can be broken when they are read out of the icon cache. <!-- Please describe the current behavior that you are modifying and link to one or more relevant issues. --> Issue Number: Fixes #8689 ## What is the new behavior? SVGs with embedded `id`s can be read out of the icon cache without breaking because the `id` references are now updated in addition to the `id`s themselves. ## Does this PR introduce a breaking change? ``` [ ] Yes [x] No ``` <!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. --> <!-- Note that breaking changes are highly unlikely to get merged to master unless the validation is clear and the use case is critical. --> ## Other information Thanks to @ystreibel for starting this work in #11315!
1 parent f616b25 commit 841e8b2

File tree

6 files changed

+154
-105
lines changed

6 files changed

+154
-105
lines changed

src/components/icon/demoLoadSvgIconsFromUrl/script.js

Lines changed: 4 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
21
angular.module('appSvgIconSets', ['ngMaterial'])
32
.controller('DemoCtrl', function($scope) {})
4-
.config(['$mdIconProvider', function($mdIconProvider) {
3+
.config(function($mdIconProvider) {
54
$mdIconProvider
65
.iconSet('social', 'img/icons/sets/social-icons.svg', 24)
76
.iconSet('symbol', 'img/icons/sets/symbol-icons.svg', 24)
87
.defaultIconSet('img/icons/sets/core-icons.svg', 24);
9-
}]);
8+
});
Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,13 @@
1-
21
angular.module('appUsingTemplateCache', ['ngMaterial'])
32
.controller('DemoCtrl', function($scope) {})
43
.config(function($mdIconProvider) {
5-
64
// Register icon IDs with sources. Future $mdIcon( <id> ) lookups
75
// will load by url and retrieve the data via the $templateRequest
8-
96
$mdIconProvider
10-
.iconSet('core', 'img/icons/sets/core-icons.svg',24)
11-
.icon('social:cake', 'img/icons/cake.svg',24);
12-
7+
.iconSet('core', 'img/icons/sets/core-icons.svg', 24)
8+
.icon('social:cake', 'img/icons/cake.svg', 24);
139
})
1410
.run(function($templateRequest) {
15-
1611
var urls = [
1712
'img/icons/sets/core-icons.svg',
1813
'img/icons/cake.svg',
@@ -21,10 +16,7 @@ angular.module('appUsingTemplateCache', ['ngMaterial'])
2116

2217
// Pre-fetch icons sources by URL and cache in the $templateCache...
2318
// subsequent $templateRequest calls will look there first.
24-
2519
angular.forEach(urls, function(url) {
2620
$templateRequest(url);
2721
});
28-
29-
})
30-
;
22+
});

src/components/icon/icon.spec.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,11 @@ describe('MdIcon service', function() {
419419
$scope = $rootScope;
420420

421421
$templateCache.put('android.svg' , '<svg><g id="android"></g></svg>');
422+
$templateCache.put('angular-logo.svg',
423+
'<svg><g id="angular"></g><defs><filter id="shadow"></filter>' +
424+
'<g id="bg" fill="#000000"><path d="M10 10"/></g></defs>' +
425+
'<path filter="url(#shadow)"></path><use x="0" y="0" xlink:href="#bg"></use>' +
426+
'</svg>');
422427
$templateCache.put('social.svg' , '<svg><g id="s1"></g><g id="s2"></g></svg>');
423428
$templateCache.put('symbol.svg' , '<svg><symbol id="s1"></symbol><symbol id="s2" viewBox="0 0 32 32"></symbol></svg>');
424429
$templateCache.put('core.svg' , '<svg><g id="c1"></g><g id="c2" class="core"></g></svg>');
@@ -587,6 +592,25 @@ describe('MdIcon service', function() {
587592

588593
$scope.$digest();
589594
});
595+
596+
it('should suffix duplicated ids and refs', function() {
597+
// Just request the icon to be stored in the cache.
598+
$mdIcon('angular-logo.svg');
599+
600+
$scope.$digest();
601+
602+
$mdIcon('angular-logo.svg').then(function(el) {
603+
expect(el.querySelector('defs').firstChild.id).toMatch(/.+_cache\d+/g);
604+
expect(el.querySelectorAll('path')[1].attributes.filter.value.split(/url\(#(.*)\)$/g)[1])
605+
.toMatch(/.+_cache[0-9]+/g);
606+
expect(el.querySelectorAll('path')[1].attributes.filter.value.split(/url\(#(.*)\)$/g)[1])
607+
.toEqual(el.querySelector('defs').firstChild.id);
608+
expect(el.querySelector('use').attributes['xlink:href'].value.split(/#(.*)/)[1]).toEqual(
609+
el.querySelector('defs').children[1].id);
610+
});
611+
612+
$scope.$digest();
613+
});
590614
});
591615

592616
describe('icon in a group is not found', function() {

src/components/icon/js/iconDirective.js

Lines changed: 58 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,11 @@ angular
2424
* `md-icon` lets you consume an icon font by letting you reference specific icons in that font
2525
* by name rather than character code.
2626
*
27-
* ### SVG
28-
* For SVGs, the problem with using `<img>` or a CSS `background-image` is that you can't take
29-
* advantage of some SVG features, such as styling specific parts of the icon with CSS or SVG
30-
* animation.
31-
*
32-
* `md-icon` makes it easier to use SVG icons by *inlining* the SVG into an `<svg>` element in the
33-
* document. The most straightforward way of referencing an SVG icon is via URL, just like a
34-
* traditional `<img>`. `$mdIconProvider`, as a convenience, lets you _name_ an icon so you can
35-
* reference it by name instead of URL throughout your templates.
36-
*
37-
* Additionally, you may not want to make separate HTTP requests for every icon, so you can bundle
38-
* your SVG icons together and pre-load them with $mdIconProvider as an icon set. An icon set can
39-
* also be given a name, which acts as a namespace for individual icons, so you can reference them
40-
* like `"social:cake"`.
41-
*
42-
* When using SVGs, both external SVGs (via URLs) or sets of SVGs [from icon sets] can be
43-
* easily loaded and used. When using font-icons, developers must follow three (3) simple steps:
27+
* When using font-icons, developers must follow three (3) simple steps:
4428
*
4529
* <ol>
4630
* <li>Load the font library. e.g.<br/>
47-
* `<link href="https://fonts.googleapis.com/icon?family=Material+Icons"
48-
* rel="stylesheet">`
31+
* `<link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">`
4932
* </li>
5033
* <li>
5134
* Use either (a) font-icon class names or (b) a fontset and a font ligature to render the font glyph by
@@ -63,26 +46,36 @@ angular
6346
* </li>
6447
* </ol>
6548
*
66-
* Full details for these steps can be found:
49+
* Full details for these steps can be found in the
50+
* <a href="http://google.github.io/material-design-icons/#icon-font-for-the-web" target="_blank">
51+
* Material Design Icon font for the web docs</a>.
6752
*
68-
* <ul>
69-
* <li>http://google.github.io/material-design-icons/</li>
70-
* <li>http://google.github.io/material-design-icons/#icon-font-for-the-web</li>
71-
* </ul>
53+
* You can browse and search the Material Design icon style <code>.material-icons</code>
54+
* in the <a href="https://material.io/tools/icons/" target="_blank">Material Design Icons tool</a>.
7255
*
73-
* The Material Design icon style <code>.material-icons</code> and the icon font references are published in
74-
* Material Design Icons:
56+
* ### SVG
57+
* For SVGs, the problem with using `<img>` or a CSS `background-image` is that you can't take
58+
* advantage of some SVG features, such as styling specific parts of the icon with CSS or SVG
59+
* animation.
7560
*
76-
* <ul>
77-
* <li>https://design.google.com/icons/</li>
78-
* <li>https://design.google.com/icons/#ic_accessibility</li>
79-
* </ul>
61+
* `md-icon` makes it easier to use SVG icons by *inlining* the SVG into an `<svg>` element in the
62+
* document. The most straightforward way of referencing an SVG icon is via URL, just like a
63+
* traditional `<img>`. `$mdIconProvider`, as a convenience, lets you _name_ an icon so you can
64+
* reference it by name instead of URL throughout your templates.
65+
*
66+
* Additionally, you may not want to make separate HTTP requests for every icon, so you can bundle
67+
* your SVG icons together and pre-load them with `$mdIconProvider` as an icon set. An icon set can
68+
* also be given a name, which acts as a namespace for individual icons, so you can reference them
69+
* like `"social:cake"`.
70+
*
71+
* When using SVGs, both external SVGs (via URLs) or sets of SVGs (from icon sets) can be
72+
* easily loaded and used.
8073
*
8174
* ### Localization
8275
*
83-
* Because an `md-icon` element's text content is not intended to be translated, it is recommended to declare the text
84-
* content for an `md-icon` element in its start tag. Instead of using the HTML text content, consider using `ng-bind`
85-
* with a scope variable or literal string.
76+
* Because an `md-icon` element's text content is not intended to be translated, it is recommended
77+
* to declare the text content for an `md-icon` element in its start tag. Instead of using the HTML
78+
* text content, consider using `ng-bind` with a scope variable or literal string.
8679
*
8780
* Examples:
8881
*
@@ -91,20 +84,25 @@ angular
9184
* <li>`<md-icon ng-bind="'menu'"></md-icon>`
9285
* </ul>
9386
*
94-
* <h2 id="material_design_icons">Material Design Icons</h2>
95-
* Using the Material Design Icon-Selector, developers can easily and quickly search for a Material Design font-icon and
96-
* determine its textual name and character reference code. Click on any icon to see the slide-up information
97-
* panel with details regarding a SVG download or information on the font-icon usage.
87+
* <h2 id="material_design_icons">Material Design Icons tool</h2>
88+
* Using the Material Design Icons tool, developers can easily and quickly search for a specific
89+
* open source Material Design icon. The search is in the top left. Below search, you can select
90+
* from the new icon themes or filter by icon category.
9891
*
99-
* <a href="https://material.io/tools/icons/?icon=accessibility&style=baseline" target="_blank" style="border-bottom:none;">
100-
* <img src="https://cloud.githubusercontent.com/assets/210413/7902490/fe8dd14c-0780-11e5-98fb-c821cc6475e6.png"
101-
* aria-label="Material Design Icon-Selector" style="max-width:75%;padding-left:10%">
92+
* <a href="https://material.io/tools/icons/" target="_blank" style="border-bottom:none;">
93+
* <img src="https://user-images.githubusercontent.com/3506071/41942584-ef0695d0-796d-11e8-9436-44f25023a111.png"
94+
* aria-label="Material Design Icons tool" style="max-width:95%">
10295
* </a>
10396
*
104-
* <span class="image_caption">
105-
* Click on the image above to link to the
106-
* <a href="https://design.google.com/icons/#ic_accessibility" target="_blank">Material Design Icon-Selector</a>.
107-
* </span>
97+
* <div class="md-caption" style="text-align: center; margin-bottom: 24px">
98+
* Click on the image above to open the
99+
* <a href="https://material.io/tools/icons/" target="_blank">Material Design Icons tool</a>.
100+
* </div>
101+
*
102+
* Click on any icon, then click on the "Selected Icon" chevron to see the slide-up
103+
* information panel with details regarding a SVG download and information on the font-icon's
104+
* textual name. This panel also allows you to select a black on transparent or white on transparent
105+
* icon and to change the icon size. These settings only affect the downloaded icons.
108106
*
109107
* @param {string} md-font-icon String name of CSS icon associated with the font-face will be used
110108
* to render the icon. Requires the fonts and the named CSS styles to be preloaded.
@@ -128,34 +126,35 @@ angular
128126
* When using SVGs:
129127
* <hljs lang="html">
130128
*
131-
* <!-- Icon ID; may contain optional icon set prefix; icons must registered using $mdIconProvider -->
132-
* <md-icon md-svg-icon="social:android" aria-label="android " ></md-icon>
129+
*<!-- Icon ID; may contain optional icon set prefix.
130+
* Icons must be registered using $mdIconProvider. -->
131+
*<md-icon md-svg-icon="social:android" aria-label="android " ></md-icon>
133132
*
134-
* <!-- Icon urls; may be preloaded in templateCache -->
135-
* <md-icon md-svg-src="/android.svg" aria-label="android " ></md-icon>
136-
* <md-icon md-svg-src="{{ getAndroid() }}" aria-label="android " ></md-icon>
133+
*<!-- Icon urls; may be preloaded in templateCache -->
134+
*<md-icon md-svg-src="/android.svg" aria-label="android " ></md-icon>
135+
*<md-icon md-svg-src="{{ getAndroid() }}" aria-label="android " ></md-icon>
137136
*
138137
* </hljs>
139138
*
140139
* Use the <code>$mdIconProvider</code> to configure your application with
141-
* svg iconsets.
140+
* SVG icon sets.
142141
*
143142
* <hljs lang="js">
144-
* angular.module('appSvgIconSets', ['ngMaterial'])
145-
* .controller('DemoCtrl', function($scope) {})
146-
* .config(function($mdIconProvider) {
147-
* $mdIconProvider
148-
* .iconSet('social', 'img/icons/sets/social-icons.svg', 24)
149-
* .defaultIconSet('img/icons/sets/core-icons.svg', 24);
150-
* });
143+
* angular.module('appSvgIconSets', ['ngMaterial'])
144+
* .controller('DemoCtrl', function($scope) {})
145+
* .config(function($mdIconProvider) {
146+
* $mdIconProvider
147+
* .iconSet('social', 'img/icons/sets/social-icons.svg', 24)
148+
* .defaultIconSet('img/icons/sets/core-icons.svg', 24);
149+
* });
151150
* </hljs>
152151
*
153152
*
154153
* When using Font Icons with classnames:
155154
* <hljs lang="html">
156155
*
157-
* <md-icon md-font-icon="android" aria-label="android" ></md-icon>
158-
* <md-icon class="icon_home" aria-label="Home" ></md-icon>
156+
* <md-icon md-font-icon="android" aria-label="android" ></md-icon>
157+
* <md-icon class="icon_home" aria-label="Home"></md-icon>
159158
*
160159
* </hljs>
161160
*

0 commit comments

Comments
 (0)