diff --git a/packages/components/icon/icon-registry.spec.ts b/packages/components/icon/icon-registry.spec.ts index c3fb3f725b..48c6a82244 100644 --- a/packages/components/icon/icon-registry.spec.ts +++ b/packages/components/icon/icon-registry.spec.ts @@ -5,6 +5,7 @@ import { DomSanitizer, SafeHtml } from '@angular/platform-browser'; import { firstValueFrom } from 'rxjs'; import type { MockInstance } from 'vitest'; import { KbqIconRegistry } from './icon-registry'; +import { kbqIconsDictProvider, kbqIconsProvider } from './icon-registry-providers'; const ICON_SVG = ''; const SPRITE_SVG = ` @@ -169,6 +170,31 @@ describe('KbqIconRegistry', () => { }); }); + describe('findSvgIcon', () => { + it('is null on an empty registry', () => { + expect(registry.findSvgIcon('check')).toBeNull(); + }); + + it('finds only the names registered', () => { + registry.addSvgIconLiteral('check', sanitizer.bypassSecurityTrustHtml(ICON_SVG)); + registry.addSvgIconLiteralInNamespace('brand', 'logo', sanitizer.bypassSecurityTrustHtml(ICON_SVG)); + + expect(registry.findSvgIcon('check')).not.toBeNull(); + expect(registry.findSvgIcon('brand:logo')).not.toBeNull(); + expect(registry.findSvgIcon('logo', 'brand')).not.toBeNull(); + expect(registry.findSvgIcon('close')).toBeNull(); + expect(registry.findSvgIcon('logo')).toBeNull(); + }); + + it('leaves any name of a namespace with an icon set to the set', () => { + registry.addSvgIconSetInNamespace('sprite', sanitizer.bypassSecurityTrustResourceUrl('/sprite.svg')); + + expect(registry.findSvgIcon('sprite:anything')).not.toBeNull(); + expect(registry.findSvgIcon('anything', 'sprite')).not.toBeNull(); + expect(registry.findSvgIcon('anything')).toBeNull(); + }); + }); + describe('getNamedSvgIcon errors', () => { it('errors when no icon registered', async () => { await expect(firstValueFrom(registry.getNamedSvgIcon('missing'))).rejects.toThrow('missing'); @@ -264,3 +290,20 @@ describe('KbqIconRegistry', () => { }); }); }); + +describe('KbqIconRegistry configured through providers', () => { + it('finds only the names a resolver answers for', () => { + TestBed.configureTestingModule({ providers: [kbqIconsDictProvider({ check: '/check.svg' })] }); + + const registry = TestBed.inject(KbqIconRegistry); + + expect(registry.findSvgIcon('check')).not.toBeNull(); + expect(registry.findSvgIcon('close')).toBeNull(); + }); + + it('leaves any bare name to a sprite configured without a namespace', () => { + TestBed.configureTestingModule({ providers: [kbqIconsProvider({ spriteUrl: '/sprite.svg' })] }); + + expect(TestBed.inject(KbqIconRegistry).findSvgIcon('anything')).not.toBeNull(); + }); +}); diff --git a/packages/components/icon/icon-registry.ts b/packages/components/icon/icon-registry.ts index d0c3b2ceef..39704de573 100644 --- a/packages/components/icon/icon-registry.ts +++ b/packages/components/icon/icon-registry.ts @@ -23,6 +23,21 @@ function iconKey(namespace: string, name: string): string { return namespace ? `${namespace}:${name}` : name; } +/** + * Splits "namespace:name" at the first colon; a name without one stays in the given namespace. + * + * @internal + */ +export function parseIconName(name: string, namespace = ''): [namespace: string, name: string] { + const colonIndex = name.indexOf(':'); + + return colonIndex === -1 ? [namespace, name] : [name.slice(0, colonIndex), name.slice(colonIndex + 1)]; +} + +function isSvgMarkup(text: string): boolean { + return text.trimStart().startsWith('<'); +} + function cloneSvg(svg: SVGElement): SVGElement { return svg.cloneNode(true) as SVGElement; } @@ -108,24 +123,29 @@ export class KbqIconRegistry { * Accepts "namespace:name" syntax. */ getNamedSvgIcon(name: string, namespace = ''): Observable { - let resolvedNs = namespace; - let resolvedName = name; + const icon = this.findSvgIcon(name, namespace); - if (resolvedName.trimStart().startsWith('<')) { - // `defer` so that unparsable markup arrives as an error notification: every failure of this - // class has to reach the caller through the same channel. - return defer(() => - of(this.svgElementFromText(this.sanitizer.sanitize(SecurityContext.HTML, resolvedName) ?? '')) - ); - } + if (icon) return icon; - const colonIndex = name.indexOf(':'); + const key = iconKey(...parseIconName(name, namespace)); - if (colonIndex !== -1) { - resolvedNs = name.slice(0, colonIndex); - resolvedName = name.slice(colonIndex + 1); + return throwError(() => Error(`KbqIconRegistry: no icon registered for "${key}".`)); + } + + /** + * `getNamedSvgIcon` without the error for a miss: `null` when nothing registered can resolve the name. Every font + * icon is looked up, and an error per miss would cost a stack trace each. + * + * @internal + */ + findSvgIcon(name: string, namespace = ''): Observable | null { + if (isSvgMarkup(name)) { + // `defer` so that unparsable markup arrives as an error notification: every failure of this + // class has to reach the caller through the same channel. + return defer(() => of(this.svgElementFromText(this.sanitizer.sanitize(SecurityContext.HTML, name) ?? ''))); } + const [resolvedNs, resolvedName] = parseIconName(name, namespace); const key = iconKey(resolvedNs, resolvedName); const iconConfig = this.iconConfigs.get(key); @@ -146,7 +166,7 @@ export class KbqIconRegistry { options: undefined }; - if (result.trimStart().startsWith('<')) { + if (isSvgMarkup(result)) { const sanitized = this.sanitizer.sanitize(SecurityContext.HTML, result) ?? ''; config.svgText = this.sanitizer.bypassSecurityTrustHtml(sanitized); @@ -165,7 +185,7 @@ export class KbqIconRegistry { return this.loadIconFromSets(resolvedName, sets); } - return throwError(() => Error(`KbqIconRegistry: no icon registered for "${key}".`)); + return null; } private cacheIcon(namespace: string, name: string, config: KbqSvgIconConfig): this { diff --git a/packages/components/icon/icon.component.spec.ts b/packages/components/icon/icon.component.spec.ts index de5e506332..983ef4090a 100644 --- a/packages/components/icon/icon.component.spec.ts +++ b/packages/components/icon/icon.component.spec.ts @@ -1,9 +1,10 @@ import { provideHttpClient } from '@angular/common/http'; import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; -import { ChangeDetectionStrategy, Component, Signal, signal, SimpleChange } from '@angular/core'; +import { ChangeDetectionStrategy, Component, Injectable, Signal, signal, SimpleChange } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { By, DomSanitizer } from '@angular/platform-browser'; import { KbqComponentColors } from '@koobiq/components/core'; +import { Observable, of } from 'rxjs'; import { KbqIconButton } from './icon-button.component'; import { KBQ_ICON_ERROR_STATE_CONTEXT } from './icon-error-state-context'; import { KbqIconRegistry } from './icon-registry'; @@ -14,6 +15,10 @@ const svgOf = (path: string) => ` + +`; + @Component({ imports: [KbqIcon], template: ` @@ -295,6 +300,48 @@ describe('KbqIcon', () => { expect(hostOf(fixture).classList).toContain('kbq-plus_16'); }); + // A miss through `getNamedSvgIcon` builds an error, with its stack trace, for every font icon on the page. + it('should not build an error for a font icon the registry cannot resolve', () => { + register('kbq-minus_16'); + + // On the prototype: an own property would read as a replaced lookup and be asked instead. + const lookup = vi.spyOn(KbqIconRegistry.prototype, 'getNamedSvgIcon'); + + onTestFinished(() => lookup.mockRestore()); + + const fixture = TestBed.createComponent(TestApp); + + fixture.detectChanges(); + + expect(lookup).not.toHaveBeenCalled(); + expect(hostOf(fixture).classList).toContain('kbq-plus_16'); + }); + + it('should still look up a name of a namespace with an icon set', async () => { + registry.addSvgIconSetInNamespace('sprite', sanitizer.bypassSecurityTrustResourceUrl('/sprite.svg')); + + const fixture = TestBed.createComponent(TestApp); + + fixture.componentInstance.name.set('sprite:check_16'); + fixture.detectChanges(); + + TestBed.inject(HttpTestingController).expectOne('/sprite.svg').flush(SPRITE_SVG); + await fixture.whenStable(); + + expect(hostOf(fixture).querySelector('svg')?.getAttribute('width')).toBe('16'); + }); + + it('should size an svg whose name has a colon after the namespace', () => { + registry.addSvgIconLiteralInNamespace('brand', 'logo:dark_24', sanitizer.bypassSecurityTrustHtml(ICON_SVG)); + + const fixture = TestBed.createComponent(TestApp); + + fixture.componentInstance.name.set('brand:logo:dark_24'); + fixture.detectChanges(); + + expect(hostOf(fixture).querySelector('svg')?.getAttribute('width')).toBe('24'); + }); + it('should render an SVG that arrives after the first render', async () => { registry.addSvgIcon('kbq-plus_16', sanitizer.bypassSecurityTrustResourceUrl('/icons/plus.svg')); @@ -499,6 +546,47 @@ describe('KbqIcon', () => { expect(hostOf(fixture).querySelector('svg')).not.toBeNull(); }); + // The registry is public: an application may replace its lookup, and its specs may provide a double of it. + describe('replaced registry lookup', () => { + const svgElement = (): SVGElement => { + const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); + + svg.setAttribute('viewBox', '0 0 16 16'); + + return svg; + }; + + it('should ask a registry subclass that overrides getNamedSvgIcon', () => { + @Injectable() + class AppIconRegistry extends KbqIconRegistry { + override getNamedSvgIcon(name: string): Observable { + return name === 'kbq-plus_16' ? of(svgElement()) : super.getNamedSvgIcon(name); + } + } + + TestBed.configureTestingModule({ providers: [{ provide: KbqIconRegistry, useClass: AppIconRegistry }] }); + + const fixture = TestBed.createComponent(TestApp); + + fixture.detectChanges(); + + expect(hostOf(fixture).querySelector('svg')).not.toBeNull(); + expect(hostOf(fixture).classList).not.toContain('kbq-plus_16'); + }); + + it('should ask a test double that only has getNamedSvgIcon', () => { + TestBed.configureTestingModule({ + providers: [{ provide: KbqIconRegistry, useValue: { getNamedSvgIcon: () => of(svgElement()) } }] + }); + + const fixture = TestBed.createComponent(TestApp); + + fixture.detectChanges(); + + expect(hostOf(fixture).querySelector('svg')).not.toBeNull(); + }); + }); + // The color class of an error icon is `kbq-error` too; the `kbq-error` class binding must not take it away. describe('error color', () => { const iconOf = (fixture: ComponentFixture): KbqIcon => diff --git a/packages/components/icon/icon.component.ts b/packages/components/icon/icon.component.ts index 52ec8ce3e9..924103beb8 100644 --- a/packages/components/icon/icon.component.ts +++ b/packages/components/icon/icon.component.ts @@ -15,9 +15,9 @@ import { ViewEncapsulation } from '@angular/core'; import { KbqColorDirective, KbqComponentColors } from '@koobiq/components/core'; -import { defer, Subscription } from 'rxjs'; +import { defer, of, Subscription } from 'rxjs'; import { KBQ_ICON_ERROR_STATE_CONTEXT } from './icon-error-state-context'; -import { KbqIconRegistry } from './icon-registry'; +import { KbqIconRegistry, parseIconName } from './icon-registry'; /** Marks an injected `` with the name it renders, so hydration can recognize and adopt it. */ const ICON_NAME_ATTRIBUTE = 'data-kbq-icon-name'; @@ -248,10 +248,15 @@ export class KbqIcon extends KbqColorDirective implements AfterContentInit, OnCh } const registry = this.registry; + // A registry that replaces the lookup, such as a subclass overriding `getNamedSvgIcon` or a test double, is + // asked through it: only the library's own lookup can report a miss without building an error. + const ownLookup = registry.getNamedSvgIcon === KbqIconRegistry.prototype.getNamedSvgIcon; // Deferred so that a resolver or a literal that throws falls back to the font class as well, and the // next name is resolved again. - this.svgIconSubscription = defer(() => registry.getNamedSvgIcon(name)).subscribe({ + this.svgIconSubscription = defer( + () => (ownLookup ? registry.findSvgIcon(name) : registry.getNamedSvgIcon(name)) ?? of(null) + ).subscribe({ next: (svg) => this.renderSvgIcon(svg), error: () => this.renderSvgIcon(null) }); @@ -309,8 +314,7 @@ export class KbqIcon extends KbqColorDirective implements AfterContentInit, OnCh } private parseIconSize(): number | undefined { - const iconName = this.iconName; - const baseName = iconName?.includes(':') ? iconName.split(':')[1] : iconName; + const baseName = this.iconName && parseIconName(this.iconName)[1]; const size = parseInt(baseName?.split('_').pop() ?? ''); return Number.isNaN(size) ? undefined : size;