Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(modal): keep focus trap between stack modals #3422

Merged
merged 1 commit into from Oct 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 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} 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,7 @@ import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-val
DropdownPositionComponent,
ModalFocusComponent,
ModalNestingComponent,
ModalStackComponent,
PopoverAutocloseComponent,
TooltipAutocloseComponent,
TooltipFocusComponent,
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
2 changes: 2 additions & 0 deletions e2e-app/src/app/modal/focus/modal-focus.component.html
Expand Up @@ -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>
7 changes: 7 additions & 0 deletions e2e-app/src/app/modal/focus/modal-focus.component.ts
Expand Up @@ -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);
}
}
10 changes: 10 additions & 0 deletions e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts
Expand Up @@ -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');

Expand Down
25 changes: 25 additions & 0 deletions 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>
9 changes: 9 additions & 0 deletions 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); }
}
32 changes: 32 additions & 0 deletions 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;
maxokorokov marked this conversation as resolved.
Show resolved Hide resolved

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

});

});
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()); }, 0, 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;
});
}
}
90 changes: 49 additions & 41 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,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', () => {
Expand Down