Skip to content

Commit 4778f49

Browse files
authored
fix(icon): use ErrorHandler to log MatIcon errors (#16999)
This reverts commit 680ed00, which reverted #16967 (roll forward)
1 parent 09ded04 commit 4778f49

File tree

3 files changed

+53
-18
lines changed

3 files changed

+53
-18
lines changed

src/material/icon/icon.spec.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,10 +278,35 @@ describe('MatIcon', () => {
278278
http.expectOne('farm-set-1.svg').error(new ErrorEvent('Network error'));
279279
fixture.detectChanges();
280280

281-
expect(errorHandler.handleError).toHaveBeenCalledTimes(1);
281+
// Called twice once for the HTTP request failing and once for the icon
282+
// then not being able to be found.
283+
expect(errorHandler.handleError).toHaveBeenCalledTimes(2);
282284
expect(errorHandler.handleError.calls.argsFor(0)[0].message).toEqual(
283285
'Loading icon set URL: farm-set-1.svg failed: Http failure response ' +
284286
'for farm-set-1.svg: 0 ');
287+
expect(errorHandler.handleError.calls.argsFor(1)[0].message)
288+
.toEqual(
289+
`Error retrieving icon ${testComponent.iconName}! ` +
290+
'Unable to find icon with the name "pig"');
291+
});
292+
293+
it('should delegate an error getting an SVG icon to the ErrorHandler', () => {
294+
iconRegistry.addSvgIconSetInNamespace('farm', trustUrl('farm-set-1.svg'));
295+
296+
const fixture = TestBed.createComponent(IconFromSvgName);
297+
const testComponent = fixture.componentInstance;
298+
299+
testComponent.iconName = 'farm:DNE';
300+
fixture.detectChanges();
301+
http.expectOne('farm-set-1.svg').flush(FAKE_SVGS.farmSet1);
302+
fixture.detectChanges();
303+
304+
// The HTTP request succeeded but the icon was not found so we logged.
305+
expect(errorHandler.handleError).toHaveBeenCalledTimes(1);
306+
expect(errorHandler.handleError.calls.argsFor(0)[0].message)
307+
.toEqual(
308+
`Error retrieving icon ${testComponent.iconName}! ` +
309+
'Unable to find icon with the name "DNE"');
285310
});
286311

287312
it('should extract icon from SVG icon set', () => {

src/material/icon/icon.ts

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,29 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {take} from 'rxjs/operators';
9+
import {coerceBooleanProperty} from '@angular/cdk/coercion';
10+
import {DOCUMENT} from '@angular/common';
1011
import {
12+
AfterViewChecked,
1113
Attribute,
1214
ChangeDetectionStrategy,
1315
Component,
1416
ElementRef,
17+
ErrorHandler,
18+
inject,
19+
Inject,
20+
InjectionToken,
1521
Input,
1622
OnChanges,
23+
OnDestroy,
1724
OnInit,
25+
Optional,
1826
SimpleChanges,
1927
ViewEncapsulation,
20-
Optional,
21-
InjectionToken,
22-
inject,
23-
Inject,
24-
OnDestroy,
25-
AfterViewChecked,
2628
} from '@angular/core';
27-
import {DOCUMENT} from '@angular/common';
2829
import {CanColor, CanColorCtor, mixinColor} from '@angular/material/core';
29-
import {coerceBooleanProperty} from '@angular/cdk/coercion';
30+
import {take} from 'rxjs/operators';
31+
3032
import {MatIconRegistry} from './icon-registry';
3133

3234

@@ -178,14 +180,15 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Aft
178180
private _elementsWithExternalReferences?: Map<Element, {name: string, value: string}[]>;
179181

180182
constructor(
181-
elementRef: ElementRef<HTMLElement>,
182-
private _iconRegistry: MatIconRegistry,
183+
elementRef: ElementRef<HTMLElement>, private _iconRegistry: MatIconRegistry,
183184
@Attribute('aria-hidden') ariaHidden: string,
184185
/**
185186
* @deprecated `location` parameter to be made required.
186187
* @breaking-change 8.0.0
187188
*/
188-
@Optional() @Inject(MAT_ICON_LOCATION) private _location?: MatIconLocation) {
189+
@Optional() @Inject(MAT_ICON_LOCATION) private _location?: MatIconLocation,
190+
// @breaking-change 9.0.0 _errorHandler parameter to be made required
191+
@Optional() private readonly _errorHandler?: ErrorHandler) {
189192
super(elementRef);
190193

191194
// If the user has not explicitly set aria-hidden, mark the icon as hidden, as this is
@@ -228,10 +231,17 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Aft
228231
if (this.svgIcon) {
229232
const [namespace, iconName] = this._splitIconName(this.svgIcon);
230233

231-
this._iconRegistry.getNamedSvgIcon(iconName, namespace).pipe(take(1)).subscribe(
232-
svg => this._setSvgElement(svg),
233-
(err: Error) => console.log(`Error retrieving icon: ${err.message}`)
234-
);
234+
this._iconRegistry.getNamedSvgIcon(iconName, namespace)
235+
.pipe(take(1))
236+
.subscribe(svg => this._setSvgElement(svg), (err: Error) => {
237+
const errorMessage = `Error retrieving icon ${namespace}:${iconName}! ${err.message}`;
238+
// @breaking-change 9.0.0 _errorHandler parameter to be made required.
239+
if (this._errorHandler) {
240+
this._errorHandler.handleError(new Error(errorMessage));
241+
} else {
242+
console.error(errorMessage);
243+
}
244+
});
235245
} else if (svgIconChanges.previousValue) {
236246
this._clearSvgElement();
237247
}

tools/public_api_guard/material/icon.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export declare class MatIcon extends _MatIconMixinBase implements OnChanges, OnI
2828
inline: boolean;
2929
svgIcon: string;
3030
constructor(elementRef: ElementRef<HTMLElement>, _iconRegistry: MatIconRegistry, ariaHidden: string,
31-
_location?: MatIconLocation | undefined);
31+
_location?: MatIconLocation | undefined, _errorHandler?: ErrorHandler | undefined);
3232
ngAfterViewChecked(): void;
3333
ngOnChanges(changes: SimpleChanges): void;
3434
ngOnDestroy(): void;

0 commit comments

Comments
 (0)