Skip to content

Commit

Permalink
fix(modal): keep focus trap between stack modals
Browse files Browse the repository at this point in the history
  • Loading branch information
fbasso committed Oct 25, 2019
1 parent d64aad4 commit 7491b2e
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 19 deletions.
4 changes: 4 additions & 0 deletions e2e-app/src/app/app.module.ts
Expand Up @@ -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, NgbdStackedModal} 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';
Expand All @@ -36,6 +37,8 @@ import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-val
DropdownPositionComponent,
ModalFocusComponent,
ModalNestingComponent,
ModalStackComponent,
NgbdStackedModal,
PopoverAutocloseComponent,
TooltipAutocloseComponent,
TooltipFocusComponent,
Expand All @@ -45,6 +48,7 @@ import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-val
TypeaheadAutoCloseComponent,
TimepickerNavigationComponent,
],
entryComponents: [NgbdStackedModal],
imports: [BrowserModule, FormsModule, ReactiveFormsModule, routing, NgbModule],
bootstrap: [AppComponent]
})
Expand Down
7 changes: 6 additions & 1 deletion e2e-app/src/app/app.routing.ts
Expand Up @@ -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 = [
Expand All @@ -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',
Expand Down
16 changes: 16 additions & 0 deletions e2e-app/src/app/modal/stack/modal-stack.component.html
@@ -0,0 +1,16 @@
<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)="openSecondModal()">Open stack modal</button>
</div>
</ng-template>

<button class="btn btn-outline-secondary ml-2" type="button" id="open-modal" (click)="openModal(t)">Open Modal</button>
33 changes: 33 additions & 0 deletions e2e-app/src/app/modal/stack/modal-stack.component.ts
@@ -0,0 +1,33 @@
import {Component, TemplateRef} from '@angular/core';
import {NgbActiveModal, NgbModal} from '@ng-bootstrap/ng-bootstrap';
import {Observable} from 'rxjs';
import {map} from 'rxjs/operators';

@Component({templateUrl: './modal-stack.component.html'})
export class ModalStackComponent {
constructor(private modalService: NgbModal) {}

openModal(content: TemplateRef<any>) { this.modalService.open(content); }

search = (text$: Observable<string>) => text$.pipe(map(() => ['one', 'two', 'three']));

openSecondModal() { this.modalService.open(NgbdStackedModal); }
}

@Component({
template: `
<div class="modal-header">
<h4 class="modal-title">Stacked modal</h4>
<button type="button" class="close" aria-label="Close" (click)="activeModal.dismiss('Cross click')">
<span aria-hidden="true">&times;</span>
</button>
</div>
<div class="modal-body"></div>
<div class="modal-footer">
<button type="button" class="btn btn-outline-dark" (click)="activeModal.close('Close click')">Close</button>
</div>
`
})
export class NgbdStackedModal {
constructor(public activeModal: NgbActiveModal) {}
}
43 changes: 43 additions & 0 deletions e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts
@@ -0,0 +1,43 @@
import {Key, browser} from 'protractor';
import {expectFocused, expectNoOpenModals, openUrl, sendKey} from '../../tools.po';
import {ModalStackPage} from './modal-stack.po';
import {DatepickerPage} from '../../datepicker/datepicker.po';
import {DropdownPage} from '../../dropdown/dropdown.po';
import {TypeaheadPage} from '../../typeahead/typeahead.po';

describe('Modal stacked', () => {
let page: ModalStackPage;
let datepickerPage: DatepickerPage;
let dropdownPage: DropdownPage;
let typeaheadPage: TypeaheadPage;

beforeAll(() => {
page = new ModalStackPage();
datepickerPage = new DatepickerPage();
dropdownPage = new DropdownPage();
typeaheadPage = new TypeaheadPage();
});

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

});

});
25 changes: 25 additions & 0 deletions 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;
}
}
4 changes: 2 additions & 2 deletions e2e-app/src/app/tools.po.ts
Expand Up @@ -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()); }, 500, message);
};

/**
Expand Down
13 changes: 8 additions & 5 deletions src/modal/modal-window.ts
Expand Up @@ -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));
}
}));
});
Expand Down Expand Up @@ -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;
});
}
}
26 changes: 15 additions & 11 deletions src/modal/modal.spec.ts
Expand Up @@ -597,6 +597,8 @@ describe('ngb-modal', () => {
expect(fixture.nativeElement).toHaveModal();

modalInstance.close();
tick();

fixture.detectChanges();
expect(fixture.nativeElement).not.toHaveModal();
}));
Expand Down Expand Up @@ -681,18 +683,20 @@ 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', () => {
Expand Down

0 comments on commit 7491b2e

Please sign in to comment.