Skip to content

refactor(icon): remove disableSvgIcons from ´provideIconConfig()` o… #2239

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.

Sign up for GitHub

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

Open
spliffone wants to merge 2 commits into
base: main
Choose a base branch
from
Open

refactor(icon): remove disableSvgIcons from ´provideIconConfig()` o… #2239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
021e471
refactor(icon): remove `disableSvgIcons` from ´provideIconConfig()` o…
spliffone Jun 26, 2026
7a3d545
Merge branch 'main' into refactor/remove-disable-svg-from-icons
spliffone Jun 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion api-goldens/element-ng/icon/index.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export const elementCheckedImageShape = "data:image/svg+xml;base64,PHN2ZyBpZD0iS

// @public
export interface IconConfig {
disableSvgIcons?: boolean;
}

// @public
Expand Down
23 changes: 1 addition & 22 deletions projects/element-ng/icon/si-icon.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { ComponentFixture, TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { SiThemeService } from '@siemens/element-ng/theme';

import { provideIconConfig, SiIconComponent } from './si-icon.component';
import { SiIconComponent } from './si-icon.component';
import { addIcons, IconService } from './si-icons';

// It seems like the debug element is not able to query the SVGs.
Expand Down Expand Up @@ -41,9 +41,6 @@ describe('SiSvgIconComponent', () => {

describe('with enabled svg icons', () => {
beforeEach(async () => {
TestBed.configureTestingModule({
providers: [provideIconConfig({ disableSvgIcons: false })]
});
fixture = TestBed.createComponent(TestHostComponent);
component = fixture.componentInstance;
fixture.detectChanges();
Comment thread
spliffone marked this conversation as resolved.
Expand Down Expand Up @@ -109,24 +106,6 @@ describe('SiSvgIconComponent', () => {
});
});
});

describe('with disabled svg icons', () => {
beforeEach(() => {
TestBed.configureTestingModule({
providers: [provideIconConfig({ disableSvgIcons: true })]
});
fixture = TestBed.createComponent(TestHostComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});

it('should always use the icon-font', () => {
component.icon.set('element-svg');
fixture.detectChanges();
expect(getIconHostStyle()).toBe('');
expect(fixture.debugElement.query(By.css('.element-svg'))).toBeTruthy();
});
});
});

describe('with multiple instances', () => {
Expand Down
15 changes: 4 additions & 11 deletions projects/element-ng/icon/si-icon.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,12 @@ import { IconService } from './si-icons';
/**
* Global configuration for icons.
*/
export interface IconConfig {
/**
* If true, the si-icon component will always render the icon font instead of the svg.
*
* @defaultValue false
*/
disableSvgIcons?: boolean;
}
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
export interface IconConfig {}

const ICON_CONFIG = new InjectionToken<IconConfig>('ICON_CONFIG', {
providedIn: 'root',
factory: () => ({ disableSvgIcons: false })
factory: () => ({})
});
Comment on lines +20 to 26

@gemini-code-assist gemini-code-assist Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since disableSvgIcons was the only configuration option and has been removed, the entire icon configuration infrastructure (IconConfig, ICON_CONFIG, and provideIconConfig) is now unused dead code. Keeping an empty interface and bypassing the linter with // eslint-disable-next-line @typescript-eslint/no-empty-object-type is a code smell.

Unless there are plans to introduce other configuration options in the near future, please consider removing the configuration infrastructure entirely:

  1. Remove IconConfig, ICON_CONFIG, and provideIconConfig.
  2. Remove the unused InjectionToken and Provider imports from @angular/core.


/**
Expand Down Expand Up @@ -82,11 +76,10 @@ export class SiIconComponent {
*/
readonly icon = input.required<string>();

private readonly config = inject(ICON_CONFIG);
private readonly iconService = inject(IconService);

protected readonly svgIcon = computed(() => {
const icon = this.config.disableSvgIcons ? undefined : this.iconService.getIcon(this.icon());
const icon = this.iconService.getIcon(this.icon());
if (!icon) {
return undefined;
}
Expand Down
Loading