fix(modal): trap focus correctly with stacked modals (#3422)

Fixes #3392

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,

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',

e2e-app/src/app/modal/focus/modal-focus.component.html

@@ -45,3 +45,5 @@ <h4 class="modal-title">Autofocus modal</h4>
 </ng-template>
 
 <button class="btn btn-outline-secondary ml-2" type="button" id="open-modal-autofocus" (click)="openModal(autofocus)">Autofocus Modal</button>
+
+<button class="btn btn-outline-secondary ml-2" type="button" id="open-modal-disable" (click)="openAndDisable()" [disabled]="disabledButton">Open and disabled</button>

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<any>) { this.modalService.open(content ? content : 'Modal content'); }
+
+  openAndDisable(content?: TemplateRef<any>) {
+    this.disabledButton = true;
+    this.openModal(content);
+  }
 }

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');
 

e2e-app/src/app/modal/stack/modal-stack.component.html

@@ -0,0 +1,25 @@
+<h3>Modal nesting tests</h3>
+
+<ng-template #t let-modal>
+  <div class="modal-header">
+    <h4 class="modal-title">Modal with nested popups</h4>
+    <button type="button" class="close" aria-label="Close" (click)="modal.dismiss()">
+      <span aria-hidden="true">&times;</span>
+    </button>
+  </div>
+  <div class="modal-body">
+    <!-- modal -->
+    <button id="open-inner-modal" class="btn btn-lg btn-outline-primary" (click)="openModal(t2)">Open stack modal</button>
+  </div>
+</ng-template>
+
+<ng-template #t2 let-modal>
+  <div class="modal-header">
+    <h4 class="modal-title">Stacked modal</h4>
+    <button type="button" class="close" aria-label="Close" (click)="modal.dismiss()">
+      <span aria-hidden="true">&times;</span>
+    </button>
+  </div>
+</ng-template>
+
+<button class="btn btn-outline-secondary ml-2" type="button" id="open-modal" (click)="openModal(t)">Open Modal</button>

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<any>) { this.modalService.open(content); }
+}

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);
+
+  });
+
+});

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;
+  }
+}

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);
 };
 
 /**

src/modal/modal-window.ts

@@ -52,16 +52,17 @@ export class NgbModalWindow implements OnInit,
 
   @Output('dismiss') dismissEvent = new EventEmitter();
 
-  constructor(@Inject(DOCUMENT) private _document: any, private _elRef: ElementRef<HTMLElement>, zone: NgZone) {
-    zone.runOutsideAngular(() => {
+  constructor(
+      @Inject(DOCUMENT) private _document: any, private _elRef: ElementRef<HTMLElement>, private _zone: NgZone) {
+    _zone.runOutsideAngular(() => {
       fromEvent<KeyboardEvent>(this._elRef.nativeElement, 'keyup')
           .pipe(
               takeUntil(this.dismissEvent),
               // tslint:disable-next-line:deprecation
               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;
+    });
   }
 }

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', () => {