Merge pull request #81 from shiftcode/#80-expose-svg-base-class

feat(svg): expose `SvgBaseDirective` to allow custom icon component implementations

Author: mumenthalers

apps/styleguide/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@shiftcode/styleguide",
-  "version": "15.0.0",
+  "version": "15.1.0-pr80.1",
   "private": true,
   "type": "module",
   "scripts": {

lerna.json

@@ -2,7 +2,7 @@
   "$schema": "node_modules/lerna/schemas/lerna-schema.json",
   "useNx": false,
   "packages": ["libs/*", "apps/*"],
-  "version": "15.0.0",
+  "version": "15.1.0-pr80.1",
   "command": {
     "version": {
       "allowBranch": "*",

libs/components/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@shiftcode/ngx-components",
-  "version": "15.0.0",
+  "version": "15.1.0-pr80.1",
   "repository": "https://github.com/shiftcode/sc-ng-commons-public",
   "license": "MIT",
   "author": "shiftcode GmbH <team@shiftcode.ch>",

libs/components/src/lib/svg/svg-base.directive.spec.ts

@@ -0,0 +1,202 @@
+import { HttpErrorResponse } from '@angular/common/http'
+import { Component, inject, signal } from '@angular/core'
+import { TestBed } from '@angular/core/testing'
+import { Logger } from '@shiftcode/logger'
+import { describe, expect, test, vi } from 'vitest'
+
+import { SvgBaseDirective } from './svg-base.directive'
+import { SvgRegistry } from './svg-registry.service'
+
+const MOCK_SVG_URL = '/assets/test.svg'
+
+@Component({ selector: 'sc-test-svg-base', template: '', standalone: true })
+class TestSvgComponent extends SvgBaseDirective {
+  readonly data = signal<{ url: string; attrs?: Record<string, string> }>({ url: MOCK_SVG_URL })
+  protected readonly logger = inject(Logger)
+}
+
+function createSvgElement(): SVGElement {
+  const div = document.createElement('div')
+  div.innerHTML = `<svg xmlns="http://www.w3.org/2000/svg" data-ts="${Date.now()}"><path d="M0 0"/></svg>`
+  return div.querySelector('svg') as SVGElement
+}
+
+function setup() {
+  const mockLogger = {
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+  } as unknown as Logger
+
+  const mockRegistry = { getFromUrl: vi.fn<() => Promise<SVGElement>>() }
+
+  TestBed.configureTestingModule({
+    providers: [
+      { provide: Logger, useValue: mockLogger },
+      { provide: SvgRegistry, useValue: mockRegistry },
+    ],
+  })
+
+  return { mockLogger, mockRegistry }
+}
+
+async function flushMicrotasks(count = 2): Promise<void> {
+  for (let i = 0; i < count; i++) {
+    await Promise.resolve()
+  }
+}
+
+async function renderComponent(url: string = MOCK_SVG_URL, attrs?: Record<string, string>) {
+  const fixture = TestBed.createComponent(TestSvgComponent)
+
+  fixture.componentInstance.data.set({ url, attrs })
+  TestBed.tick()
+  await flushMicrotasks()
+
+  return {
+    fixture,
+    hostElement: fixture.nativeElement as HTMLElement,
+  }
+}
+
+describe('SvgBaseDirective', () => {
+  test('fetches SVG from the registry and inserts it into the DOM', async () => {
+    const { mockRegistry } = setup()
+    const svg = createSvgElement()
+    mockRegistry.getFromUrl.mockResolvedValue(svg)
+
+    const { hostElement } = await renderComponent()
+
+    expect(mockRegistry.getFromUrl).toHaveBeenCalledWith(MOCK_SVG_URL)
+    expect(hostElement.contains(svg)).toBe(true)
+  })
+
+  test('applies provided attrs to the SVG element', async () => {
+    const { mockRegistry } = setup()
+    const svg = createSvgElement()
+    mockRegistry.getFromUrl.mockResolvedValue(svg)
+
+    const { hostElement } = await renderComponent(MOCK_SVG_URL, { 'data-test': 'true', class: 'my-icon' })
+
+    expect(svg.getAttribute('data-test')).toBe('true')
+    expect(svg.getAttribute('class')).toBe('my-icon')
+    expect(hostElement.contains(svg)).toBe(true)
+  })
+
+  test('does not modify SVG attributes when no attrs are provided', async () => {
+    const { mockRegistry } = setup()
+    const svg = createSvgElement()
+    const originalAttributes = Array.from(svg.attributes).map((a) => ({ name: a.name, value: a.value }))
+    mockRegistry.getFromUrl.mockResolvedValue(svg)
+
+    const { hostElement } = await renderComponent()
+
+    expect(Array.from(svg.attributes).map((a) => ({ name: a.name, value: a.value }))).toEqual(originalAttributes)
+    expect(hostElement.contains(svg)).toBe(true)
+  })
+
+  test('allows to update the SVG when the url signal changes', async () => {
+    const { mockRegistry } = setup()
+    const svgFirst = createSvgElement()
+    const svgSecond = createSvgElement()
+
+    mockRegistry.getFromUrl.mockResolvedValueOnce(svgFirst).mockResolvedValueOnce(svgSecond)
+
+    const { fixture, hostElement } = await renderComponent()
+
+    expect(hostElement.contains(svgFirst)).toBe(true)
+
+    fixture.componentInstance.data.set({ url: '/assets/other.svg' })
+    TestBed.tick()
+    await flushMicrotasks()
+
+    expect(hostElement.contains(svgSecond)).toBe(true)
+  })
+
+  test('clears existing DOM content before inserting the new SVG on signal change', async () => {
+    const { mockRegistry } = setup()
+    const svgFirst = createSvgElement()
+    const svgSecond = createSvgElement()
+    mockRegistry.getFromUrl.mockResolvedValueOnce(svgFirst).mockResolvedValueOnce(svgSecond)
+
+    const { fixture, hostElement } = await renderComponent()
+
+    expect(hostElement.contains(svgFirst)).toBe(true)
+
+    fixture.componentInstance.data.set({ url: '/assets/other.svg' })
+    TestBed.tick()
+    await flushMicrotasks()
+
+    expect(hostElement.contains(svgFirst)).toBe(false)
+    expect(hostElement.contains(svgSecond)).toBe(true)
+    expect(hostElement.querySelectorAll('svg').length).toBe(1)
+  })
+
+  test('does not insert SVG when the signal changes before the first request resolved', async () => {
+    const { mockRegistry } = setup()
+
+    let resolveFirst!: (svg: SVGElement) => void
+    const firstPromise = new Promise<SVGElement>((resolve) => (resolveFirst = resolve))
+
+    const firstSvg = createSvgElement()
+    const secondSvg = createSvgElement()
+
+    mockRegistry.getFromUrl
+      .mockReturnValueOnce(firstPromise) // first call returns a promise that we can control
+      .mockResolvedValueOnce(secondSvg) // second call resolves immediately
+
+    const { fixture, hostElement } = await renderComponent()
+
+    expect(mockRegistry.getFromUrl).toHaveBeenCalledTimes(1)
+
+    // Change signal before firstPromise resolves
+    fixture.componentInstance.data.set({ url: '/assets/other.svg' })
+    TestBed.tick()
+    await flushMicrotasks()
+
+    // At this point, the second svg should be rendered.
+    expect(mockRegistry.getFromUrl).toHaveBeenCalledTimes(2)
+    expect(hostElement.contains(secondSvg)).toBe(true)
+
+    // Now resolve the first promise; it should have no effect in the DOM anymore.
+    resolveFirst(firstSvg)
+    TestBed.tick()
+    await flushMicrotasks()
+    expect(hostElement.contains(secondSvg)).toBe(true)
+  })
+
+  test('logs a debug info for network errors (HttpErrorResponse with status 0)', async () => {
+    const { mockLogger, mockRegistry } = setup()
+
+    const networkError = new HttpErrorResponse({ status: 0, url: MOCK_SVG_URL })
+    mockRegistry.getFromUrl.mockRejectedValue(networkError)
+
+    await renderComponent()
+
+    expect(mockLogger.debug).toHaveBeenCalledWith(expect.stringContaining(MOCK_SVG_URL), networkError)
+    expect(mockLogger.error).not.toHaveBeenCalled()
+  })
+
+  test('logs an error for HttpErrorResponse with a non-zero status', async () => {
+    const { mockLogger, mockRegistry } = setup()
+
+    const notFoundErrorResponse = new HttpErrorResponse({ status: 404, url: MOCK_SVG_URL })
+    mockRegistry.getFromUrl.mockRejectedValue(notFoundErrorResponse)
+
+    await renderComponent()
+
+    expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining(MOCK_SVG_URL), notFoundErrorResponse)
+  })
+
+  test('logs an error for non-HTTP errors', async () => {
+    const { mockLogger, mockRegistry } = setup()
+
+    const genericError = new Error('Something went wrong')
+    mockRegistry.getFromUrl.mockRejectedValue(genericError)
+
+    await renderComponent()
+
+    expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining(MOCK_SVG_URL), genericError)
+  })
+})

libs/components/src/lib/svg/svg-base.directive.ts

@@ -0,0 +1,71 @@
+import { HttpErrorResponse } from '@angular/common/http'
+import { Directive, effect, ElementRef, inject, Renderer2, Signal } from '@angular/core'
+import { Logger } from '@shiftcode/logger'
+
+import { SvgRegistry } from './svg-registry.service'
+
+/**
+ * Base class for components that display an SVG element inline.
+ * The SVG content is directly inlined as a child of the component, so that CSS styles can easily be applied to it.
+ */
+@Directive()
+export abstract class SvgBaseDirective {
+  protected readonly elRef = inject<ElementRef<HTMLElement>>(ElementRef)
+  protected readonly renderer = inject(Renderer2)
+  protected readonly svgRegistry = inject(SvgRegistry)
+
+  protected abstract readonly logger: Logger
+  protected abstract readonly data: Signal<{ url: string; attrs?: Record<string, string> }>
+
+  constructor() {
+    effect((onCleanup) => {
+      const { url, attrs } = this.data()
+      const abortController = new AbortController()
+      this.getAndSet(url, attrs, abortController.signal)
+      onCleanup(() => abortController.abort())
+    })
+  }
+
+  private getAndSet(url: string, attrs: Record<string, string> | undefined, abortSignal: AbortSignal) {
+    // due to the caching in SvgRegistry we cannot simply abort the fetching of the svg.
+    // but we ensure that we do not set the svg element if the abort signal has been triggered in the meantime.
+    this.svgRegistry
+      .getFromUrl(url)
+      .then(this.getSvgModifierFn(attrs))
+      .then(this.getSvgSetterFn(abortSignal))
+      .catch((err: any) => {
+        if (err instanceof HttpErrorResponse && err.status === 0) {
+          // in case of no internet or a timeout log a warning, we can not do anything about that
+          this.logger.debug(`Error retrieving icon for path ${url}, due to no network`, err)
+        } else {
+          this.logger.error(`Error retrieving icon for path ${url}`, err)
+        }
+      })
+  }
+
+  private getSvgModifierFn(attrs?: Record<string, string>) {
+    const attrsEntries = attrs ? Object.entries(attrs) : []
+    if (attrsEntries.length === 0) {
+      return (svg: SVGElement): SVGElement => svg
+    }
+
+    return (svg: SVGElement): SVGElement => {
+      for (const [key, val] of attrsEntries) {
+        svg.setAttribute(key, val)
+      }
+      return svg
+    }
+  }
+
+  private getSvgSetterFn = (abortSignal: AbortSignal) => {
+    return (svg: SVGElement) => {
+      if (abortSignal.aborted) {
+        this.logger.debug('Aborting setSvgElement due to abort signal')
+        return
+      }
+      // Remove existing child nodes and add the new SVG element.
+      this.elRef.nativeElement.innerHTML = ''
+      this.renderer.appendChild(this.elRef.nativeElement, svg)
+    }
+  }
+}

libs/components/src/lib/svg/svg.component.ts

@@ -1,21 +1,19 @@
-import { HttpErrorResponse } from '@angular/common/http'
-import { ChangeDetectionStrategy, Component, effect, ElementRef, inject, input, Renderer2 } from '@angular/core'
+import { ChangeDetectionStrategy, Component, computed, inject, input } from '@angular/core'
 import { Logger } from '@shiftcode/logger'
 import { LoggerService } from '@shiftcode/ngx-core'
 
-import { SvgRegistry } from './svg-registry.service'
+import { SvgBaseDirective } from './svg-base.directive'
 
 /**
  * Standalone SvgComponent to display svg inline.
- * (Initially copied from material MdIcon Directive but got rid of unused functionality and refactored to Promises)
  *
  *  - Specify the url input to load an SVG icon from a URL.
  *   The SVG content is directly inlined as a child of the <sc-svg> component,
  *   so that CSS styles can easily be applied to it.
- *   The URL is loaded via an XMLHttpRequest, so it must be on the same domain as the page or its
+ *   The URL is loaded via Angular's {@link HttpClient}, it must be on the same domain as the page or its
  *   server must be configured to allow cross-domain requests.
  * @example
- *   <sc-svg url="assets/arrow.svg"></sc-svg>
+ *   <sc-svg url="assets/arrow.svg" />
  */
 @Component({
   selector: 'sc-svg',
@@ -24,52 +22,14 @@ import { SvgRegistry } from './svg-registry.service'
   styleUrls: ['./svg.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class SvgComponent {
+export class SvgComponent extends SvgBaseDirective {
   readonly url = input.required<string>()
 
   readonly attrs = input<Record<string, string>>({})
 
-  protected readonly elRef: ElementRef<HTMLElement> = inject(ElementRef)
-  protected readonly renderer = inject(Renderer2)
-  protected readonly svgRegistry = inject(SvgRegistry)
-
-  private readonly logger: Logger = inject(LoggerService).getInstance('SvgComponent')
-
-  constructor() {
-    effect(() => {
-      this.loadAndSetSvg(this.url(), this.attrs())
-    })
-  }
-
-  private loadAndSetSvg(url: string, attrs: Record<string, string>) {
-    if (!url.endsWith('.svg')) {
-      this.logger.warn('svg url does not end with *.svg')
-    }
-    this.svgRegistry
-      .getFromUrl(url)
-      .then(this.modifySvgElement(attrs))
-      .then(this.setSvgElement)
-      .catch((err: any) => {
-        if (err instanceof HttpErrorResponse && err.status === 0) {
-          // in case of no internet or a timeout log a warning, we can not do anything about that
-          this.logger.warn(`Error retrieving icon for path ${this.url()}, due to no network`, err)
-        } else {
-          this.logger.error(`Error retrieving icon for path ${this.url()}`, err)
-        }
-      })
-  }
-
-  private modifySvgElement(attrs: Record<string, string>) {
-    return (svg: SVGElement): SVGElement => {
-      Object.keys(attrs).forEach((key) => svg.setAttribute(key, attrs[key]))
-      return svg
-    }
-  }
-
-  private setSvgElement = (svg: SVGElement | null) => {
-    const layoutElement = this.elRef.nativeElement
-    // Remove existing child nodes and add the new SVG element.
-    layoutElement.innerHTML = ''
-    this.renderer.appendChild(layoutElement, svg)
-  }
+  protected readonly logger: Logger = inject(LoggerService).getInstance('SvgComponent')
+  protected readonly data = computed(() => ({
+    url: this.url(),
+    attrs: this.attrs(),
+  }))
 }

libs/components/src/public-api.ts

@@ -1,5 +1,6 @@
 // svg
 export * from './lib/svg/svg.component'
+export * from './lib/svg/svg-base.directive'
 export * from './lib/svg/svg-registry.service'
 
 // svg-animate

libs/core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@shiftcode/ngx-core",
-  "version": "15.0.0",
+  "version": "15.1.0-pr80.1",
   "repository": "https://github.com/shiftcode/sc-ng-commons-public",
   "license": "MIT",
   "author": "shiftcode GmbH <team@shiftcode.ch>",