refactor(icon-service): cache svg icons as text (#10202)

* refactor(icon-service): cache svg icons as text * refactor(icon): return SafeHtml from IconService * spec(icon-service): update * refactor(icon): remove lifecycle hook changes * docs(icon, icon-service): update after changes * docs(icon-service): update * refactor(icon): update template and theme * chore(*): update svg assets * refactor(icon-service): simplify family initialization * spec(icon-service): fix failing test * refactor(advanced-filtering): show friendly name for igx-select text * refactor(excel-filter): use condition friendly name * refactor(icon, icon-service): simplify code * test(grid): dont include the svg icon in the query for dropdown item * refactor(icon, icon service): cache svg icons as SafeHtml * refactor(icon): replace svg wrapper with a div * refactor(demos): add a new test icon * refactor(icon): set svg and div to display block Co-authored-by: Hristo Anastasov <[email protected]> Co-authored-by: Hristo <[email protected]>
IgniteUI · Oct 11, 2021 · 791b9a5 · 791b9a5
1 parent dc629e2
commit 791b9a5
Show file tree

Hide file tree

Showing 19 changed files with 70 additions and 239 deletions.
diff --git a/projects/igniteui-angular/src/lib/core/styles/components/icon/_icon-theme.scss b/projects/igniteui-angular/src/lib/core/styles/components/icon/_icon-theme.scss
@@ -64,14 +64,12 @@
         font-size: $igx-icon-font-size;
         color: --var($theme, 'color');
 
+        div,
         svg {
+            display: block;
             width: inherit;
             height: inherit;
             fill: currentColor;
-
-            use {
-                pointer-events: none;
-            }
         }
     }
 

diff --git a/...gular/src/lib/grids/filtering/advanced-filtering/advanced-filtering-dialog.component.html b/...gular/src/lib/grids/filtering/advanced-filtering/advanced-filtering-dialog.component.html
@@ -162,7 +162,7 @@ <h6 class="igx-filter-empty__title">
                         <igx-icon family="imx-icons" [name]="selectedColumn.filters.condition(conditionSelect.value).iconName">
                         </igx-icon>
                     </igx-prefix>
-                    <igx-select-item *ngFor="let condition of getConditionList()" [value]="condition">
+                    <igx-select-item *ngFor="let condition of getConditionList()" [value]="condition" [text]="getConditionFriendlyName(condition)">
                         <div class="igx-grid__filtering-dropdown-items">
                             <igx-icon family="imx-icons"
                                       [name]="selectedColumn.filters.condition(condition).iconName">

diff --git a/...angular/src/lib/grids/filtering/excel-style/excel-style-default-expression.component.html b/...angular/src/lib/grids/filtering/excel-style/excel-style-default-expression.component.html
@@ -8,7 +8,7 @@
         <igx-icon *ngIf="expressionUI.expression.condition" family="imx-icons" [name]="getIconName()"></igx-icon>
         <igx-icon *ngIf="!expressionUI.expression.condition">filter_list</igx-icon>
     </igx-prefix>
-    <igx-select-item *ngFor="let condition of conditions" [value]="condition" [selected]="isConditionSelected(condition)">
+    <igx-select-item *ngFor="let condition of conditions" [value]="condition" [text]="getConditionFriendlyName(condition)" [selected]="isConditionSelected(condition)">
         <div class="igx-grid__filtering-dropdown-items">
             <igx-icon family="imx-icons" [name]="getCondition(condition).iconName"></igx-icon>
             <span class="igx-grid__filtering-dropdown-text">{{translateCondition(condition)}}</span>

diff --git a/...i-angular/src/lib/grids/filtering/excel-style/excel-style-default-expression.component.ts b/...i-angular/src/lib/grids/filtering/excel-style/excel-style-default-expression.component.ts
@@ -155,6 +155,10 @@ export class IgxExcelStyleDefaultExpressionComponent implements AfterViewInit {
         return this.column.filters.condition(value);
     }
 
+    public getConditionFriendlyName(name: string): string {
+        return this.grid.resourceStrings[`igx_grid_filter_${name}`] || name;
+    }
+
     public onValuesInput(eventArgs) {
         this.expressionUI.expression.searchVal = DataUtil.parseValue(this.column.dataType, eventArgs.target.value);
     }

diff --git a/projects/igniteui-angular/src/lib/icon/icon.component.html b/projects/igniteui-angular/src/lib/icon/icon.component.html
@@ -5,9 +5,7 @@
 </ng-template>
 
 <ng-template #svgImage>
-    <svg>
-        <use [attr.href]="getSvgKey"></use>
-    </svg>
+    <div [innerHTML]="getSvg"></div>
 </ng-template>
 
 <ng-container *ngTemplateOutlet="template"></ng-container>
diff --git a/projects/igniteui-angular/src/lib/icon/icon.component.ts b/projects/igniteui-angular/src/lib/icon/icon.component.ts
@@ -3,6 +3,7 @@ import { IgxIconService } from './icon.service';
 import { first, takeUntil } from 'rxjs/operators';
 import { Subject } from 'rxjs';
 import { DeprecateProperty } from '../core/deprecateDecorators';
+import { SafeHtml } from '@angular/platform-browser';
 
 /**
  * Icon provides a way to include material icons to markup
@@ -114,16 +115,19 @@ export class IgxIconComponent implements OnInit, OnDestroy {
 
     private destroy$ = new Subject<void>();
 
-    constructor(public el: ElementRef,
-                private iconService: IgxIconService,
-                private ref: ChangeDetectorRef) {
+    constructor(
+        public el: ElementRef,
+        private iconService: IgxIconService,
+        private ref: ChangeDetectorRef,
+    ) {
         this.family = this.iconService.defaultFamily;
         this.iconService.registerFamilyAlias('material', 'material-icons');
-        this.iconService.iconLoaded.pipe(
-            first(e => e.name === this.name && e.family === this.family),
-            takeUntil(this.destroy$)
-        )
-        .subscribe(() => this.ref.detectChanges());
+        this.iconService.iconLoaded
+            .pipe(
+                first((e) => e.name === this.name && e.family === this.family),
+                takeUntil(this.destroy$)
+            )
+            .subscribe(() => this.ref.detectChanges());
     }
 
     /**
@@ -226,21 +230,20 @@ export class IgxIconComponent implements OnInit, OnDestroy {
     }
 
     /**
-     *  An accessor that returns the key of the SVG image.
-     *  The key consists of the font-family and the name separated by underscore.
+     *  An accessor that returns the underlying SVG image as SafeHtml.
      *
      * @example
      * ```typescript
      * @ViewChild("MyIcon")
      * public icon: IgxIconComponent;
      * ngAfterViewInit() {
-     *    let svgKey = this.icon.getSvgKey;
+     *    let svg: SafeHtml = this.icon.getSvg;
      * }
      * ```
      */
-    public get getSvgKey(): string {
+    public get getSvg(): SafeHtml {
         if (this.iconService.isSvgIconCached(this.name, this.family)) {
-            return '#' + this.iconService.getSvgIconKey(this.name, this.family);
+            return this.iconService.getSvgIcon(this.name, this.family);
         }
 
         return null;

diff --git a/projects/igniteui-angular/src/lib/icon/icon.service.spec.ts b/projects/igniteui-angular/src/lib/icon/icon.service.spec.ts
@@ -1,6 +1,5 @@
-import { TestBed } from '@angular/core/testing';
+import { TestBed, fakeAsync } from '@angular/core/testing';
 import { IgxIconService } from './icon.service';
-import { DOCUMENT } from '@angular/common';
 
 import { configureTestSuite } from '../test-utils/configure-suite';
 import { first } from 'rxjs/operators';
@@ -53,13 +52,11 @@ describe('Icon Service', () => {
         expect(iconService.familyClassName(ALIAS)).toBe(MY_FONT);
     });
 
-    it('should add custom svg icon from url', () => {
+    it('should add custom svg icon from url', fakeAsync((done) => {
         const iconService = TestBed.inject(IgxIconService) as IgxIconService;
-        const document = TestBed.inject(DOCUMENT);
 
         const name = 'test';
         const family = 'svg-icons';
-        const iconKey = family + '_' + name;
 
         spyOn(XMLHttpRequest.prototype, 'open').and.callThrough();
         spyOn(XMLHttpRequest.prototype, 'send');
@@ -69,25 +66,20 @@ describe('Icon Service', () => {
         expect(XMLHttpRequest.prototype.open).toHaveBeenCalledTimes(1);
         expect(XMLHttpRequest.prototype.send).toHaveBeenCalledTimes(1);
 
-        const svgElement = document.querySelector(`svg[id='${iconKey}']`);
-        expect(svgElement).toBeDefined();
-    });
+        iconService.iconLoaded.pipe().subscribe(() => {
+            expect(iconService.isSvgIconCached(name, family)).toBeTruthy();
+            done();
+        });
+    }));
 
     it('should add custom svg icon from text', () => {
         const iconService = TestBed.inject(IgxIconService) as IgxIconService;
-        const document = TestBed.inject(DOCUMENT);
 
         const name = 'test';
         const family = 'svg-icons';
-        const iconKey = family + '_' + name;
 
         iconService.addSvgIconFromText(name, svgText, family);
-
         expect(iconService.isSvgIconCached(name, family)).toBeTruthy();
-        expect(iconService.getSvgIconKey(name, family)).toEqual(iconKey);
-
-        const svgElement = document.querySelector(`svg[id='${iconKey}']`);
-        expect(svgElement).toBeDefined();
     });
 
     it('should emit loading event for a custom svg icon from url', done => {
@@ -113,17 +105,4 @@ describe('Icon Service', () => {
 
         iconService.addSvgIcon(name, 'test.svg', family);
     });
-
-    it('should create svg container inside the body', () => {
-        const iconService = TestBed.inject(IgxIconService) as IgxIconService;
-        const document = TestBed.inject(DOCUMENT);
-
-        const name = 'test';
-        const family = 'svg-icons';
-
-        iconService.addSvgIconFromText(name, svgText, family);
-
-        const svgContainer = document.body.querySelector('.igx-svg-container');
-        expect(svgContainer).not.toBeNull();
-    });
 });
diff --git a/projects/igniteui-angular/src/lib/icon/icon.service.ts b/projects/igniteui-angular/src/lib/icon/icon.service.ts
@@ -1,5 +1,5 @@
-import { Injectable, SecurityContext, Inject, OnDestroy, Optional } from '@angular/core';
-import { DomSanitizer } from '@angular/platform-browser';
+import { Injectable, SecurityContext, Inject, Optional } from '@angular/core';
+import { DomSanitizer, SafeHtml } from '@angular/platform-browser';
 import { DOCUMENT } from '@angular/common';
 import { HttpClient } from '@angular/common/http';
 import { Observable, Subject } from 'rxjs';
@@ -32,7 +32,7 @@ export interface IgxIconLoadedEvent {
 @Injectable({
     providedIn: 'root'
 })
-export class IgxIconService implements OnDestroy {
+export class IgxIconService {
     /**
      * Observable that emits when an icon is successfully loaded
      * through a HTTP request.
@@ -46,9 +46,9 @@ export class IgxIconService implements OnDestroy {
 
     private _family = 'material-icons';
     private _familyAliases = new Map<string, string>();
-    private _svgContainer: HTMLElement;
-    private _cachedSvgIcons: Set<string> = new Set<string>();
+    private _cachedSvgIcons = new Map<string, Map<string, SafeHtml>>();
     private _iconLoaded = new Subject<IgxIconLoadedEvent>();
+    private _domParser = new DOMParser();
 
     constructor(
         @Optional() private _sanitizer: DomSanitizer,
@@ -58,14 +58,6 @@ export class IgxIconService implements OnDestroy {
         this.iconLoaded = this._iconLoaded.asObservable();
     }
 
-    /**
-     * @hidden
-     * @internal
-     */
-    public ngOnDestroy(): void {
-        this.cleanSvgContainer();
-    }
-
     /**
      *  Returns the default font-family.
      * ```typescript
@@ -162,18 +154,22 @@ export class IgxIconService implements OnDestroy {
      * ```
      */
     public isSvgIconCached(name: string, family: string = ''): boolean {
-        const iconKey = this.getSvgIconKey(name, family);
-        return this._cachedSvgIcons.has(iconKey);
+        if(this._cachedSvgIcons.has(family)) {
+            const familyRegistry = this._cachedSvgIcons.get(family) as Map<string, SafeHtml>;
+            return familyRegistry.has(name);
+        }
+
+        return false;
     }
 
     /**
-     *  Returns the key of a cached SVG image.
+     *  Returns the cached SVG image as string.
      * ```typescript
-     *   const svgIconKey = this.iconService.getSvgIconKey('aruba', 'svg-flags');
+     *   const svgIcon = this.iconService.getSvgIcon('aruba', 'svg-flags');
      * ```
      */
-    public getSvgIconKey(name: string, family: string = '') {
-        return family + '_' + name;
+    public getSvgIcon(name: string, family: string = '') {
+        return this._cachedSvgIcons.get(family)?.get(name);
     }
 
     /**
@@ -184,57 +180,23 @@ export class IgxIconService implements OnDestroy {
         return req;
     }
 
-    /**
-     * @hidden
-     */
-    private cleanSvgContainer() {
-        const container = this._document.documentElement.querySelector('.igx-svg-container');
-
-        while (container.firstChild) {
-            container.removeChild(container.firstChild);
-        }
-    }
-
     /**
      * @hidden
      */
     private cacheSvgIcon(name: string, value: string, family: string = '') {
         if (name && value) {
-            this.ensureSvgContainerCreated();
+            const doc = this._domParser.parseFromString(value, 'image/svg+xml');
+            const svg = doc.querySelector('svg') as SVGElement;
 
-            const div = this._document.createElement('DIV');
-            div.innerHTML = value;
-            const svg = div.querySelector('svg') as SVGElement;
+            if (!this._cachedSvgIcons.has(family)) {
+                this._cachedSvgIcons.set(family, new Map<string, SafeHtml>());
+            }
 
             if (svg) {
-                const iconKey = this.getSvgIconKey(name, family);
-
-                svg.setAttribute('id', iconKey);
                 svg.setAttribute('fit', '');
                 svg.setAttribute('preserveAspectRatio', 'xMidYMid meet');
-                svg.setAttribute('focusable', 'false'); // Disable IE11 default behavior to make SVGs focusable.
-
-                if (this.isSvgIconCached(name, family)) {
-                    const oldChild = this._svgContainer.querySelector(`svg[id='${iconKey}']`);
-                    this._svgContainer.removeChild(oldChild);
-                }
-
-                this._svgContainer.appendChild(svg);
-                this._cachedSvgIcons.add(iconKey);
-            }
-        }
-    }
-
-    /**
-     * @hidden
-     */
-    private ensureSvgContainerCreated() {
-        if (!this._svgContainer) {
-            this._svgContainer = this._document.documentElement.querySelector('.igx-svg-container');
-            if (!this._svgContainer) {
-                this._svgContainer = this._document.createElement('DIV');
-                this._svgContainer.classList.add('igx-svg-container');
-                this._document.body.appendChild(this._svgContainer);
+                const safeSvg = this._sanitizer.bypassSecurityTrustHtml(svg.outerHTML);
+                this._cachedSvgIcons.get(family).set(name, safeSvg);
             }
         }
     }

diff --git a/projects/igniteui-angular/src/lib/test-utils/grid-functions.spec.ts b/projects/igniteui-angular/src/lib/test-utils/grid-functions.spec.ts
@@ -596,8 +596,9 @@ export class GridFunctions {
         const ddItems = ddList.nativeElement.children;
         let i;
         for (i = 0; i < ddItems.length; i++) {
-            if (ddItems[i].textContent === cond) {
-                ddItems[i].click();
+            const ddItem = ddItems[i].querySelector('.igx-grid__filtering-dropdown-items span');
+            if (ddItem.textContent === cond) {
+                ddItem.click();
                 tick(100);
                 return;
             }

diff --git a/src/app/icon/icon.sample.html b/src/app/icon/icon.sample.html
@@ -86,6 +86,7 @@ <h4 class="sample-title">Using SVG Icons</h4>
             <igx-icon family="svg-flags" name="equals"></igx-icon>
             <igx-icon family="svg-flags" name="is_empty"></igx-icon>
             <igx-icon family="svg-flags" name="starts_with"></igx-icon>
+            <igx-icon family="svg-flags" name="copy"></igx-icon>
         </div>
     </article>
 

diff --git a/src/app/icon/icon.sample.ts b/src/app/icon/icon.sample.ts
@@ -19,5 +19,6 @@ export class IconSampleComponent implements OnInit {
         this._iconService.addSvgIcon('equals', '/assets/svg/filtering/equals.svg', 'svg-flags');
         this._iconService.addSvgIcon('is_empty', '/assets/svg/filtering/is_empty.svg', 'svg-flags');
         this._iconService.addSvgIcon('starts_with', '/assets/svg/filtering/starts_with.svg', 'svg-flags');
+        this._iconService.addSvgIcon('copy', '/assets/svg/filtering/copy.svg', 'svg-flags');
     }
 }
diff --git a/src/assets/svg/filtering/contains.svg b/src/assets/svg/filtering/contains.svg
diff --git a/src/assets/svg/filtering/copy.svg b/src/assets/svg/filtering/copy.svg