Skip to content

Commit

Permalink
fix(menu): reduce specificity of icon selector (#14389)
Browse files Browse the repository at this point in the history
In #14161 we had to tweak the selector for an icon inside a `mat-menu-item`, in order to fix an issue, however the new selector's specificity is a lot higher which can break people's overrides. These changes rework the approach to have a lower specificity selector.
  • Loading branch information
crisbeto authored and mmalerba committed Dec 5, 2018
1 parent 31f0e6d commit 74e945a
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 10 deletions.
38 changes: 29 additions & 9 deletions src/lib/icon/icon.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,19 @@ describe('MatIcon', () => {
expect(sortedClassNames(matIconElement)).toEqual(['mat-icon', 'mat-primary', 'material-icons']);
});

it('should apply a class if there is no color', () => {
let fixture = TestBed.createComponent(IconWithColor);

const testComponent = fixture.componentInstance;
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
testComponent.iconName = 'home';
testComponent.iconColor = '';
fixture.detectChanges();

expect(sortedClassNames(matIconElement))
.toEqual(['mat-icon', 'mat-icon-no-color', 'material-icons']);
});

it('should mark mat-icon as aria-hidden by default', () => {
const fixture = TestBed.createComponent(IconWithLigature);
const iconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
Expand Down Expand Up @@ -121,7 +134,8 @@ describe('MatIcon', () => {
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
testComponent.iconName = 'home';
fixture.detectChanges();
expect(sortedClassNames(matIconElement)).toEqual(['mat-icon', 'material-icons']);
expect(sortedClassNames(matIconElement))
.toEqual(['mat-icon', 'mat-icon-no-color', 'material-icons']);
});

it('should use alternate icon font if set', () => {
Expand All @@ -133,7 +147,7 @@ describe('MatIcon', () => {
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
testComponent.iconName = 'home';
fixture.detectChanges();
expect(sortedClassNames(matIconElement)).toEqual(['mat-icon', 'myfont']);
expect(sortedClassNames(matIconElement)).toEqual(['mat-icon', 'mat-icon-no-color', 'myfont']);
});
});

Expand Down Expand Up @@ -682,17 +696,20 @@ describe('MatIcon', () => {
testComponent.fontSet = 'f1';
testComponent.fontIcon = 'house';
fixture.detectChanges();
expect(sortedClassNames(matIconElement)).toEqual(['font1', 'house', 'mat-icon']);
expect(sortedClassNames(matIconElement))
.toEqual(['font1', 'house', 'mat-icon', 'mat-icon-no-color']);

testComponent.fontSet = 'f2';
testComponent.fontIcon = 'igloo';
fixture.detectChanges();
expect(sortedClassNames(matIconElement)).toEqual(['f2', 'igloo', 'mat-icon']);
expect(sortedClassNames(matIconElement))
.toEqual(['f2', 'igloo', 'mat-icon', 'mat-icon-no-color']);

testComponent.fontSet = 'f3';
testComponent.fontIcon = 'tent';
fixture.detectChanges();
expect(sortedClassNames(matIconElement)).toEqual(['f3', 'mat-icon', 'tent']);
expect(sortedClassNames(matIconElement))
.toEqual(['f3', 'mat-icon', 'mat-icon-no-color', 'tent']);
});

it('should handle values with extraneous spaces being passed in to `fontSet`', () => {
Expand All @@ -704,14 +721,15 @@ describe('MatIcon', () => {
fixture.detectChanges();
}).not.toThrow();

expect(sortedClassNames(matIconElement)).toEqual(['font', 'mat-icon']);
expect(sortedClassNames(matIconElement)).toEqual(['font', 'mat-icon', 'mat-icon-no-color']);

expect(() => {
fixture.componentInstance.fontSet = ' changed';
fixture.detectChanges();
}).not.toThrow();

expect(sortedClassNames(matIconElement)).toEqual(['changed', 'mat-icon']);
expect(sortedClassNames(matIconElement))
.toEqual(['changed', 'mat-icon', 'mat-icon-no-color']);
});

it('should handle values with extraneous spaces being passed in to `fontIcon`', () => {
Expand All @@ -723,14 +741,16 @@ describe('MatIcon', () => {
fixture.detectChanges();
}).not.toThrow();

expect(sortedClassNames(matIconElement)).toEqual(['font', 'mat-icon', 'material-icons']);
expect(sortedClassNames(matIconElement))
.toEqual(['font', 'mat-icon', 'mat-icon-no-color', 'material-icons']);

expect(() => {
fixture.componentInstance.fontIcon = ' changed';
fixture.detectChanges();
}).not.toThrow();

expect(sortedClassNames(matIconElement)).toEqual(['changed', 'mat-icon', 'material-icons']);
expect(sortedClassNames(matIconElement))
.toEqual(['changed', 'mat-icon', 'mat-icon-no-color', 'material-icons']);
});

});
Expand Down
1 change: 1 addition & 0 deletions src/lib/icon/icon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ const funcIriPattern = /^url\(['"]?#(.*?)['"]?\)$/;
'role': 'img',
'class': 'mat-icon',
'[class.mat-icon-inline]': 'inline',
'[class.mat-icon-no-color]': 'color !== "primary" && color !== "accent" && color !== "warn"',
},
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/menu/_menu-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
}
}

.mat-menu-item .mat-icon:not(.mat-primary):not(.mat-accent):not(.mat-warn),
.mat-menu-item .mat-icon-no-color,
.mat-menu-item-submenu-trigger::after {
color: mat-color($foreground, 'icon');
}
Expand Down

0 comments on commit 74e945a

Please sign in to comment.