From 22bfa9ec4a20096da90ffe8e319065753ba1d77d Mon Sep 17 00:00:00 2001 From: Glen Staes Date: Thu, 23 Nov 2017 15:05:09 +0100 Subject: [PATCH 1/6] fix(menu): Set menu-item icon color if not set on mat-icon Set the theme icon color only if the `color` input property was not set on the `mat-icon` component. Fixes #8594 --- src/lib/menu/_menu-theme.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/menu/_menu-theme.scss b/src/lib/menu/_menu-theme.scss index 73bd3cd702e8..53053c5dcdc0 100644 --- a/src/lib/menu/_menu-theme.scss +++ b/src/lib/menu/_menu-theme.scss @@ -20,7 +20,7 @@ } } - .mat-menu-item .mat-icon, + .mat-menu-item .mat-icon:not([color]), .mat-menu-item-submenu-trigger::after { color: mat-color($foreground, 'icon'); } From 2bd57a4210bdb081a11c293102784b9cc6c5f8d2 Mon Sep 17 00:00:00 2001 From: Glen Staes Date: Thu, 23 Nov 2017 23:34:23 +0100 Subject: [PATCH 2/6] mat-menu-item with mat-icon[color] unit tests Add unit tests for setting the mat-icon[color] attribute on a menu item --- src/lib/menu/menu.spec.ts | 44 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 96ceede4320a..e835fc761520 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -25,6 +25,7 @@ import { MenuPositionY, MatMenuItem, } from './index'; +import { MatIconModule } from "../icon/index"; import {MENU_PANEL_TOP_PADDING} from './menu-trigger'; import {MatRipple} from '@angular/material/core'; import { @@ -44,7 +45,7 @@ describe('MatMenu', () => { beforeEach(async(() => { dir = 'ltr'; TestBed.configureTestingModule({ - imports: [MatMenuModule, NoopAnimationsModule], + imports: [MatIconModule, MatMenuModule, NoopAnimationsModule], declarations: [ SimpleMenu, PositionedMenu, @@ -54,6 +55,7 @@ describe('MatMenu', () => { NestedMenu, NestedMenuCustomElevation, NestedMenuRepeater, + ItemIconColorsMenu, FakeIcon ], providers: [ @@ -223,6 +225,30 @@ describe('MatMenu', () => { expect(fixture.componentInstance.items.first.getLabel()).toBe('Item'); }); + it('should have an icon with the default theme color if mat-icon has no color attribute set', () => { + const fixture = TestBed.createComponent(ItemIconColorsMenu); + fixture.detectChanges(); + + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + + const matIconElement = overlayContainerElement.querySelector("mat-icon:not([color])") as HTMLElement; + const computedIconStyle = window.getComputedStyle(matIconElement); + expect(computedIconStyle.color).toBe("rgba(0, 0, 0, 0.54)"); + }); + + it('should have an icon with the provided color if mat-icon has the color attribute set', () => { + const fixture = TestBed.createComponent(ItemIconColorsMenu); + fixture.detectChanges(); + + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + + const matIconElement = overlayContainerElement.querySelector("mat-icon[color]") as HTMLElement; + const computedIconStyle = window.getComputedStyle(matIconElement); + expect(computedIconStyle.color).toBe("rgb(255, 64, 129)"); + }); + it('should filter out non-text nodes when figuring out the label', () => { const fixture = TestBed.createComponent(SimpleMenu); fixture.detectChanges(); @@ -1380,6 +1406,22 @@ class NestedMenuRepeater { items = ['one', 'two', 'three']; } +@Component({ + template: ` + + + + + + ` +}) +class ItemIconColorsMenu { + @ViewChild(MatMenuTrigger) trigger: MatMenuTrigger; + @ViewChild('triggerEl') triggerEl: ElementRef; + @ViewChild(MatMenu) menu: MatMenu; + @ViewChildren(MatMenuItem) items: QueryList; +} + @Component({ selector: 'fake-icon', template: '' From 79b428c4de9b5369b33217eb3602dbdd3c395d19 Mon Sep 17 00:00:00 2001 From: Glen Staes Date: Thu, 23 Nov 2017 23:41:43 +0100 Subject: [PATCH 3/6] Make ItemIconColorsMenu more clear Related to #8594 --- src/lib/menu/menu.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index e835fc761520..0f84fc9501c8 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -1411,7 +1411,7 @@ class NestedMenuRepeater { - + ` }) From 693b4db79c190d76251844c95db750f2a8e9157b Mon Sep 17 00:00:00 2001 From: Glen Staes Date: Thu, 23 Nov 2017 23:59:51 +0100 Subject: [PATCH 4/6] Fix linting issues --- src/lib/menu/menu.spec.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 0f84fc9501c8..6304cddbba05 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -25,7 +25,7 @@ import { MenuPositionY, MatMenuItem, } from './index'; -import { MatIconModule } from "../icon/index"; +import { MatIconModule } from '../icon/index'; import {MENU_PANEL_TOP_PADDING} from './menu-trigger'; import {MatRipple} from '@angular/material/core'; import { @@ -225,16 +225,18 @@ describe('MatMenu', () => { expect(fixture.componentInstance.items.first.getLabel()).toBe('Item'); }); - it('should have an icon with the default theme color if mat-icon has no color attribute set', () => { + it('should have an icon with the default theme color if mat-icon has no color attribute set', + () => { const fixture = TestBed.createComponent(ItemIconColorsMenu); fixture.detectChanges(); fixture.componentInstance.trigger.openMenu(); fixture.detectChanges(); - const matIconElement = overlayContainerElement.querySelector("mat-icon:not([color])") as HTMLElement; + const matIconElement = + overlayContainerElement.querySelector('mat-icon:not([color])') as HTMLElement; const computedIconStyle = window.getComputedStyle(matIconElement); - expect(computedIconStyle.color).toBe("rgba(0, 0, 0, 0.54)"); + expect(computedIconStyle.color).toBe('rgba(0, 0, 0, 0.54)'); }); it('should have an icon with the provided color if mat-icon has the color attribute set', () => { @@ -244,9 +246,9 @@ describe('MatMenu', () => { fixture.componentInstance.trigger.openMenu(); fixture.detectChanges(); - const matIconElement = overlayContainerElement.querySelector("mat-icon[color]") as HTMLElement; + const matIconElement = overlayContainerElement.querySelector('mat-icon[color]') as HTMLElement; const computedIconStyle = window.getComputedStyle(matIconElement); - expect(computedIconStyle.color).toBe("rgb(255, 64, 129)"); + expect(computedIconStyle.color).toBe('rgb(255, 64, 129)'); }); it('should filter out non-text nodes when figuring out the label', () => { @@ -1411,7 +1413,9 @@ class NestedMenuRepeater { - + ` }) From 597de57c246690c763e613e7c106536b524483bc Mon Sep 17 00:00:00 2001 From: Glen Staes Date: Fri, 24 Nov 2017 00:04:51 +0100 Subject: [PATCH 5/6] Fix linting --- src/lib/menu/menu.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 6304cddbba05..d3f69ba21509 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -225,7 +225,7 @@ describe('MatMenu', () => { expect(fixture.componentInstance.items.first.getLabel()).toBe('Item'); }); - it('should have an icon with the default theme color if mat-icon has no color attribute set', + it('should have an icon with the default theme color if mat-icon has no color attribute set', () => { const fixture = TestBed.createComponent(ItemIconColorsMenu); fixture.detectChanges(); @@ -233,7 +233,7 @@ describe('MatMenu', () => { fixture.componentInstance.trigger.openMenu(); fixture.detectChanges(); - const matIconElement = + const matIconElement = overlayContainerElement.querySelector('mat-icon:not([color])') as HTMLElement; const computedIconStyle = window.getComputedStyle(matIconElement); expect(computedIconStyle.color).toBe('rgba(0, 0, 0, 0.54)'); @@ -1413,8 +1413,8 @@ class NestedMenuRepeater { - ` From cabb1743720500c55fc06ca471463e73ee349598 Mon Sep 17 00:00:00 2001 From: Glen Staes Date: Wed, 29 Nov 2017 21:38:26 +0100 Subject: [PATCH 6/6] Removed "CSS engine tests" --- src/lib/menu/menu.spec.ts | 48 +-------------------------------------- 1 file changed, 1 insertion(+), 47 deletions(-) diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index d3f69ba21509..96ceede4320a 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -25,7 +25,6 @@ import { MenuPositionY, MatMenuItem, } from './index'; -import { MatIconModule } from '../icon/index'; import {MENU_PANEL_TOP_PADDING} from './menu-trigger'; import {MatRipple} from '@angular/material/core'; import { @@ -45,7 +44,7 @@ describe('MatMenu', () => { beforeEach(async(() => { dir = 'ltr'; TestBed.configureTestingModule({ - imports: [MatIconModule, MatMenuModule, NoopAnimationsModule], + imports: [MatMenuModule, NoopAnimationsModule], declarations: [ SimpleMenu, PositionedMenu, @@ -55,7 +54,6 @@ describe('MatMenu', () => { NestedMenu, NestedMenuCustomElevation, NestedMenuRepeater, - ItemIconColorsMenu, FakeIcon ], providers: [ @@ -225,32 +223,6 @@ describe('MatMenu', () => { expect(fixture.componentInstance.items.first.getLabel()).toBe('Item'); }); - it('should have an icon with the default theme color if mat-icon has no color attribute set', - () => { - const fixture = TestBed.createComponent(ItemIconColorsMenu); - fixture.detectChanges(); - - fixture.componentInstance.trigger.openMenu(); - fixture.detectChanges(); - - const matIconElement = - overlayContainerElement.querySelector('mat-icon:not([color])') as HTMLElement; - const computedIconStyle = window.getComputedStyle(matIconElement); - expect(computedIconStyle.color).toBe('rgba(0, 0, 0, 0.54)'); - }); - - it('should have an icon with the provided color if mat-icon has the color attribute set', () => { - const fixture = TestBed.createComponent(ItemIconColorsMenu); - fixture.detectChanges(); - - fixture.componentInstance.trigger.openMenu(); - fixture.detectChanges(); - - const matIconElement = overlayContainerElement.querySelector('mat-icon[color]') as HTMLElement; - const computedIconStyle = window.getComputedStyle(matIconElement); - expect(computedIconStyle.color).toBe('rgb(255, 64, 129)'); - }); - it('should filter out non-text nodes when figuring out the label', () => { const fixture = TestBed.createComponent(SimpleMenu); fixture.detectChanges(); @@ -1408,24 +1380,6 @@ class NestedMenuRepeater { items = ['one', 'two', 'three']; } -@Component({ - template: ` - - - - - - ` -}) -class ItemIconColorsMenu { - @ViewChild(MatMenuTrigger) trigger: MatMenuTrigger; - @ViewChild('triggerEl') triggerEl: ElementRef; - @ViewChild(MatMenu) menu: MatMenu; - @ViewChildren(MatMenuItem) items: QueryList; -} - @Component({ selector: 'fake-icon', template: ''