From abf069e9a47511d66057abc3285587eae0b05ce8 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 31 Mar 2017 19:17:11 +0200 Subject: [PATCH 1/4] test(select): various test inconsistencies * Fixes a select test that occasionally fails to capture the error inside the expect, causing it to get logged without actually being thrown. This seems to be a Zone-related issue since wrapping a few calls in `async` makes it behave correctly. * Fixes an RTL test that fails if it is made async. This incorporates the changes from #3866 and #3163 and fixes the underlying issue. It appears that the test was failing, because IE does a slight twitch on the select panel since it goes from a pixel-based `min-width` to one with `calc` and percentages to just percentages. I've set the default width to 100% which is the same as the pixel-based one, which appears to make IE happy. --- src/lib/select/select.scss | 2 +- src/lib/select/select.spec.ts | 73 +++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/lib/select/select.scss b/src/lib/select/select.scss index d7c9e5a24ad6..9d7fcc1c2c12 100644 --- a/src/lib/select/select.scss +++ b/src/lib/select/select.scss @@ -125,9 +125,9 @@ $mat-select-trigger-font-size: 16px !default; padding-top: 0; padding-bottom: 0; max-height: $mat-select-panel-max-height; + min-width: 100%; // prevents some animation twitching and test inconsistencies in IE11 @include cdk-high-contrast { outline: solid 1px; } } - diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index ce39310ce301..82d636b4b262 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -988,37 +988,50 @@ describe('MdSelect', () => { select.style.marginRight = '30px'; }); - it('should align the trigger and the selected option on the x-axis in ltr', () => { - trigger.click(); - fixture.detectChanges(); + it('should align the trigger and the selected option on the x-axis in ltr', async(() => { + fixture.whenStable().then(() => { + fixture.detectChanges(); - const triggerLeft = trigger.getBoundingClientRect().left; - const firstOptionLeft = - document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().left; + trigger.click(); + fixture.detectChanges(); - // Each option is 32px wider than the trigger, so it must be adjusted 16px - // to ensure the text overlaps correctly. - expect(firstOptionLeft.toFixed(2)) - .toEqual((triggerLeft - 16).toFixed(2), + fixture.whenStable().then(() => { + fixture.detectChanges(); + + const triggerLeft = trigger.getBoundingClientRect().left; + const firstOptionLeft = document.querySelector('.cdk-overlay-pane md-option') + .getBoundingClientRect().left; + + // Each option is 32px wider than the trigger, so it must be adjusted 16px + // to ensure the text overlaps correctly. + expect(firstOptionLeft.toFixed(2)).toEqual((triggerLeft - 16).toFixed(2), `Expected trigger to align with the selected option on the x-axis in LTR.`); - }); + }); + }); + })); - it('should align the trigger and the selected option on the x-axis in rtl', () => { + it('should align the trigger and the selected option on the x-axis in rtl', async(() => { dir.value = 'rtl'; + fixture.whenStable().then(() => { + fixture.detectChanges(); - trigger.click(); - fixture.detectChanges(); - - const triggerRight = trigger.getBoundingClientRect().right; - const firstOptionRight = - document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right; + trigger.click(); + fixture.detectChanges(); - // Each option is 32px wider than the trigger, so it must be adjusted 16px - // to ensure the text overlaps correctly. - expect(firstOptionRight.toFixed(2)) - .toEqual((triggerRight + 16).toFixed(2), - `Expected trigger to align with the selected option on the x-axis in RTL.`); - }); + fixture.whenStable().then(() => { + fixture.detectChanges(); + const triggerRight = trigger.getBoundingClientRect().right; + const firstOptionRight = + document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right; + + // Each option is 32px wider than the trigger, so it must be adjusted 16px + // to ensure the text overlaps correctly. + expect(firstOptionRight.toFixed(2)) + .toEqual((triggerRight + 16).toFixed(2), + `Expected trigger to align with the selected option on the x-axis in RTL.`); + }); + }); + })); }); describe('x-axis positioning in multi select mode', () => { @@ -1450,13 +1463,13 @@ describe('MdSelect', () => { let testInstance: MultiSelect; let trigger: HTMLElement; - beforeEach(() => { + beforeEach(async(() => { fixture = TestBed.createComponent(MultiSelect); testInstance = fixture.componentInstance; fixture.detectChanges(); trigger = fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement; - }); + })); it('should be able to select multiple values', () => { trigger.click(); @@ -1616,17 +1629,17 @@ describe('MdSelect', () => { expect(trigger.textContent).toContain('Tacos, Pizza, Steak'); }); - it('should throw an exception when trying to set a non-array value', () => { + it('should throw an exception when trying to set a non-array value', async(() => { expect(() => { testInstance.control.setValue('not-an-array'); }).toThrowError(wrappedErrorMessage(new MdSelectNonArrayValueError())); - }); + })); - it('should throw an exception when trying to change multiple mode after init', () => { + it('should throw an exception when trying to change multiple mode after init', async(() => { expect(() => { testInstance.select.multiple = false; }).toThrowError(wrappedErrorMessage(new MdSelectDynamicMultipleError())); - }); + })); it('should pass the `multiple` value to all of the option instances', async(() => { trigger.click(); From 202fec6c0101d0e82bc353ec77f525493513438a Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 1 Apr 2017 11:09:23 +0200 Subject: [PATCH 2/4] chore: verify that changing the min-width causes failures --- src/lib/select/select.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/select/select.scss b/src/lib/select/select.scss index 9d7fcc1c2c12..bd65367062b0 100644 --- a/src/lib/select/select.scss +++ b/src/lib/select/select.scss @@ -125,7 +125,7 @@ $mat-select-trigger-font-size: 16px !default; padding-top: 0; padding-bottom: 0; max-height: $mat-select-panel-max-height; - min-width: 100%; // prevents some animation twitching and test inconsistencies in IE11 + // min-width: 100%; // prevents some animation twitching and test inconsistencies in IE11 @include cdk-high-contrast { outline: solid 1px; From 7779350a8b7fb0f233a44ea0481b52059dfcada5 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 1 Apr 2017 11:25:17 +0200 Subject: [PATCH 3/4] chore: revert min-width change --- src/lib/select/select.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/select/select.scss b/src/lib/select/select.scss index bd65367062b0..9d7fcc1c2c12 100644 --- a/src/lib/select/select.scss +++ b/src/lib/select/select.scss @@ -125,7 +125,7 @@ $mat-select-trigger-font-size: 16px !default; padding-top: 0; padding-bottom: 0; max-height: $mat-select-panel-max-height; - // min-width: 100%; // prevents some animation twitching and test inconsistencies in IE11 + min-width: 100%; // prevents some animation twitching and test inconsistencies in IE11 @include cdk-high-contrast { outline: solid 1px; From 448cb2e4d4315969be3655bc7af67eecff5942c3 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 1 Apr 2017 22:06:02 +0200 Subject: [PATCH 4/4] chore: remove a few unnecessary expressions --- src/lib/select/select.spec.ts | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 82d636b4b262..1577f252e85c 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -989,24 +989,18 @@ describe('MdSelect', () => { }); it('should align the trigger and the selected option on the x-axis in ltr', async(() => { - fixture.whenStable().then(() => { - fixture.detectChanges(); - - trigger.click(); - fixture.detectChanges(); - - fixture.whenStable().then(() => { - fixture.detectChanges(); - - const triggerLeft = trigger.getBoundingClientRect().left; - const firstOptionLeft = document.querySelector('.cdk-overlay-pane md-option') - .getBoundingClientRect().left; + trigger.click(); + fixture.detectChanges(); - // Each option is 32px wider than the trigger, so it must be adjusted 16px - // to ensure the text overlaps correctly. - expect(firstOptionLeft.toFixed(2)).toEqual((triggerLeft - 16).toFixed(2), - `Expected trigger to align with the selected option on the x-axis in LTR.`); - }); + fixture.whenStable().then(() => { + const triggerLeft = trigger.getBoundingClientRect().left; + const firstOptionLeft = document.querySelector('.cdk-overlay-pane md-option') + .getBoundingClientRect().left; + + // Each option is 32px wider than the trigger, so it must be adjusted 16px + // to ensure the text overlaps correctly. + expect(firstOptionLeft.toFixed(2)).toEqual((triggerLeft - 16).toFixed(2), + `Expected trigger to align with the selected option on the x-axis in LTR.`); }); })); @@ -1019,7 +1013,6 @@ describe('MdSelect', () => { fixture.detectChanges(); fixture.whenStable().then(() => { - fixture.detectChanges(); const triggerRight = trigger.getBoundingClientRect().right; const firstOptionRight = document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right;