Skip to content

Commit dccddd9

Browse files
jermoweryjelbourn
authored andcommitted
fix(icon): use ErrorHandler to log MatIcon errors (#16967)
Currently a console.log is used. This change switches to using ErrorHandler. ErrorHandler is better because it allows the user of MatIcon to define how Errors should be handled instead of relying on the console.
1 parent 62e1476 commit dccddd9

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)