Components: bug(icon): custom directive cannot set svgIcon for mat-icon

Created on 1 Sep 2020  路  3Comments  路  Source: angular/components

Reproduction

_svgIcon does not seem to work with Stackblitz, providing repo instead._

Steps to reproduce:

  1. Clone https://github.com/klemenoslaj/mat-icon-svg-not-applied
  2. Install dependencies using yarn
  3. Execute ng serve

Expected Behavior

I tried to create a custom directive that would programmatically assign some icon name to the svgIcon input of mat-icon, depending on some global icon mapping configuration.
This normally works for any component and I'd expect it should for mat-icon too.

Example:

<mat-icon [iconFor]="object"></mat-icon>
@Directive({
  selector: 'mat-icon[iconFor]',
})
export class IconForDirective {
  @Input()
  set iconFor(obj: SomeInterface) {
    this._matIcon.svgIcon = _iconMap[obj.type];
  }

  constructor(
    @Inject(ICON_MAPPING_TOKEN)
    private readonly _iconMap: SomeIconMapping,
    private readonly _matIcon: MatIcon,
  ) {}
}

Actual Behavior

The behavior described above does not work with mat-icon, because the relevant logic is in ngOnChanges hook of mat-icon, which does not get triggered, unless at least one Input is present on the host component.

https://github.com/angular/components/blob/4f1238f84791f4193b5dfc8d9cbef76f04ee36d6/src/material/icon/icon.ts#L229-L264

Workaround (ugly):

Instead of

<mat-icon [iconFor]="object"></mat-icon>

add dummy/empty svgIcon

<mat-icon [iconFor]="object" svgIcon></mat-icon>

Environment

  • Angular: 10.0.14
  • CDK/Material: 10.1.3
  • Browser(s): any
  • Operating System (e.g. Windows, macOS, Ubuntu): any

NOTE: I can submit the PR for this, but would like some thoughts on that, maybe it's expected like this for some reason.

P4 materiaicon has pr

Most helpful comment

The only way would be to define a setter for svgIcon. I know that in the past we've decided not to handle cases like this, but in this one I think it's fine since we're already "paying" for the setters on fontIcon and fontSet. I've submitted a PR to support it and we can discuss any concerns there (#20509).

All 3 comments

Looking at the code, it seems like we should extract the code in ngOnChanges that actually performs the DOM update into its own function, and then call that from both ngOnChanges and ngOnInit if the value has been changed. @crisbeto thoughts on this since you've touched the icon code most recently?

The only way would be to define a setter for svgIcon. I know that in the past we've decided not to handle cases like this, but in this one I think it's fine since we're already "paying" for the setters on fontIcon and fontSet. I've submitted a PR to support it and we can discuss any concerns there (#20509).

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_This action has been performed automatically by a bot._

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Hiblton picture Hiblton  路  3Comments

shlomiassaf picture shlomiassaf  路  3Comments

alanpurple picture alanpurple  路  3Comments

michaelb-01 picture michaelb-01  路  3Comments

LoganDupont picture LoganDupont  路  3Comments