diff --git a/e2e-app/src/app/app.module.ts b/e2e-app/src/app/app.module.ts index 69833b35eb..2b08dcc290 100644 --- a/e2e-app/src/app/app.module.ts +++ b/e2e-app/src/app/app.module.ts @@ -13,6 +13,7 @@ import {DatepickerFocusComponent} from './datepicker/focus/datepicker-focus.comp import {DropdownAutoCloseComponent} from './dropdown/autoclose/dropdown-autoclose.component'; import {DropdownFocusComponent} from './dropdown/focus/dropdown-focus.component'; import {DropdownPositionComponent} from './dropdown/position/dropdown-position.component'; +import {ModalAutoCloseComponent} from './modal/autoclose/modal-autoclose.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'; @@ -35,6 +36,7 @@ import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-val DropdownAutoCloseComponent, DropdownFocusComponent, DropdownPositionComponent, + ModalAutoCloseComponent, ModalFocusComponent, ModalNestingComponent, ModalStackComponent, diff --git a/e2e-app/src/app/app.routing.ts b/e2e-app/src/app/app.routing.ts index 3ab625bbe1..4a2f46f47d 100644 --- a/e2e-app/src/app/app.routing.ts +++ b/e2e-app/src/app/app.routing.ts @@ -5,7 +5,10 @@ import {DatepickerAutoCloseComponent} from './datepicker/autoclose/datepicker-au import {DatepickerFocusComponent} from './datepicker/focus/datepicker-focus.component'; import {DropdownAutoCloseComponent} from './dropdown/autoclose/dropdown-autoclose.component'; import {DropdownFocusComponent} from './dropdown/focus/dropdown-focus.component'; +import {ModalAutoCloseComponent} from './modal/autoclose/modal-autoclose.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'; @@ -15,8 +18,6 @@ import {TypeaheadFocusComponent} from './typeahead/focus/typeahead-focus.compone import {TimepickerNavigationComponent} from './timepicker/navigation/timepicker-navigation.component'; 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 = [ @@ -30,6 +31,7 @@ export const routes: Routes = [ { path: 'modal', children: [ + {path: 'autoclose', component: ModalAutoCloseComponent}, {path: 'focus', component: ModalFocusComponent}, {path: 'nesting', component: ModalNestingComponent}, {path: 'stack', component: ModalStackComponent}, diff --git a/e2e-app/src/app/modal/autoclose/modal-autoclose.component.html b/e2e-app/src/app/modal/autoclose/modal-autoclose.component.html new file mode 100644 index 0000000000..dea48b401c --- /dev/null +++ b/e2e-app/src/app/modal/autoclose/modal-autoclose.component.html @@ -0,0 +1,30 @@ +

+ Modal autoclose tests + {{ reason }} +

+ + + +
+

Hello from modal

+ +
+ + + +
+
+
+ +
+ + + + + +
+
+ + +

Options

+
{{ options | json }}
diff --git a/e2e-app/src/app/modal/autoclose/modal-autoclose.component.ts b/e2e-app/src/app/modal/autoclose/modal-autoclose.component.ts new file mode 100644 index 0000000000..f92ee8c794 --- /dev/null +++ b/e2e-app/src/app/modal/autoclose/modal-autoclose.component.ts @@ -0,0 +1,43 @@ +import {ChangeDetectionStrategy, ChangeDetectorRef, Component, TemplateRef} from '@angular/core'; +import {ModalDismissReasons, NgbModal, NgbModalRef} from '@ng-bootstrap/ng-bootstrap'; + +@Component({templateUrl: './modal-autoclose.component.html', changeDetection: ChangeDetectionStrategy.OnPush}) +export class ModalAutoCloseComponent { + private modalRef: NgbModalRef = null; + reason = ''; + options = {}; + + constructor(private modalService: NgbModal, private cd: ChangeDetectorRef) {} + + openModal(content?: TemplateRef) { + this.modalRef = this.modalService.open(content, this.options); + this.modalRef.result.then( + () => { + this.reason = `Closed`; + this.cd.markForCheck(); + }, + reason => { + if (reason === ModalDismissReasons.BACKDROP_CLICK) { + this.reason = 'Click'; + } else if (reason === ModalDismissReasons.ESC) { + this.reason = 'Escape'; + } else { + this.reason = 'Other'; + } + this.cd.markForCheck(); + }); + } + + closeModal() { + if (this.modalRef) { + this.modalRef.close(); + this.modalRef = null; + } + } + + reset() { + this.closeModal(); + this.reason = ''; + this.options = {}; + } +} diff --git a/e2e-app/src/app/modal/autoclose/modal-autoclose.e2e-spec.ts b/e2e-app/src/app/modal/autoclose/modal-autoclose.e2e-spec.ts new file mode 100644 index 0000000000..d9ff4fc004 --- /dev/null +++ b/e2e-app/src/app/modal/autoclose/modal-autoclose.e2e-spec.ts @@ -0,0 +1,70 @@ +import {ModalAutoClosePage} from './modal-autoclose.po'; +import {expectNoOpenModals, openUrl, sendKey} from '../../tools.po'; +import {Key} from 'protractor'; + +describe('Modal', () => { + let page: ModalAutoClosePage; + + beforeAll(() => page = new ModalAutoClosePage()); + + beforeEach(async() => { + await openUrl('modal/autoclose'); + await page.getResetButton().click(); + }); + + afterEach(async() => { await expectNoOpenModals(); }); + + it('should close modal from the inside', async() => { + const modal = await page.openModal(); + + // close + await page.getModalCloseButton().click(); + expect(await modal.isPresent()).toBeFalsy('The modal should be closed imperatively'); + expect(await page.getDismissReason()).toBe('Closed', `Modal should have been closed`); + }); + + it('should close modal on ESC', async() => { + await page.openModal(); + + // close + await sendKey(Key.ESCAPE); + expect(await page.getModal().isPresent()).toBeFalsy('The modal should be closed on ESC'); + expect(await page.getDismissReason()).toBe('Escape', `Modal should have been dismissed with 'Escape' reason`); + }); + + it(`should NOT close modal on ESC when keyboard === 'false'`, async() => { + const modal = await page.openModal('no-keyboard'); + + // close + await sendKey(Key.ESCAPE); + expect(await modal.isPresent()).toBeTruthy('The modal should stay opened on ESC'); + await page.getModalCloseButton().click(); + }); + + it('should close modal on backdrop click', async() => { + const modal = await page.openModal(); + + // close + await modal.click(); + expect(await modal.isPresent()).toBeFalsy('The modal should be closed on backdrop click'); + expect(await page.getDismissReason()).toBe('Click', `Modal should have been dismissed with 'Click' reason`); + }); + + it(`should NOT close modal on 'static' backdrop click`, async() => { + const modal = await page.openModal('backdrop-static'); + + // close + await modal.click(); + expect(await modal.isPresent()).toBeTruthy('The modal should stay opened on backdrop click'); + await page.getModalCloseButton().click(); + }); + + it(`should NOT close modal on click with no backdrop`, async() => { + const modal = await page.openModal('backdrop-false'); + + // close + await modal.click(); + expect(await modal.isPresent()).toBeTruthy('The modal should stay opened on backdrop click'); + await page.getModalCloseButton().click(); + }); +}); diff --git a/e2e-app/src/app/modal/autoclose/modal-autoclose.po.ts b/e2e-app/src/app/modal/autoclose/modal-autoclose.po.ts new file mode 100644 index 0000000000..89029044ca --- /dev/null +++ b/e2e-app/src/app/modal/autoclose/modal-autoclose.po.ts @@ -0,0 +1,23 @@ +import {$} from 'protractor'; + +export class ModalAutoClosePage { + getModal(selector = 'ngb-modal-window') { return $(selector); } + + getModalCloseButton() { return $('#modal-close-button'); } + + getDismissReason() { return $('#dismiss-reason').getText(); } + + getResetButton() { return $('#reset-button'); } + + async openModal(option = '') { + if (option !== '') { + await $(`#option-${option}`).click(); + } + + await $('#open-modal').click(); + + const modal = this.getModal(); + expect(await modal.isPresent()).toBeTruthy(`A modal should have been opened`); + return modal; + } +} 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 index 397eab2750..8f9e32b056 100644 --- a/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts +++ b/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts @@ -1,7 +1,9 @@ -import {Key} from 'protractor'; +import {$, Key} from 'protractor'; import {expectFocused, expectNoOpenModals, openUrl, sendKey} from '../../tools.po'; import {ModalStackPage} from './modal-stack.po'; +const bodyClass = () => $('body').getAttribute('class'); + describe('Modal stacked', () => { let page: ModalStackPage; @@ -14,9 +16,11 @@ describe('Modal stacked', () => { it('should keep tab on the first modal after the second modal has closed', async() => { await page.openModal(); await page.openStackModal(); + expect(await bodyClass()).toContain('modal-open', `body should have 'modal-open' class`); // close the stack modal await sendKey(Key.ESCAPE); + expect(await bodyClass()).toContain('modal-open', `body should have 'modal-open' class`); // Check that the button is focused again await expectFocused(page.getStackModalButton(), 'Button element not focused'); @@ -26,7 +30,7 @@ describe('Modal stacked', () => { // close the main modal await sendKey(Key.ESCAPE); - + expect(await bodyClass()).not.toContain('modal-open', `body should have 'modal-open' class`); }); }); diff --git a/src/modal/modal-window.spec.ts b/src/modal/modal-window.spec.ts index 52446829b7..61122245c1 100644 --- a/src/modal/modal-window.spec.ts +++ b/src/modal/modal-window.spec.ts @@ -1,9 +1,6 @@ -import {TestBed, ComponentFixture} from '@angular/core/testing'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; import {NgbModalWindow} from './modal-window'; -import {ModalDismissReasons} from './modal-dismiss-reasons'; -import {createKeyEvent} from '../test/common'; -import {Key} from '../util/key'; describe('ngb-modal-dialog', () => { @@ -50,65 +47,4 @@ describe('ngb-modal-dialog', () => { expect(dialogEl.getAttribute('role')).toBe('document'); }); }); - - describe('dismiss', () => { - - it('should dismiss on backdrop click by default', (done) => { - fixture.detectChanges(); - - fixture.componentInstance.dismissEvent.subscribe(($event) => { - expect($event).toBe(ModalDismissReasons.BACKDROP_CLICK); - done(); - }); - - fixture.nativeElement.click(); - }); - - it('should not dismiss on modal content click when there is active backdrop', (done) => { - fixture.detectChanges(); - fixture.componentInstance.dismissEvent.subscribe( - () => { done.fail(new Error('Should not trigger dismiss event')); }); - - fixture.nativeElement.querySelector('.modal-content').click(); - setTimeout(done, 200); - }); - - it('should ignore backdrop clicks when there is no backdrop', (done) => { - fixture.componentInstance.backdrop = false; - fixture.detectChanges(); - - fixture.componentInstance.dismissEvent.subscribe(($event) => { - expect($event).toBe(ModalDismissReasons.BACKDROP_CLICK); - done.fail(new Error('Should not trigger dismiss event')); - }); - - fixture.nativeElement.querySelector('.modal-dialog').click(); - setTimeout(done, 200); - }); - - it('should ignore backdrop clicks when backdrop is "static"', (done) => { - fixture.componentInstance.backdrop = 'static'; - fixture.detectChanges(); - - fixture.componentInstance.dismissEvent.subscribe(($event) => { - expect($event).toBe(ModalDismissReasons.BACKDROP_CLICK); - done.fail(new Error('Should not trigger dismiss event')); - }); - - fixture.nativeElement.querySelector('.modal-dialog').click(); - setTimeout(done, 200); - }); - - it('should dismiss on esc press by default', (done) => { - fixture.detectChanges(); - - fixture.componentInstance.dismissEvent.subscribe(($event) => { - expect($event).toBe(ModalDismissReasons.ESC); - done(); - }); - - fixture.nativeElement.dispatchEvent(createKeyEvent(Key.Escape, {type: 'keydown'})); - }); - }); - }); diff --git a/src/modal/modal-window.ts b/src/modal/modal-window.ts index 59a57f5187..3554383d0a 100644 --- a/src/modal/modal-window.ts +++ b/src/modal/modal-window.ts @@ -13,7 +13,7 @@ import { ViewEncapsulation } from '@angular/core'; import {fromEvent} from 'rxjs'; -import {filter, takeUntil} from 'rxjs/operators'; +import {filter, map, takeUntil, withLatestFrom} from 'rxjs/operators'; import {getFocusableBoundaryElements} from '../util/focus-trap'; import {Key} from '../util/key'; @@ -25,7 +25,6 @@ import {ModalDismissReasons} from './modal-dismiss-reasons'; '[class]': '"modal fade show d-block" + (windowClass ? " " + windowClass : "")', 'role': 'dialog', 'tabindex': '-1', - '(click)': 'backdropClick($event)', '[attr.aria-modal]': 'true', '[attr.aria-labelledby]': 'ariaLabelledBy', }, @@ -65,13 +64,16 @@ export class NgbModalWindow implements OnInit, _zone.run(() => this.dismiss(ModalDismissReasons.ESC)); } })); - }); - } - backdropClick(event: MouseEvent): void { - if (this.backdrop === true && this._elRef.nativeElement === event.target) { - this.dismiss(ModalDismissReasons.BACKDROP_CLICK); - } + const mouseDowns$ = fromEvent(this._elRef.nativeElement, 'mousedown') + .pipe( + takeUntil(this.dismissEvent), + map(e => this.backdrop === true && this._elRef.nativeElement === e.target)); + + fromEvent(this._elRef.nativeElement, 'mouseup') + .pipe(takeUntil(this.dismissEvent), withLatestFrom(mouseDowns$), filter(([_, shouldClose]) => shouldClose)) + .subscribe(() => this._zone.run(() => this.dismiss(ModalDismissReasons.BACKDROP_CLICK))); + }); } dismiss(reason): void { this.dismissEvent.emit(reason); } diff --git a/src/modal/modal.spec.ts b/src/modal/modal.spec.ts index 88787907f5..dbe9ab50a6 100644 --- a/src/modal/modal.spec.ts +++ b/src/modal/modal.spec.ts @@ -1,19 +1,8 @@ import {CommonModule} from '@angular/common'; -import { - Component, - DebugElement, - getDebugNode, - Injectable, - Injector, - NgModule, - OnDestroy, - ViewChild -} from '@angular/core'; -import {async, ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; +import {Component, Injectable, Injector, NgModule, OnDestroy, ViewChild} from '@angular/core'; +import {async, ComponentFixture, TestBed} from '@angular/core/testing'; import {NgbModalConfig} from './modal-config'; import {NgbActiveModal, NgbModal, NgbModalModule, NgbModalRef} from './modal.module'; -import {createKeyEvent} from '../test/common'; -import {Key} from '../util/key'; const NOOP = () => {}; @@ -320,53 +309,6 @@ describe('ngb-modal', () => { })); }); - describe('stacked modals', () => { - - it('should not remove "modal-open" class on body when closed modal is not last', async(() => { - const modalRef1 = fixture.componentInstance.open('foo'); - const modalRef2 = fixture.componentInstance.open('bar'); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal(); - expect(document.body).toHaveCssClass('modal-open'); - - modalRef1.close('foo result'); - fixture.detectChanges(); - fixture.whenStable().then(() => { - expect(fixture.nativeElement).toHaveModal(); - expect(document.body).toHaveCssClass('modal-open'); - - modalRef2.close('bar result'); - fixture.detectChanges(); - fixture.whenStable().then(() => { - expect(fixture.nativeElement).not.toHaveModal(); - expect(document.body).not.toHaveCssClass('modal-open'); - }); - }); - })); - - it('should dismiss modals on ESC in correct order', fakeAsync(() => { - fixture.componentInstance.open('foo').result.catch(NOOP); - fixture.componentInstance.open('bar').result.catch(NOOP); - const ngbModalWindow1 = document.querySelectorAll('ngb-modal-window')[0]; - const ngbModalWindow2 = document.querySelectorAll('ngb-modal-window')[1]; - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal(['foo', 'bar']); - expect(document.activeElement).toBe(ngbModalWindow2); - - ngbModalWindow2.dispatchEvent(createKeyEvent(Key.Escape, {type: 'keydown'})); - tick(16); // RAF in escape handling - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal(['foo']); - expect(document.activeElement).toBe(ngbModalWindow1); - - ngbModalWindow1.dispatchEvent(createKeyEvent(Key.Escape, {type: 'keydown'})); - tick(16); // RAF in escape handling - fixture.detectChanges(); - expect(fixture.nativeElement).not.toHaveModal(); - expect(document.activeElement).toBe(document.body); - })); - }); - describe('backdrop options', () => { it('should have backdrop by default', () => { @@ -411,52 +353,6 @@ describe('ngb-modal', () => { expect(fixture.nativeElement).not.toHaveBackdrop(); }); - it('should dismiss on backdrop click', () => { - fixture.componentInstance.open('foo').result.catch(NOOP); - fixture.detectChanges(); - - expect(fixture.nativeElement).toHaveModal('foo'); - expect(fixture.nativeElement).toHaveBackdrop(); - - (document.querySelector('ngb-modal-window')).click(); - fixture.detectChanges(); - - expect(fixture.nativeElement).not.toHaveModal(); - expect(fixture.nativeElement).not.toHaveBackdrop(); - }); - - it('should not dismiss on "static" backdrop click', () => { - const modalInstance = fixture.componentInstance.open('foo', {backdrop: 'static'}); - fixture.detectChanges(); - - expect(fixture.nativeElement).toHaveModal('foo'); - expect(fixture.nativeElement).toHaveBackdrop(); - - (document.querySelector('ngb-modal-window')).click(); - fixture.detectChanges(); - - expect(fixture.nativeElement).toHaveModal(); - expect(fixture.nativeElement).toHaveBackdrop(); - - modalInstance.close(); - fixture.detectChanges(); - expect(fixture.nativeElement).not.toHaveModal(); - }); - - it('should not dismiss on clicks outside content where there is no backdrop', () => { - const modalInstance = fixture.componentInstance.open('foo', {backdrop: false}); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('foo'); - - (document.querySelector('ngb-modal-window')).click(); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal(); - - modalInstance.close(); - fixture.detectChanges(); - expect(fixture.nativeElement).not.toHaveModal(); - }); - it('should not dismiss on clicks that result in detached elements', () => { const modalInstance = fixture.componentInstance.openTplIf({}); fixture.detectChanges(); @@ -573,53 +469,6 @@ describe('ngb-modal', () => { }); }); - describe('keyboard options', () => { - - it('should dismiss modals on ESC by default', fakeAsync(() => { - fixture.componentInstance.open('foo').result.catch(NOOP); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('foo'); - - document.querySelector('ngb-modal-window').dispatchEvent(createKeyEvent(Key.Escape, {type: 'keydown'})); - tick(16); // RAF in escape handling - fixture.detectChanges(); - expect(fixture.nativeElement).not.toHaveModal(); - })); - - it('should not dismiss modals on ESC when keyboard option is false', fakeAsync(() => { - const modalInstance = fixture.componentInstance.open('foo', {keyboard: false}); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('foo'); - - document.querySelector('ngb-modal-window').dispatchEvent(createKeyEvent(Key.Escape, {type: 'keydown'})); - tick(16); // RAF in escape handling - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal(); - - modalInstance.close(); - tick(); - - fixture.detectChanges(); - expect(fixture.nativeElement).not.toHaveModal(); - })); - - it('should not dismiss modals on ESC when default is prevented', () => { - const modalInstance = fixture.componentInstance.open('foo', {keyboard: true}); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('foo'); - - (getDebugNode(document.querySelector('ngb-modal-window'))).triggerEventHandler('keyup.esc', { - defaultPrevented: true - }); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal(); - - modalInstance.close(); - fixture.detectChanges(); - expect(fixture.nativeElement).not.toHaveModal(); - }); - }); - describe('size options', () => { it('should render modals with specified size', () => { @@ -683,64 +532,6 @@ describe('ngb-modal', () => { describe('focus management', () => { - 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(); - - tick(); - expect(document.activeElement).toBe(openButtonEl); - })); - - - 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')); - - modalInstance.close('ok!'); - tick(); - expect(document.activeElement).toBe(document.body); - })); - - 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'); - - openElement.click(); - fixture.detectChanges(); - expect(fixture.nativeElement).toHaveModal('from non focusable element'); - expect(document.activeElement).toBe(document.querySelector('ngb-modal-window')); - - fixture.componentInstance.close(); - tick(); - expect(fixture.nativeElement).not.toHaveModal(); - expect(document.activeElement).toBe(document.body); - })); - - 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'); - - 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', () => { fixture.detectChanges();