From 113997b1b6c02685ee3e8f33e101f6154ff5404a Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 5 Jun 2018 13:22:08 -0700 Subject: [PATCH] fix(select): Improve the a11y of select component by making overlay always exist in the DOM --- src/cdk/overlay/overlay-directives.spec.ts | 14 ++++ src/cdk/overlay/overlay-directives.ts | 21 ++++-- src/cdk/overlay/overlay-ref.ts | 8 +- src/lib/select/select.html | 11 ++- src/lib/select/select.spec.ts | 86 ++++++++++++++++------ src/lib/select/select.ts | 51 ++++++++++--- 6 files changed, 146 insertions(+), 45 deletions(-) diff --git a/src/cdk/overlay/overlay-directives.spec.ts b/src/cdk/overlay/overlay-directives.spec.ts index 875a0a16ecc2..855946b32c07 100644 --- a/src/cdk/overlay/overlay-directives.spec.ts +++ b/src/cdk/overlay/overlay-directives.spec.ts @@ -120,6 +120,18 @@ describe('Overlay directives', () => { 'Expected overlay to have been detached.'); }); + it('should not close when pressing escape when changed the setting of "disableClose"', () => { + fixture.componentInstance.isOpen = true; + fixture.componentInstance.disableClose = true; + fixture.detectChanges(); + + dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); + fixture.detectChanges(); + + expect(overlayContainerElement.textContent!.trim()).not.toBe('', + 'Expected overlay to have not been detached.'); + }); + it('should not depend on the order in which the `origin` and `open` are set', async(() => { fixture.destroy(); @@ -479,6 +491,7 @@ describe('Overlay directives', () => { [cdkConnectedOverlayFlexibleDimensions]="flexibleDimensions" [cdkConnectedOverlayGrowAfterOpen]="growAfterOpen" [cdkConnectedOverlayPush]="push" + [cdkConnectedOverlayDisableClose]="disableClose" cdkConnectedOverlayBackdropClass="mat-test-class" (backdropClick)="backdropClickHandler($event)" [cdkConnectedOverlayOffsetX]="offsetX" @@ -511,6 +524,7 @@ class ConnectedOverlayDirectiveTest { flexibleDimensions: boolean; growAfterOpen: boolean; push: boolean; + disableClose: boolean = false; backdropClickHandler = jasmine.createSpy('backdropClick handler'); positionChangeHandler = jasmine.createSpy('positionChange handler'); keydownHandler = jasmine.createSpy('keydown handler'); diff --git a/src/cdk/overlay/overlay-directives.ts b/src/cdk/overlay/overlay-directives.ts index da50ed72485c..6d4630a6eb9c 100644 --- a/src/cdk/overlay/overlay-directives.ts +++ b/src/cdk/overlay/overlay-directives.ts @@ -114,6 +114,8 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges { private _offsetX: number; private _offsetY: number; private _position: FlexibleConnectedPositionStrategy; + private _disableClose = false; + private _open = false; /** Origin for the connected overlay. */ @Input('cdkConnectedOverlayOrigin') origin: CdkOverlayOrigin; @@ -165,8 +167,15 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges { @Input('cdkConnectedOverlayScrollStrategy') scrollStrategy: ScrollStrategy = this._scrollStrategy(); + /** Whether the overlay closes when the user pressed the Escape key. */ + @Input('cdkConnectedOverlayDisableClose') + get disableClose() { return this._disableClose; } + set disableClose(value: any) { this._disableClose = coerceBooleanProperty(value); } + /** Whether the overlay is open. */ - @Input('cdkConnectedOverlayOpen') open: boolean = false; + @Input('cdkConnectedOverlayOpen') + get open() { return this._open; } + set open(value: any) { this._open = coerceBooleanProperty(value); } /** Whether or not the overlay should attach a backdrop. */ @Input('cdkConnectedOverlayHasBackdrop') @@ -340,7 +349,7 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges { this._overlayRef!.keydownEvents().subscribe((event: KeyboardEvent) => { this.overlayKeydown.next(event); - if (event.keyCode === ESCAPE) { + if (!this.disableClose && event.keyCode === ESCAPE) { this._detachOverlay(); } }); @@ -359,11 +368,9 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges { this.attach.emit(); } - if (this.hasBackdrop) { - this._backdropSubscription = this._overlayRef.backdropClick().subscribe(event => { - this.backdropClick.emit(event); - }); - } + this._backdropSubscription = this._overlayRef.backdropClicks().subscribe(event => { + this.backdropClick.emit(event); + }); } /** Detaches the overlay and unsubscribes to backdrop clicks if backdrop exists */ diff --git a/src/cdk/overlay/overlay-ref.ts b/src/cdk/overlay/overlay-ref.ts index f55a36abcfb7..c23df3de8262 100644 --- a/src/cdk/overlay/overlay-ref.ts +++ b/src/cdk/overlay/overlay-ref.ts @@ -124,7 +124,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference { this._togglePointerEvents(true); if (this._config.hasBackdrop) { - this._attachBackdrop(); + this.attachBackdrop(); } if (this._config.panelClass) { @@ -213,6 +213,10 @@ export class OverlayRef implements PortalOutlet, OverlayReference { return this._portalOutlet.hasAttached(); } + backdropClicks() { + return this._backdropClick; + } + /** Gets an observable that emits when the backdrop has been clicked. */ backdropClick(): Observable { return this._backdropClick.asObservable(); @@ -293,7 +297,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference { } /** Attaches a backdrop for this overlay. */ - private _attachBackdrop() { + attachBackdrop() { const showingClass = 'cdk-overlay-backdrop-showing'; this._backdropElement = this._document.createElement('div'); diff --git a/src/lib/select/select.html b/src/lib/select/select.html index f22f93087785..fb73d7a4d8f6 100644 --- a/src/lib/select/select.html +++ b/src/lib/select/select.html @@ -18,32 +18,31 @@ -
-
diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index fe29c1e208e5..611f22a6a7f9 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -333,7 +333,6 @@ describe('MatSelect', () => { it('should open a single-selection select using ALT + DOWN_ARROW', fakeAsync(() => { const {control: formControl, select: selectInstance} = fixture.componentInstance; - expect(selectInstance.panelOpen).toBe(false, 'Expected select to be closed.'); expect(formControl.value).toBeFalsy('Expected no initial value.'); @@ -435,6 +434,10 @@ describe('MatSelect', () => { expect(instance.select.panelOpen).toBe(true, 'Expected panel to be open.'); expect(instance.control.value).toBe(initialValue, 'Expected value to stay the same.'); expect(event.defaultPrevented).toBe(true, 'Expected default to be prevented.'); + + const options = + overlayContainerElement.querySelectorAll('mat-option') as NodeListOf; + expect(options.length).toEqual(8, 'Expect select has 8 options'); })); it('should open the panel when pressing a horizontal arrow key on closed multiple select', @@ -790,6 +793,10 @@ describe('MatSelect', () => { overlayContainerElement.querySelectorAll('mat-option') as NodeListOf; })); + it('should have correct number of options', fakeAsync(() => { + expect(options.length).toEqual(8, 'Expect select has 8 options'); + })); + it('should set the role of mat-option to option', fakeAsync(() => { expect(options[0].getAttribute('role')).toEqual('option'); expect(options[1].getAttribute('role')).toEqual('option'); @@ -904,7 +911,6 @@ describe('MatSelect', () => { fixture.detectChanges(); flush(); - expect(overlayContainerElement.textContent).toEqual(''); expect(fixture.componentInstance.select.panelOpen).toBe(false); })); @@ -920,12 +926,13 @@ describe('MatSelect', () => { fixture.detectChanges(); flush(); - expect(overlayContainerElement.textContent).toEqual(''); expect(fixture.componentInstance.select.panelOpen).toBe(false); })); it('should set the width of the overlay based on the trigger', fakeAsync(() => { trigger.style.width = '200px'; + fixture.detectChanges(); + flush(); trigger.click(); fixture.detectChanges(); @@ -1084,6 +1091,7 @@ describe('MatSelect', () => { it('should be able to render options inside groups with an ng-container', fakeAsync(() => { fixture.destroy(); + flush(); const groupFixture = TestBed.createComponent(SelectWithGroupsAndNgContainer); groupFixture.detectChanges(); @@ -1343,6 +1351,7 @@ describe('MatSelect', () => { it('should not select options inside a disabled group', fakeAsync(() => { fixture.destroy(); + flush(); const groupFixture = TestBed.createComponent(SelectWithGroups); groupFixture.detectChanges(); @@ -1384,6 +1393,8 @@ describe('MatSelect', () => { it('should handle accessing `optionSelectionChanges` before the options are initialized', fakeAsync(() => { fixture.destroy(); + flush(); + fixture = TestBed.createComponent(BasicSelect); let spy = jasmine.createSpy('option selection spy'); @@ -1639,8 +1650,6 @@ describe('MatSelect', () => { trigger.click(); fixture.detectChanges(); - expect(overlayContainerElement.textContent) - .toEqual('', `Expected select panel to stay closed.`); expect(fixture.componentInstance.select.panelOpen) .toBe(false, `Expected select panelOpen property to stay false.`); @@ -1765,6 +1774,7 @@ describe('MatSelect', () => { it('should skip option group labels', fakeAsync(() => { fixture.destroy(); + flush(); const groupFixture = TestBed.createComponent(SelectWithGroups); @@ -1908,8 +1918,6 @@ describe('MatSelect', () => { trigger.click(); fixture.detectChanges(); - expect(overlayContainerElement.textContent) - .toEqual('', `Expected select panel to stay closed.`); expect(fixture.componentInstance.select.panelOpen) .toBe(false, `Expected select panelOpen property to stay false.`); @@ -1937,9 +1945,11 @@ describe('MatSelect', () => { it('should handle nesting in an ngIf', fakeAsync(() => { const fixture = TestBed.createComponent(NgIfSelect); fixture.detectChanges(); + flush(); fixture.componentInstance.isShowing = true; fixture.detectChanges(); + flush(); const trigger = fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement; trigger.style.width = '300px'; @@ -2001,13 +2011,14 @@ describe('MatSelect', () => { options = overlayContainerElement.querySelectorAll('mat-option') as NodeListOf; + // selects[0] has 2 options, so selects[1]'s option starts at options[2] expect(selects[1].nativeElement.getAttribute('aria-owns')) - .toContain(options[0].id, `Expected aria-owns to contain IDs of its child options.`); + .toContain(options[2].id, `Expected aria-owns to contain IDs of its child options.`); expect(selects[1].nativeElement.getAttribute('aria-owns')) - .toContain(options[1].id, `Expected aria-owns to contain IDs of its child options.`); + .toContain(options[3].id, `Expected aria-owns to contain IDs of its child options.`); })); - it('should remove aria-owns when the options are not visible', fakeAsync(() => { + it('should have aria-owns when the options are not visible', fakeAsync(() => { const select = fixture.debugElement.query(By.css('mat-select')); expect(select.nativeElement.hasAttribute('aria-owns')) @@ -2020,7 +2031,7 @@ describe('MatSelect', () => { flush(); expect(select.nativeElement.hasAttribute('aria-owns')) - .toBe(false, 'Expected select not to have aria-owns when closed.'); + .toBe(true, 'Expected select to have aria-owns when closed.'); })); it('should set the option id properly', fakeAsync(() => { @@ -2044,7 +2055,7 @@ describe('MatSelect', () => { overlayContainerElement.querySelectorAll('mat-option') as NodeListOf; expect(options[0].id) .toContain('mat-option', `Expected option ID to have the correct prefix.`); - expect(options[0].id).not.toEqual(firstOptionID, `Expected option IDs to be unique.`); + expect(options[0].id).toEqual(firstOptionID, `Expected option Id stays the same`); expect(options[0].id).not.toEqual(options[1].id, `Expected option IDs to be unique.`); })); }); @@ -2089,6 +2100,7 @@ describe('MatSelect', () => { it ('should default to global floating label type', fakeAsync(() => { fixture.destroy(); + flush(); TestBed.resetTestingModule(); TestBed.configureTestingModule({ @@ -2147,7 +2159,7 @@ describe('MatSelect', () => { describe('change events', () => { beforeEach(async(() => configureMatSelectTestingModule([SelectWithPlainTabindex]))); - it('should complete the stateChanges stream on destroy', () => { + it('should complete the stateChanges stream on destroy', fakeAsync(() => { const fixture = TestBed.createComponent(SelectWithPlainTabindex); fixture.detectChanges(); @@ -2158,9 +2170,10 @@ describe('MatSelect', () => { const subscription = select.stateChanges.subscribe(undefined, undefined, spy); fixture.destroy(); + flush(); expect(spy).toHaveBeenCalled(); subscription.unsubscribe(); - }); + })); }); describe('when initially hidden', () => { @@ -2401,6 +2414,7 @@ describe('MatSelect', () => { }; fixture.destroy(); + flush(); TestBed.resetTestingModule().configureTestingModule({ imports: [MatSelectModule, ReactiveFormsModule, FormsModule, NoopAnimationsModule], @@ -3028,6 +3042,7 @@ describe('MatSelect', () => { } fixture.destroy(); + flush(); let groupFixture = TestBed.createComponent(SelectWithGroups); groupFixture.detectChanges(); @@ -3191,10 +3206,12 @@ describe('MatSelect', () => { // Push the select to a position with not enough space on the bottom to open formField.style.bottom = '56px'; fixture.detectChanges(); + flush(); // Select an option that cannot be scrolled any farther upward fixture.componentInstance.control.setValue('coke-0'); fixture.detectChanges(); + flush(); trigger.click(); fixture.detectChanges(); @@ -3223,6 +3240,8 @@ describe('MatSelect', () => { fakeAsync(() => { // Push the select to a position with not enough space on the top to open formField.style.top = '85px'; + fixture.detectChanges(); + flush(); // Select an option that cannot be scrolled any farther downward fixture.componentInstance.control.setValue('sushi-7'); @@ -3258,6 +3277,9 @@ describe('MatSelect', () => { it('should stay within the viewport when overflowing on the left in ltr', fakeAsync(() => { formField.style.left = '-100px'; + fixture.detectChanges(); + flush(); + trigger.click(); fixture.detectChanges(); flush(); @@ -3271,6 +3293,9 @@ describe('MatSelect', () => { it('should stay within the viewport when overflowing on the left in rtl', fakeAsync(() => { dir.value = 'rtl'; formField.style.left = '-100px'; + fixture.detectChanges(); + flush(); + trigger.click(); fixture.detectChanges(); flush(); @@ -3283,6 +3308,9 @@ describe('MatSelect', () => { it('should stay within the viewport when overflowing on the right in ltr', fakeAsync(() => { formField.style.right = '-100px'; + fixture.detectChanges(); + flush(); + trigger.click(); fixture.detectChanges(); flush(); @@ -3449,6 +3477,8 @@ describe('MatSelect', () => { // Scroll the select into view setScrollTop(1400); + fixture.detectChanges(); + flush(); // In the iOS simulator (BrowserStack & SauceLabs), adding the content to the // body causes karma's iframe for the test to stretch to fit that content once we attempt to @@ -3459,12 +3489,13 @@ describe('MatSelect', () => { return; } + const triggerBottom = trigger.getBoundingClientRect().bottom; + trigger.click(); fixture.detectChanges(); flush(); const overlayPane = overlayContainerElement.querySelector('.cdk-overlay-pane')!; - const triggerBottom = trigger.getBoundingClientRect().bottom; const overlayBottom = overlayPane.getBoundingClientRect().bottom; const difference = Math.floor(overlayBottom) - Math.floor(triggerBottom); @@ -3512,6 +3543,9 @@ describe('MatSelect', () => { beforeEach(fakeAsync(() => { formField.style.position = 'fixed'; formField.style.left = '30px'; + + fixture.detectChanges(); + flush(); })); it('should align the trigger and the selected option on the x-axis in ltr', fakeAsync(() => { @@ -3560,6 +3594,9 @@ describe('MatSelect', () => { formField.style.position = 'fixed'; formField.style.left = '60px'; + + multiFixture.detectChanges(); + flush(); })); it('should adjust for the checkbox in ltr', fakeAsync(() => { @@ -3568,8 +3605,8 @@ describe('MatSelect', () => { flush(); const triggerLeft = trigger.getBoundingClientRect().left; - const firstOptionLeft = - document.querySelector('.cdk-overlay-pane mat-option')!.getBoundingClientRect().left; + const firstOptionLeft = document.querySelector('.cdk-overlay-pane mat-option.mat-active')! + .getBoundingClientRect().left; // 44px accounts for the checkbox size, margin and the panel's padding. expect(Math.floor(firstOptionLeft)) @@ -3579,13 +3616,16 @@ describe('MatSelect', () => { it('should adjust for the checkbox in rtl', fakeAsync(() => { dir.value = 'rtl'; + multiFixture.detectChanges(); + flush(); + trigger.click(); multiFixture.detectChanges(); flush(); const triggerRight = trigger.getBoundingClientRect().right; - const firstOptionRight = - document.querySelector('.cdk-overlay-pane mat-option')!.getBoundingClientRect().right; + const firstOptionRight = document.querySelector('.cdk-overlay-pane mat-option.mat-active')! + .getBoundingClientRect().right; // 44px accounts for the checkbox size, margin and the panel's padding. expect(Math.floor(firstOptionRight)) @@ -3630,6 +3670,7 @@ describe('MatSelect', () => { dir.value = 'rtl'; groupFixture.componentInstance.control.setValue('oddish-1'); groupFixture.detectChanges(); + flush(); trigger.click(); groupFixture.detectChanges(); @@ -3662,9 +3703,11 @@ describe('MatSelect', () => { expect(Math.floor(selectedOptionLeft)).toEqual(Math.floor(triggerLeft - 16)); })); - it('should align the first option to the trigger, if nothing is selected', fakeAsync(() => { + it('should align the first option to the trigger, if nothing is selected', fakeAsync(() => { // Push down the form field so there is space for the item to completely align. formField.style.top = '100px'; + groupFixture.detectChanges(); + flush(); const menuItemHeight = 48; const triggerFontSize = 16; @@ -3677,7 +3720,8 @@ describe('MatSelect', () => { const triggerTop = trigger.getBoundingClientRect().top; - const option = overlayContainerElement.querySelector('.cdk-overlay-pane mat-option'); + const option = overlayContainerElement.querySelector + ('.cdk-overlay-pane mat-option.mat-active'); const optionTop = option ? option.getBoundingClientRect().top : 0; // There appears to be a small rounding error on IE, so we verify that the value is close, diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index cd11b499f569..7411a5a3360f 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -14,6 +14,7 @@ import { DOWN_ARROW, END, ENTER, + ESCAPE, HOME, LEFT_ARROW, RIGHT_ARROW, @@ -195,7 +196,7 @@ export class MatSelectTrigger {} '[attr.aria-required]': 'required.toString()', '[attr.aria-disabled]': 'disabled.toString()', '[attr.aria-invalid]': 'errorState', - '[attr.aria-owns]': 'panelOpen ? _optionIds : null', + '[attr.aria-owns]': '_optionIds', '[attr.aria-multiselectable]': 'multiple', '[attr.aria-describedby]': '_ariaDescribedby || null', '[attr.aria-activedescendant]': '_getAriaActiveDescendant()', @@ -560,25 +561,32 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, // Note: The computed font-size will be a string pixel value (e.g. "16px"). // `parseInt` ignores the trailing 'px' and converts this to a number. this._triggerFontSize = parseInt(getComputedStyle(this.trigger.nativeElement)['font-size']); + if (this._triggerFontSize && this.overlayDir.overlayRef && + this.overlayDir.overlayRef.overlayElement) { + this.overlayDir.overlayRef.overlayElement.style.fontSize = `${this._triggerFontSize}px`; + } this._panelOpen = true; this._keyManager.withHorizontalOrientation(null); this._calculateOverlayPosition(); this._highlightCorrectOption(); - this._changeDetectorRef.markForCheck(); - // Set the font size on the panel element once it exists. - this._ngZone.onStable.asObservable().pipe(take(1)).subscribe(() => { - if (this._triggerFontSize && this.overlayDir.overlayRef && - this.overlayDir.overlayRef.overlayElement) { - this.overlayDir.overlayRef.overlayElement.style.fontSize = `${this._triggerFontSize}px`; - } - }); + this.overlayDir.overlayRef.updateSize({minWidth: `${this._triggerRect.width}px`}); + if (this._dir) { + this.overlayDir.overlayRef.setDirection(this._dir); + } + this.overlayDir.overlayRef.updatePosition(); + + this._changeDetectorRef.detectChanges(); + this._calculateOverlayOffsetX(); + this.panel.nativeElement.scrollTop = this._scrollTop; + this._showBackdrop(); } /** Closes the overlay panel and focuses the host element. */ close(): void { if (this._panelOpen) { + this._hideBackdrop(); this._panelOpen = false; this._keyManager.withHorizontalOrientation(this._isRtl() ? 'rtl' : 'ltr'); this._changeDetectorRef.markForCheck(); @@ -662,6 +670,12 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, return this._selectionModel.selected[0].viewValue; } + /** The animation transform for overlay panel. */ + get _transformPanel() { + return this.panelOpen ? (this.multiple ? 'showing-multiple' : 'showing') : 'void'; + } + + /** Whether the element is in RTL mode. */ _isRtl(): boolean { return this._dir ? this._dir.value === 'rtl' : false; @@ -710,6 +724,9 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, event.preventDefault(); const hasDeselectedOptions = this.options.some(option => !option.selected); this.options.forEach(option => hasDeselectedOptions ? option.select() : option.deselect()); + } else if (keyCode === ESCAPE) { + event.preventDefault(); + this.close(); } else { const previouslyFocusedIndex = manager.activeItemIndex; @@ -773,6 +790,22 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, return !this._selectionModel || this._selectionModel.isEmpty(); } + /** Show the backdrop of the overlay. */ + private _showBackdrop() { + if (this.overlayDir.overlayRef.backdropElement) { + this.overlayDir.overlayRef.backdropElement.style.display = 'block'; + } else { + this.overlayDir.overlayRef.attachBackdrop(); + } + } + + /** Hide the backdrop of the overlay. */ + private _hideBackdrop() { + if (this.overlayDir.overlayRef.backdropElement) { + this.overlayDir.overlayRef.backdropElement.style.display = 'none'; + } + } + private _initializeSelection(): void { // Defer setting the value in order to avoid the "Expression // has changed after it was checked" errors from Angular.