Skip to content

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

Open
spliffone wants to merge 2 commits into
mainfrom
refactor/remove-disable-svg-from-icons
Open

refactor(icon): remove disableSvgIcons from ´provideIconConfig()` o…#2239
spliffone wants to merge 2 commits into
mainfrom
refactor/remove-disable-svg-from-icons

Conversation

@spliffone

@spliffone spliffone commented Jun 26, 2026

Copy link
Copy Markdown
Member

…ptions

fixes #1510

BREAKING CHANGE: removed disableSvgIcons from ´provideIconConfig()` configuration

Consumer which want to provide their own custom icons must call addIcons during application bootstrap.


Documentation.
Examples.
Dashboards Demo.
Playwright report.

Coverage Reports:

Code Coverage

@spliffone spliffone requested review from a team as code owners June 26, 2026 14:14
@spliffone spliffone force-pushed the refactor/remove-disable-svg-from-icons branch from add2e3e to cc49bf8 Compare June 26, 2026 14:15
@spliffone spliffone added the breaking-changes Marks issues and PRs that are breaking the API label Jun 26, 2026
@spliffone spliffone added this to the 51.x milestone Jun 26, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request removes the disableSvgIcons configuration option from the SiIconComponent and its corresponding tests, simplifying the icon rendering logic. Feedback on these changes suggests removing the now-obsolete icon configuration infrastructure (IconConfig, ICON_CONFIG, and provideIconConfig) entirely to prevent dead code, as well as cleaning up the empty TestBed.configureTestingModule({}) call in the unit tests since it is redundant for standalone components.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +20 to 26
// 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: () => ({})
});

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.

Comment thread projects/element-ng/icon/si-icon.component.spec.ts
@spliffone spliffone modified the milestones: 51.x, 51.0.0 Jun 26, 2026
…ptions

fixes #1510

BREAKING CHANGE: removed `disableSvgIcons` from ´provideIconConfig()` configuration

Consumer which want to provide their own custom icons must call `addIcons` during application bootstrap.
@spliffone spliffone force-pushed the refactor/remove-disable-svg-from-icons branch from cc49bf8 to 021e471 Compare June 26, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-changes Marks issues and PRs that are breaking the API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove icon option disableSvgIcons

1 participant