-
Notifications
You must be signed in to change notification settings - Fork 14
refactor(icon): bind icon as CSS mask #1519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,8 +3,8 @@ | |||||
| * SPDX-License-Identifier: MIT | ||||||
| */ | ||||||
| import { isPlatformBrowser } from '@angular/common'; | ||||||
| import { EventEmitter, inject, Injectable, PLATFORM_ID, signal, DOCUMENT } from '@angular/core'; | ||||||
| import { DomSanitizer, Meta, SafeHtml } from '@angular/platform-browser'; | ||||||
| import { DOCUMENT, EventEmitter, inject, Injectable, PLATFORM_ID, signal } from '@angular/core'; | ||||||
| import { Meta } from '@angular/platform-browser'; | ||||||
| import { Observable, of, ReplaySubject, throwError } from 'rxjs'; | ||||||
| import { map, switchMap, take, tap } from 'rxjs/operators'; | ||||||
|
|
||||||
|
|
@@ -75,7 +75,7 @@ export class SiThemeService { | |||||
| * {} | ||||||
| * ``` | ||||||
| */ | ||||||
| readonly themeIcons = signal<Record<string, SafeHtml>>({}); | ||||||
| readonly themeIcons = signal<Record<string, string>>({}); | ||||||
|
|
||||||
| private themes: Map<string, Theme> = new Map(); | ||||||
| private darkMediaQuery?: MediaQueryList; | ||||||
|
|
@@ -87,7 +87,6 @@ export class SiThemeService { | |||||
| inject(SiThemeStore, { optional: true }) ?? new SiDefaultThemeStore(this.isBrowser); | ||||||
| private meta = inject(Meta); | ||||||
| private document = inject(DOCUMENT); | ||||||
| private domSanitizer = inject(DomSanitizer); | ||||||
|
|
||||||
| constructor() { | ||||||
| this.resolvedColorScheme$.subscribe(scheme => (this._resolvedColorScheme = scheme)); | ||||||
|
|
@@ -468,17 +467,8 @@ export class SiThemeService { | |||||
| ); | ||||||
| } | ||||||
|
|
||||||
| private parseDataSvgIcon(icon: string): SafeHtml { | ||||||
| private parseDataSvgIcon(icon: string): string { | ||||||
| // This method is currently a copy of parseDataSvgIcon in si-icon.registry.ts. | ||||||
| // Those are likely to diverge in the future, as this version will get support for other formats like: | ||||||
| // - URLs | ||||||
| // - Plain SVG data | ||||||
| // - Promises to enable lazy icon loading using import() | ||||||
| const parsed = /^data:image\/svg\+xml;utf8,(.*)$/.exec(icon); | ||||||
| if (!parsed) { | ||||||
| console.error('Failed to parse icon', icon); | ||||||
| return ''; | ||||||
| } | ||||||
| return this.domSanitizer.bypassSecurityTrustHtml(parsed[1]); | ||||||
| return `url("${icon}")`; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation of
Suggested change
|
||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation of
parseDataSvgIcondoes not handle double quotes within the icon string, which can lead to broken image URLs. If an icon's SVG data contains a double quote, it will prematurely terminate the string within theurl()function, causing the icon to fail to render. To fix this, you should escape any double quotes within the icon string.