diff --git a/e2e-app/src/app/app.module.ts b/e2e-app/src/app/app.module.ts index 2d34155436..69833b35eb 100644 --- a/e2e-app/src/app/app.module.ts +++ b/e2e-app/src/app/app.module.ts @@ -15,6 +15,7 @@ import {DropdownFocusComponent} from './dropdown/focus/dropdown-focus.component' import {DropdownPositionComponent} from './dropdown/position/dropdown-position.component'; import {ModalFocusComponent} from './modal/focus/modal-focus.component'; import {ModalNestingComponent} from './modal/nesting/modal-nesting.component'; +import {ModalStackComponent} from './modal/stack/modal-stack.component'; import {PopoverAutocloseComponent} from './popover/autoclose/popover-autoclose.component'; import {TooltipAutocloseComponent} from './tooltip/autoclose/tooltip-autoclose.component'; import {TooltipFocusComponent} from './tooltip/focus/tooltip-focus.component'; @@ -36,6 +37,7 @@ import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-val DropdownPositionComponent, ModalFocusComponent, ModalNestingComponent, + ModalStackComponent, PopoverAutocloseComponent, TooltipAutocloseComponent, TooltipFocusComponent, diff --git a/e2e-app/src/app/app.routing.ts b/e2e-app/src/app/app.routing.ts index 3a4840748d..3ab625bbe1 100644 --- a/e2e-app/src/app/app.routing.ts +++ b/e2e-app/src/app/app.routing.ts @@ -16,6 +16,7 @@ import {TimepickerNavigationComponent} from './timepicker/navigation/timepicker- import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-validation.component'; import {DropdownPositionComponent} from './dropdown/position/dropdown-position.component'; import {ModalNestingComponent} from './modal/nesting/modal-nesting.component'; +import {ModalStackComponent} from './modal/stack/modal-stack.component'; export const routes: Routes = [ @@ -28,7 +29,11 @@ export const routes: Routes = [ }, { path: 'modal', - children: [{path: 'focus', component: ModalFocusComponent}, {path: 'nesting', component: ModalNestingComponent}] + children: [ + {path: 'focus', component: ModalFocusComponent}, + {path: 'nesting', component: ModalNestingComponent}, + {path: 'stack', component: ModalStackComponent}, + ] }, { path: 'dropdown', diff --git a/e2e-app/src/app/modal/focus/modal-focus.component.html b/e2e-app/src/app/modal/focus/modal-focus.component.html index c981852474..3f1313912a 100644 --- a/e2e-app/src/app/modal/focus/modal-focus.component.html +++ b/e2e-app/src/app/modal/focus/modal-focus.component.html @@ -45,3 +45,5 @@ + + diff --git a/e2e-app/src/app/modal/focus/modal-focus.component.ts b/e2e-app/src/app/modal/focus/modal-focus.component.ts index d523bbf3e2..495330c874 100644 --- a/e2e-app/src/app/modal/focus/modal-focus.component.ts +++ b/e2e-app/src/app/modal/focus/modal-focus.component.ts @@ -3,7 +3,14 @@ import {NgbModal} from '@ng-bootstrap/ng-bootstrap'; @Component({templateUrl: './modal-focus.component.html'}) export class ModalFocusComponent { + disabledButton = false; + constructor(private modalService: NgbModal) {} openModal(content?: TemplateRef) { this.modalService.open(content ? content : 'Modal content'); } + + openAndDisable(content?: TemplateRef) { + this.disabledButton = true; + this.openModal(content); + } } diff --git a/e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts b/e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts index f76e0fa8d6..4356255daf 100644 --- a/e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts +++ b/e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts @@ -36,6 +36,16 @@ describe('Modal', () => { await expectFocused($('#open-modal-simple'), 'Should focus trigger button after closing'); }); + it('should focus body if opener is not focusable', async() => { + await page.openModal('disable'); + + // close + await sendKey(Key.ESCAPE); + + // body should be focused + await expectFocused($('body'), 'Should focus body after closing'); + }); + it('should focus modal window if there is no focusable content after opening', async() => { const modal = await page.openModal('simple'); diff --git a/e2e-app/src/app/modal/stack/modal-stack.component.html b/e2e-app/src/app/modal/stack/modal-stack.component.html new file mode 100644 index 0000000000..59662d9336 --- /dev/null +++ b/e2e-app/src/app/modal/stack/modal-stack.component.html @@ -0,0 +1,25 @@ +

Modal nesting tests

+ + + + + + + + + + + diff --git a/e2e-app/src/app/modal/stack/modal-stack.component.ts b/e2e-app/src/app/modal/stack/modal-stack.component.ts new file mode 100644 index 0000000000..a20d3e7809 --- /dev/null +++ b/e2e-app/src/app/modal/stack/modal-stack.component.ts @@ -0,0 +1,9 @@ +import {Component, TemplateRef} from '@angular/core'; +import {NgbModal} from '@ng-bootstrap/ng-bootstrap'; + +@Component({templateUrl: './modal-stack.component.html'}) +export class ModalStackComponent { + constructor(private modalService: NgbModal) {} + + openModal(content: TemplateRef) { this.modalService.open(content); } +} diff --git a/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts b/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts new file mode 100644 index 0000000000..397eab2750 --- /dev/null +++ b/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts @@ -0,0 +1,32 @@ +import {Key} from 'protractor'; +import {expectFocused, expectNoOpenModals, openUrl, sendKey} from '../../tools.po'; +import {ModalStackPage} from './modal-stack.po'; + +describe('Modal stacked', () => { + let page: ModalStackPage; + + beforeAll(() => { page = new ModalStackPage(); }); + + beforeEach(async() => await openUrl('modal/stack')); + + afterEach(async() => { await expectNoOpenModals(); }); + + it('should keep tab on the first modal after the second modal has closed', async() => { + await page.openModal(); + await page.openStackModal(); + + // close the stack modal + await sendKey(Key.ESCAPE); + + // Check that the button is focused again + await expectFocused(page.getStackModalButton(), 'Button element not focused'); + await sendKey(Key.TAB); + + await expectFocused(page.getCoseIcon(), 'Close icon not focused'); + + // close the main modal + await sendKey(Key.ESCAPE); + + }); + +}); diff --git a/e2e-app/src/app/modal/stack/modal-stack.po.ts b/e2e-app/src/app/modal/stack/modal-stack.po.ts new file mode 100644 index 0000000000..2227bcd5f2 --- /dev/null +++ b/e2e-app/src/app/modal/stack/modal-stack.po.ts @@ -0,0 +1,25 @@ +import {$, $$} from 'protractor'; + +export class ModalStackPage { + getModal(index) { return $$('ngb-modal-window').get(index); } + + getModalButton() { return $('#open-modal'); } + + getStackModalButton() { return $('#open-inner-modal'); } + + getCoseIcon() { return $('button.close'); } + + async openModal() { + await this.getModalButton().click(); + const modal = this.getModal(0); + expect(await modal.isPresent()).toBeTruthy(`A modal should have been opened`); + return modal; + } + + async openStackModal() { + await this.getStackModalButton().click(); + const modal = this.getModal(1); + expect(await modal.isPresent()).toBeTruthy(`A second modal should have been opened`); + return modal; + } +} diff --git a/e2e-app/src/app/tools.po.ts b/e2e-app/src/app/tools.po.ts index e038591ade..ac28ace456 100644 --- a/e2e-app/src/app/tools.po.ts +++ b/e2e-app/src/app/tools.po.ts @@ -34,8 +34,8 @@ export const offsetClick = async(el: ElementFinder, offset) => { * @param message to display in case of error */ export const expectFocused = async(el: ElementFinder, message: string) => { - const focused = await browser.driver.switchTo().activeElement(); - expect(await WebElement.equals(el.getWebElement(), focused)).toBeTruthy(message); + await browser.wait( + () => { return WebElement.equals(el.getWebElement(), browser.driver.switchTo().activeElement()); }, 0, message); }; /** diff --git a/src/modal/modal-window.ts b/src/modal/modal-window.ts index ef9bf3947f..d1106e940c 100644 --- a/src/modal/modal-window.ts +++ b/src/modal/modal-window.ts @@ -52,8 +52,9 @@ export class NgbModalWindow implements OnInit, @Output('dismiss') dismissEvent = new EventEmitter(); - constructor(@Inject(DOCUMENT) private _document: any, private _elRef: ElementRef, zone: NgZone) { - zone.runOutsideAngular(() => { + constructor( + @Inject(DOCUMENT) private _document: any, private _elRef: ElementRef, private _zone: NgZone) { + _zone.runOutsideAngular(() => { fromEvent(this._elRef.nativeElement, 'keyup') .pipe( takeUntil(this.dismissEvent), @@ -61,7 +62,7 @@ export class NgbModalWindow implements OnInit, filter(e => e.which === Key.Escape && this.keyboard)) .subscribe(event => requestAnimationFrame(() => { if (!event.defaultPrevented) { - zone.run(() => this.dismiss(ModalDismissReasons.ESC)); + _zone.run(() => this.dismiss(ModalDismissReasons.ESC)); } })); }); @@ -97,7 +98,9 @@ export class NgbModalWindow implements OnInit, } else { elementToFocus = body; } - elementToFocus.focus(); - this._elWithFocus = null; + this._zone.runOutsideAngular(() => { + setTimeout(() => elementToFocus.focus()); + this._elWithFocus = null; + }); } } diff --git a/src/modal/modal.spec.ts b/src/modal/modal.spec.ts index bce49e5caa..6d863ad46b 100644 --- a/src/modal/modal.spec.ts +++ b/src/modal/modal.spec.ts @@ -597,6 +597,8 @@ describe('ngb-modal', () => { expect(fixture.nativeElement).toHaveModal(); modalInstance.close(); + tick(); + fixture.detectChanges(); expect(fixture.nativeElement).not.toHaveModal(); })); @@ -681,57 +683,63 @@ describe('ngb-modal', () => { describe('focus management', () => { - it('should return focus to previously focused element', () => { - fixture.detectChanges(); - const openButtonEl = fixture.nativeElement.querySelector('button#open'); - openButtonEl.focus(); - openButtonEl.click(); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('from button'); + it('should return focus to previously focused element', fakeAsync(() => { + fixture.detectChanges(); + const openButtonEl = fixture.nativeElement.querySelector('button#open'); + openButtonEl.focus(); + openButtonEl.click(); + fixture.detectChanges(); + expect(fixture.nativeElement).toHaveModal('from button'); - fixture.componentInstance.close(); - expect(fixture.nativeElement).not.toHaveModal(); - expect(document.activeElement).toBe(openButtonEl); - }); + fixture.componentInstance.close(); + expect(fixture.nativeElement).not.toHaveModal(); + tick(); + expect(document.activeElement).toBe(openButtonEl); + })); - it('should return focus to body if no element focused prior to modal opening', () => { - const modalInstance = fixture.componentInstance.open('foo'); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('foo'); - expect(document.activeElement).toBe(document.querySelector('ngb-modal-window')); - modalInstance.close('ok!'); - expect(document.activeElement).toBe(document.body); - }); + it('should return focus to body if no element focused prior to modal opening', fakeAsync(() => { + const modalInstance = fixture.componentInstance.open('foo'); + fixture.detectChanges(); + expect(fixture.nativeElement).toHaveModal('foo'); + expect(document.activeElement).toBe(document.querySelector('ngb-modal-window')); - it('should return focus to body if the opening element is not stored as previously focused element', () => { - fixture.detectChanges(); - const openElement = fixture.nativeElement.querySelector('#open-no-focus'); + modalInstance.close('ok!'); + tick(); + expect(document.activeElement).toBe(document.body); + })); - openElement.click(); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('from non focusable element'); - expect(document.activeElement).toBe(document.querySelector('ngb-modal-window')); + it('should return focus to body if the opening element is not stored as previously focused element', + fakeAsync(() => { + fixture.detectChanges(); + const openElement = fixture.nativeElement.querySelector('#open-no-focus'); - fixture.componentInstance.close(); - expect(fixture.nativeElement).not.toHaveModal(); - expect(document.activeElement).toBe(document.body); - }); + openElement.click(); + fixture.detectChanges(); + expect(fixture.nativeElement).toHaveModal('from non focusable element'); + expect(document.activeElement).toBe(document.querySelector('ngb-modal-window')); - it('should return focus to body if the opening element is stored but cannot be focused', () => { - fixture.detectChanges(); - const openElement = fixture.nativeElement.querySelector('#open-no-focus-ie'); + fixture.componentInstance.close(); + tick(); + expect(fixture.nativeElement).not.toHaveModal(); + expect(document.activeElement).toBe(document.body); + })); - openElement.click(); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('from non focusable element but stored as activeElement on IE'); - expect(document.activeElement).toBe(document.querySelector('ngb-modal-window')); + it('should return focus to body if the opening element is stored but cannot be focused', fakeAsync(() => { + fixture.detectChanges(); + const openElement = fixture.nativeElement.querySelector('#open-no-focus-ie'); - fixture.componentInstance.close(); - expect(fixture.nativeElement).not.toHaveModal(); - expect(document.activeElement).toBe(document.body); - }); + openElement.click(); + fixture.detectChanges(); + expect(fixture.nativeElement).toHaveModal('from non focusable element but stored as activeElement on IE'); + expect(document.activeElement).toBe(document.querySelector('ngb-modal-window')); + + fixture.componentInstance.close(); + tick(); + expect(fixture.nativeElement).not.toHaveModal(); + expect(document.activeElement).toBe(document.body); + })); describe('initial focus', () => { it('should focus the proper specified element when [ngbAutofocus] is used', () => {