Dialog Mouse Event (#389)

* Fixed bug for the dialog, where a mouse up event closes the dialog and results in unexpected UX behaviour;
Update and fixed README.md;

* Forgot to change the ux-dialog-renderer, my bad...

* Fixed some failing tests which only happens when running the following scripts:
- npm run test
- npm run cut-release

If the run script 'npm run test:watch' is called this doesn't happen, really weird.

* Added mouseEventType setting in the dialog-setting file, so the click handling is now configurable.
Added MouseEventType typing with the following values: 'click' | 'mouseup' | 'mousedown'
Update the setup and clear click handling in the native-dialog-renderer and ux-dialog-renderer, so it uses the new setting property 'mouseEventType', default fallback is 'click' so this is certainly not breaking;

* Renamed mouseEventType to mouseEvent to have the same symmetry.
Renamed setup and clearClickHandling to setup and clearEventHandling, to be consistent with the native-diaglog-renderer file.
Fixed tslint error for the null value assignment on a zIndex string. Assign default/initial value 'auto' when no value is provided via the settings;

* Update typos in the code documentation;

* Attempt to fix the flaky tests

Co-authored-by: Marco Knol <[email protected]>

README.md

@@ -37,7 +37,13 @@ npm run test
 ## Developing
 
 ```shell
-npm run test-watch
+npm run test:watch
+```
+
+## Debugging
+
+```shell
+npm run test:debugging
 ```
 
 ## Publishing

src/dialog-settings.ts

@@ -1,8 +1,10 @@
 import { Container } from 'aurelia-dependency-injection';
+
 import { ViewStrategy } from 'aurelia-templating';
 
 export type ActionKey = 'Escape' | 'Enter';
 export type KeyEventType = 'keyup' | 'keydown';
+export type MouseEventType = 'click' | 'mouseup' | 'mousedown';
 
 /**
  * All available dialog settings.
@@ -54,13 +56,20 @@ export interface DialogSettings {
   keyboard?: boolean | ActionKey | ActionKey[];
 
   /**
-   * Determines which type of keyevent should be used to listen for
+   * Determines which type of key event should be used to listen for
    * ENTER and ESC keys
    *
    * Default: keyup
    */
   keyEvent?: KeyEventType;
 
+  /**
+   * Determines which type of mouse event should be used for closing the dialog
+   *
+   * Default: click
+   */
+  mouseEvent?: MouseEventType;
+
   /**
    * When set to "true" allows for the dismissal of the dialog by clicking outside of it.
    */

src/renderers/native-dialog-renderer.ts

@@ -1,7 +1,11 @@
 import { DOM } from 'aurelia-pal';
+
 import { transient } from 'aurelia-dependency-injection';
-import { Renderer } from '../renderer';
+
 import { DialogController } from '../dialog-controller';
+import { MouseEventType } from '../dialog-settings';
+import { Renderer } from '../renderer';
+
 import { transitionEvent, hasTransition } from './ux-dialog-renderer';
 
 const containerTagName = 'dialog';
@@ -128,15 +132,17 @@ export class NativeDialogRenderer implements Renderer {
         e.preventDefault();
       }
     };
-    this.dialogContainer.addEventListener('click', this.closeDialogClick);
+    const mouseEvent: MouseEventType = dialogController.settings.mouseEvent || 'click';
+    this.dialogContainer.addEventListener(mouseEvent, this.closeDialogClick);
     this.dialogContainer.addEventListener('cancel', this.dialogCancel);
-    this.anchor.addEventListener('click', this.stopPropagation);
+    this.anchor.addEventListener(mouseEvent, this.stopPropagation);
   }
 
-  private clearEventHandling(): void {
-    this.dialogContainer.removeEventListener('click', this.closeDialogClick);
+  private clearEventHandling(dialogController: DialogController): void {
+    const mouseEvent: MouseEventType = dialogController.settings.mouseEvent || 'click';
+    this.dialogContainer.removeEventListener(mouseEvent, this.closeDialogClick);
     this.dialogContainer.removeEventListener('cancel', this.dialogCancel);
-    this.anchor.removeEventListener('click', this.stopPropagation);
+    this.anchor.removeEventListener(mouseEvent, this.stopPropagation);
   }
 
   private awaitTransition(setActiveInactive: () => void, ignore: boolean): Promise<void> {
@@ -187,7 +193,7 @@ export class NativeDialogRenderer implements Renderer {
   }
 
   public hideDialog(dialogController: DialogController): Promise<void> {
-    this.clearEventHandling();
+    this.clearEventHandling(dialogController);
     NativeDialogRenderer.untrackController(dialogController);
     return this.awaitTransition(() => this.setAsInactive(), dialogController.settings.ignoreTransitions as boolean)
       .then(() => { this.detach(dialogController); });

src/renderers/ux-dialog-renderer.ts

@@ -1,8 +1,10 @@
 import { DOM } from 'aurelia-pal';
+
 import { transient } from 'aurelia-dependency-injection';
-import { ActionKey } from '../dialog-settings';
-import { Renderer } from '../renderer';
+
 import { DialogController } from '../dialog-controller';
+import { ActionKey, MouseEventType } from '../dialog-settings';
+import { Renderer } from '../renderer';
 
 const containerTagName = 'ux-dialog-container';
 const overlayTagName = 'ux-dialog-overlay';
@@ -134,7 +136,7 @@ export class DialogRenderer implements Renderer {
     const dialogOverlay = this.dialogOverlay = DOM.createElement(overlayTagName) as HTMLElement;
     const zIndex = typeof dialogController.settings.startingZIndex === 'number'
       ? dialogController.settings.startingZIndex + ''
-      : null;
+      : 'auto'; // default/initial zIndex value is auto
 
     dialogOverlay.style.zIndex = zIndex;
     dialogContainer.style.zIndex = zIndex;
@@ -176,20 +178,22 @@ export class DialogRenderer implements Renderer {
     this.dialogContainer.classList.remove('active');
   }
 
-  private setupClickHandling(dialogController: DialogController): void {
+  private setupEventHandling(dialogController: DialogController): void {
     this.stopPropagation = e => { e._aureliaDialogHostClicked = true; };
     this.closeDialogClick = e => {
       if (dialogController.settings.overlayDismiss && !e._aureliaDialogHostClicked) {
         dialogController.cancel();
       }
     };
-    this.dialogContainer.addEventListener('click', this.closeDialogClick);
-    this.anchor.addEventListener('click', this.stopPropagation);
+    const mouseEvent: MouseEventType = dialogController.settings.mouseEvent || 'click';
+    this.dialogContainer.addEventListener(mouseEvent, this.closeDialogClick);
+    this.anchor.addEventListener(mouseEvent, this.stopPropagation);
   }
 
-  private clearClickHandling(): void {
-    this.dialogContainer.removeEventListener('click', this.closeDialogClick);
-    this.anchor.removeEventListener('click', this.stopPropagation);
+  private clearEventHandling(dialogController: DialogController): void {
+    const mouseEvent: MouseEventType = dialogController.settings.mouseEvent || 'click';
+    this.dialogContainer.removeEventListener(mouseEvent, this.closeDialogClick);
+    this.anchor.removeEventListener(mouseEvent, this.stopPropagation);
   }
 
   private centerDialog() {
@@ -244,12 +248,12 @@ export class DialogRenderer implements Renderer {
     }
 
     DialogRenderer.trackController(dialogController);
-    this.setupClickHandling(dialogController);
+    this.setupEventHandling(dialogController);
     return this.awaitTransition(() => this.setAsActive(), dialogController.settings.ignoreTransitions as boolean);
   }
 
   public hideDialog(dialogController: DialogController) {
-    this.clearClickHandling();
+    this.clearEventHandling(dialogController);
     DialogRenderer.untrackController(dialogController);
     return this.awaitTransition(() => this.setAsInactive(), dialogController.settings.ignoreTransitions as boolean)
       .then(() => { this.detach(dialogController); });

test/unit/aurelia-dialog.spec.ts

@@ -10,6 +10,18 @@ describe('testing aurelia configure routine', () => {
     globalResources() { return; },
     transient() { return; }
   } as any;
+  let userDefaultsSpy: jasmine.Spy;
+  let applySpy: jasmine.Spy;
+
+  beforeEach(() => {
+    userDefaultsSpy = spyOn(DialogConfiguration.prototype, 'useDefaults').and.callThrough();
+    applySpy = spyOn(DialogConfiguration.prototype as any, '_apply');
+  });
+
+  afterEach(() => {
+    userDefaultsSpy.calls.reset();
+    applySpy.calls.reset();
+  });
 
   it('should export configure function', () => {
     expect(typeof configure).toBe('function');
@@ -28,20 +40,17 @@ describe('testing aurelia configure routine', () => {
   });
 
   it('should apply the defaults when no setup callback is supplied', () => {
-    spyOn(DialogConfiguration.prototype, 'useDefaults').and.callThrough();
     configure(frameworkConfig as any);
-    expect(DialogConfiguration.prototype.useDefaults).toHaveBeenCalled();
+    expect(userDefaultsSpy).toHaveBeenCalled();
   });
 
   it('should apply the configurations when setup callback is provided', () => {
-    spyOn(DialogConfiguration.prototype as any, '_apply');
     configure(frameworkConfig, () => { return; });
-    expect((DialogConfiguration.prototype as any)._apply).toHaveBeenCalled();
+    expect(applySpy).toHaveBeenCalled();
   });
 
   it('should apply the configurations when no setup callback is provided', () => {
-    spyOn(DialogConfiguration.prototype as any, '_apply');
     configure(frameworkConfig);
-    expect((DialogConfiguration.prototype as any)._apply).toHaveBeenCalled();
+    expect(applySpy).toHaveBeenCalled();
   });
 });

test/unit/native-dialog-renderer.spec.ts

@@ -1,9 +1,10 @@
-import '../setup';
 import { DOM } from 'aurelia-pal';
+
+import '../setup';
 import { DialogController } from '../../src/dialog-controller';
+import { DefaultDialogSettings, DialogSettings } from '../../src/dialog-settings';
 import { NativeDialogRenderer } from '../../src/renderers/native-dialog-renderer';
 import { hasTransition, transitionEvent } from '../../src/renderers/ux-dialog-renderer';
-import { DefaultDialogSettings, DialogSettings } from '../../src/dialog-settings';
 
 type TestDialogRenderer = NativeDialogRenderer & { [key: string]: any, __controller: DialogController };
 
@@ -350,7 +351,7 @@ describe('native-dialog-renderer.spec.ts', () => {
     });
   });
 
-  describe('"backdropDismiss" handlers', () => {
+  describe('"backdropDismiss" handlers as default', () => {
     it('do not stop events propagation', async done => {
       const renderer = createRenderer();
       const event = new MouseEvent('click');
@@ -373,6 +374,30 @@ describe('native-dialog-renderer.spec.ts', () => {
       done();
     });
   });
+
+  describe('"backdropDismiss" handlers with custom mouseEvent setting set', () => {
+    it('do not stop events propagation', async done => {
+      const renderer = createRenderer({mouseEvent: 'mousedown'});
+      const event = new MouseEvent('mousedown');
+      spyOn(event, 'stopPropagation').and.callThrough();
+      spyOn(event, 'stopImmediatePropagation').and.callThrough();
+      await show(done, renderer);
+      renderer.dialogContainer.dispatchEvent(event);
+      expect(event.stopPropagation).not.toHaveBeenCalled();
+      expect(event.stopImmediatePropagation).not.toHaveBeenCalled();
+      done();
+    });
+
+    it('do not cancel events', async done => {
+      const renderer = createRenderer({mouseEvent: 'mousedown'});
+      const event = new MouseEvent('mousedown');
+      spyOn(event, 'preventDefault').and.callThrough();
+      await show(done, renderer);
+      renderer.dialogContainer.dispatchEvent(event);
+      expect(event.preventDefault).not.toHaveBeenCalled();
+      done();
+    });
+  });
 });
 
 describe('"hasTransition"', () => {

test/unit/ux-dialog-renderer.spec.ts

@@ -1,8 +1,9 @@
-import '../setup';
 import { DOM } from 'aurelia-pal';
+
+import '../setup';
 import { DialogController } from '../../src/dialog-controller';
-import { DialogRenderer, hasTransition, transitionEvent } from '../../src/renderers/ux-dialog-renderer';
 import { DefaultDialogSettings, DialogSettings } from '../../src/dialog-settings';
+import { DialogRenderer, hasTransition, transitionEvent } from '../../src/renderers/ux-dialog-renderer';
 
 type TestDialogRenderer = DialogRenderer & { [key: string]: any, __controller: DialogController };
 
@@ -387,7 +388,7 @@ describe('ux-dialog-renderer.spec.ts', () => {
     });
   });
 
-  describe('"backdropDismiss" handlers', () => {
+  describe('"backdropDismiss" handlers as default', () => {
     it('do not stop events propagation', async done => {
       const renderer = createRenderer();
       const event = new MouseEvent('click');
@@ -410,6 +411,30 @@ describe('ux-dialog-renderer.spec.ts', () => {
       done();
     });
   });
+
+  describe('"backdropDismiss" handlers with custom mouseEvent setting set', () => {
+    it('do not stop events propagation', async done => {
+      const renderer = createRenderer({mouseEvent: 'mousedown'});
+      const event = new MouseEvent('mousedown');
+      spyOn(event, 'stopPropagation').and.callThrough();
+      spyOn(event, 'stopImmediatePropagation').and.callThrough();
+      await show(done, renderer);
+      renderer.dialogContainer.dispatchEvent(event);
+      expect(event.stopPropagation).not.toHaveBeenCalled();
+      expect(event.stopImmediatePropagation).not.toHaveBeenCalled();
+      done();
+    });
+
+    it('do not cancel events', async done => {
+      const renderer = createRenderer({mouseEvent: 'mousedown'});
+      const event = new MouseEvent('mousedown');
+      spyOn(event, 'preventDefault').and.callThrough();
+      await show(done, renderer);
+      renderer.dialogContainer.dispatchEvent(event);
+      expect(event.preventDefault).not.toHaveBeenCalled();
+      done();
+    });
+  });
 });
 
 describe('"hasTransition"', () => {