Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Commit

Permalink
fix(icon): rename nodes id's from cache to prevent duplicates.
Browse files Browse the repository at this point in the history
Currently we always rename the icons id's on cache initialization, but that causes problems, when using the cached icon multiple times in the app.
A super elegant solution, is to transform the ids, when the icon is requested from the cache.

Closes #7468
  • Loading branch information
devversion authored and ThomasBurleson committed Mar 30, 2016
1 parent 81c15e3 commit 67bd35d
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 22 deletions.
40 changes: 35 additions & 5 deletions src/components/icon/icon.spec.js
Expand Up @@ -478,6 +478,41 @@ describe('mdIcon service', function() {
});
});

describe('icon is cached', function() {

it('should prevent duplicate ids', function() {
var firstId;

$mdIcon('android.svg').then(function(el) {
// First child is in our case always the node with an id.
firstId = el.firstChild.id;
});

$scope.$digest();

$mdIcon('android.svg').then(function(el) {
expect(el.firstChild.id).not.toBe(firstId);
});

$scope.$digest();

});

it('should suffix duplicated ids', function() {
// Just request the icon to be stored in the cache.
$mdIcon('android.svg');

$scope.$digest();

$mdIcon('android.svg').then(function(el) {
expect(el.firstChild.id).toMatch(/.+_cache[0-9]+/g);
});

$scope.$digest();
});

});

describe('icon group is not found', function() {
it('should log Error', function() {
var msg;
Expand Down Expand Up @@ -509,11 +544,6 @@ describe('mdIcon service', function() {
function updateDefaults(svg) {
svg = angular.element(svg)[0];

svg.removeAttribute('id');
angular.forEach(svg.querySelectorAll('[id]'), function(item) {
item.removeAttribute('id');
});

angular.forEach({
'xmlns' : 'http://www.w3.org/2000/svg',
'fit' : '',
Expand Down
38 changes: 21 additions & 17 deletions src/components/icon/js/iconService.js
Expand Up @@ -316,9 +316,9 @@

},

$get : ['$http', '$q', '$log', '$templateCache', function($http, $q, $log, $templateCache) {
$get : ['$http', '$q', '$log', '$templateCache', '$mdUtil', function($http, $q, $log, $templateCache, $mdUtil) {
this.preloadIcons($templateCache);
return MdIconService(config, $http, $q, $log, $templateCache);
return MdIconService(config, $http, $q, $log, $templateCache, $mdUtil);
}]
};

Expand Down Expand Up @@ -373,7 +373,7 @@
*/

/* @ngInject */
function MdIconService(config, $http, $q, $log, $templateCache) {
function MdIconService(config, $http, $q, $log, $templateCache, $mdUtil) {
var iconCache = {};
var urlRegex = /[-\w@:%\+.~#?&//=]{2,}\.[a-z]{2,4}\b(\/[-\w@:%\+.~#?&//=]*)?/i;
var dataUrlRegex = /^data:image\/svg\+xml[\s*;\w\-\=]*?(base64)?,(.*)$/i;
Expand All @@ -393,7 +393,7 @@
// If already loaded and cached, use a clone of the cached icon.
// Otherwise either load by URL, or lookup in the registry and then load by URL, and cache.

if ( iconCache[id] ) return $q.when( iconCache[id].clone() );
if ( iconCache[id] ) return $q.when( transformClone(iconCache[id]) );
if ( urlRegex.test(id) || dataUrlRegex.test(id) ) return loadByURL(id).then( cacheIcon(id) );
if ( id.indexOf(':') == -1 ) id = '$default:' + id;

Expand All @@ -418,24 +418,28 @@
return result;
}

function transformClone(cacheElement) {
var clone = cacheElement.clone();
var cacheSuffix = '_cache' + $mdUtil.nextUid();

// We need to modify for each cached icon the id attributes.
// This is needed because SVG id's are treated as normal DOM ids
// and should not have a duplicated id.
if (clone.id) clone.id += cacheSuffix;
angular.forEach(clone.querySelectorAll('[id]'), function (item) {
item.id += cacheSuffix;
});

return clone;
}

/**
* Prepare and cache the loaded icon for the specified `id`
*/
function cacheIcon( id ) {

return function updateCache( _icon ) {
var icon = isIcon(_icon) ? _icon : new Icon(_icon, config[id]);

//clear id attributes to prevent aria issues
var elem = icon.element;
elem.removeAttribute('id');

angular.forEach(elem.querySelectorAll('[id]'), function(item) {
item.removeAttribute('id');
});

iconCache[id] = icon;

return function updateCache( icon ) {
iconCache[id] = isIcon(icon) ? icon : new Icon(icon, config[id]);

return iconCache[id].clone();
};
Expand Down

0 comments on commit 67bd35d

Please sign in to comment.