Add beforeunload as first class window event (#185)

Add beforeunload as first class window event to allow providing custom implementation
morganstanley · Aug 16, 2018 · edd6745 · edd6745
1 parent 033402f
commit edd6745
Show file tree

Hide file tree

Showing 8 changed files with 126 additions and 53 deletions.
diff --git a/src/Electron/electron.ts b/src/Electron/electron.ts
@@ -35,10 +35,12 @@ const windowEventMap = {};
  */
 export class ElectronContainerWindow extends ContainerWindow {
     private readonly container: ElectronContainer;
+    private readonly window: Window;
 
-    public constructor(wrap: any, container: ElectronContainer) {
+    public constructor(wrap: any, container: ElectronContainer, win?: Window) {
         super(wrap);
         this.container = container;
+        this.window = win;
     }
 
     private get isRemote(): boolean {
@@ -176,7 +178,16 @@ export class ElectronContainerWindow extends ContainerWindow {
     }
 
     protected attachListener(eventName: string, listener: (...args: any[]) => void): void {
-        this.innerWindow.addListener(windowEventMap[eventName] || eventName, listener);
+        if (eventName === "beforeunload") {
+            const win = this.window || window;
+            if (win && this.id === this.container.getCurrentWindow().id) {
+                win.addEventListener("beforeunload", listener);
+            } else {
+                throw new Error("Event handler for 'beforeunload' can only be added on current window");
+            }
+        } else {
+            this.innerWindow.addListener(windowEventMap[eventName] || eventName, listener);
+        }
     }
 
     protected detachListener(eventName: string, listener: (...args: any[]) => void): void {

diff --git a/src/OpenFin/openfin.ts b/src/OpenFin/openfin.ts
@@ -158,7 +158,17 @@ export class OpenFinContainerWindow extends ContainerWindow {
     }
 
     protected attachListener(eventName: string, listener: (...args: any[]) => void): void {
-        this.innerWindow.addEventListener(windowEventMap[eventName] || eventName, listener);
+        if (eventName === "beforeunload") {
+            this.innerWindow.addEventListener("close-requested", (e) => {
+                const args = new EventArgs(this, "beforeunload", e);
+                listener(args);
+                if (typeof args.returnValue === "undefined") {
+                    this.innerWindow.close(true);
+                }
+            });
+        } else {
+            this.innerWindow.addEventListener(windowEventMap[eventName] || eventName, listener);
+        }
     }
 
     protected wrapListener(eventName: string, listener: (event: EventArgs) => void): (event: EventArgs) => void {

diff --git a/src/container.ts b/src/container.ts
@@ -251,19 +251,19 @@ export abstract class ContainerBase extends Container {
             let logger;
             switch (level) {
                 case "debug": {
-                    logger = console.debug;
+                    logger = console.debug; // tslint:disable-line
                     break;
                 }
                 case "warn": {
-                    logger = console.warn;
+                    logger = console.warn; // tslint:disable-line
                     break;
                 }
                 case "error": {
-                    logger = console.error;
+                    logger = console.error; // tslint:disable-line
                     break;
                 }
                 default: {
-                    logger = console.log;
+                    logger = console.log; // tslint:disable-line
                 }
             }
 

diff --git a/src/events.ts b/src/events.ts
@@ -7,6 +7,8 @@ export class EventArgs {
 
     public readonly name: string;
 
+    public returnValue? : any;
+
     constructor(sender: any, name: string, innerEvent?: any) {
         this.sender = sender;
         this.name = name;
@@ -41,10 +43,24 @@ export class EventEmitter {
 
     protected wrapListener(eventName: string, listener: (event: EventArgs) => void): (event: EventArgs) => void {
         return (event) => {
-            listener(new EventArgs(this, eventName, event));
+            const args = new EventArgs(this, eventName, event);
+            this.preProcessArgs(args);
+            const result = listener(args);
+            this.postProcessArgs(args);
+            return result;
         };
     }
 
+    protected preProcessArgs(args: EventArgs) {
+        // Do nothing in base
+    }
+
+    protected postProcessArgs(args: EventArgs) {
+        if (args && typeof args.returnValue !== "undefined") {
+            (<any>args.innerEvent).returnValue = args.returnValue;
+        }
+    }
+
     protected unwrapAndUnRegisterListener(listener: (event: EventArgs) => void): (event: EventArgs) => void {
         const callback = this.wrappedListeners.get(listener);
         if (callback) {

diff --git a/src/window.ts b/src/window.ts
@@ -60,7 +60,8 @@ export type WindowEventType =
     "blur" |
     "maximize" |
     "minimize" |
-    "restore"
+    "restore" |
+    "beforeunload"
     ;
 
 export class WindowEventArgs extends EventArgs {

diff --git a/tests/unit/Electron/electron.spec.ts b/tests/unit/Electron/electron.spec.ts
@@ -236,10 +236,31 @@ describe("ElectronContainerWindow", () => {
             }).then(done);
         });
 
-        it("addListener calls underlying Electron window addListener", () => {
-            spyOn(win.innerWindow, "addListener").and.callThrough()
-            win.addListener("move", () => {});
-            expect(win.innerWindow.addListener).toHaveBeenCalledWith("move", jasmine.any(Function));
+        describe("addListener", () => {
+            it("calls underlying Electron window addListener", () => {
+                spyOn(win.innerWindow, "addListener").and.callThrough()
+                win.addListener("move", () => {});
+                expect(win.innerWindow.addListener).toHaveBeenCalledWith("move", jasmine.any(Function));
+            });
+
+            describe("beforeunload", () => {
+                it ("beforeunload attaches to underlying dom window", () => {
+                    var window = jasmine.createSpyObj("window", ["addEventListener"]);
+                    spyOn(container, "getCurrentWindow").and.returnValue({ id: 1});
+                    win = new ElectronContainerWindow(innerWin, container, window);
+                    spyOnProperty(win, "id", "get").and.returnValue(1);
+                    win.addListener("beforeunload", () => {});
+                    expect(window.addEventListener).toHaveBeenCalledWith("beforeunload", jasmine.any(Function));
+                });
+
+                it ("beforeunload throws error if not current window", () => {
+                    var window = jasmine.createSpyObj("window", ["addEventListener"]);
+                    spyOn(container, "getCurrentWindow").and.returnValue({ id: 2});
+                    win = new ElectronContainerWindow(innerWin, container, window);
+                    spyOnProperty(win, "id", "get").and.returnValue(1);
+                    expect(() => {win.addListener("beforeunload", () => {})}).toThrowError("Event handler for 'beforeunload' can only be added on current window");
+                });
+            });
         });
 
         it("removeListener calls underlying Electron window removeListener", () => {

diff --git a/tests/unit/OpenFin/openfin.spec.ts b/tests/unit/OpenFin/openfin.spec.ts
@@ -314,58 +314,64 @@ describe("OpenFinContainerWindow", () => {
                 }).then(done);
             });
         });
+    });
 
-        describe("addListener", () => {
-            it("addListener calls underlying OpenFin window addEventListener with mapped event name", () => {
-                spyOn(win.innerWindow, "addEventListener").and.callThrough()
-                win.addListener("move", () => { });
-                expect(win.innerWindow.addEventListener).toHaveBeenCalledWith("bounds-changing", jasmine.any(Function));
-            });
+    describe("addListener", () => {
+        it("addListener calls underlying OpenFin window addEventListener with mapped event name", () => {
+            spyOn(win.innerWindow, "addEventListener").and.callThrough()
+            win.addListener("move", () => { });
+            expect(win.innerWindow.addEventListener).toHaveBeenCalledWith("bounds-changing", jasmine.any(Function));
+        });
 
-            it("addListener calls underlying OpenFin window addEventListener with unmapped event name", () => {
-                const unmappedEvent = "closed";
-                spyOn(win.innerWindow, "addEventListener").and.callThrough()
-                win.addListener(unmappedEvent, () => { });
-                expect(win.innerWindow.addEventListener).toHaveBeenCalledWith(unmappedEvent, jasmine.any(Function));
-            });
+        it("addListener calls underlying OpenFin window addEventListener with unmapped event name", () => {
+            const unmappedEvent = "closed";
+            spyOn(win.innerWindow, "addEventListener").and.callThrough()
+            win.addListener(unmappedEvent, () => { });
+            expect(win.innerWindow.addEventListener).toHaveBeenCalledWith(unmappedEvent, jasmine.any(Function));
+        });
 
-            it ("resize wraps filtered bounds-changing", (done) => {
-                spyOn(win.innerWindow, "addEventListener").and.callFake((eventName, listener) => {
-                    listener({ changeType: 1 });
-                });
-                win.addListener("resize", (e) => {
-                    expect(e.innerEvent.changeType).toBeGreaterThanOrEqual(1);
-                    done();
-                });
+        it ("resize wraps filtered bounds-changing", (done) => {
+            spyOn(win.innerWindow, "addEventListener").and.callFake((eventName, listener) => {
+                listener({ changeType: 1 });
             });
-
-            it ("move wraps filtered bounds-changing", (done) => {
-                spyOn(win.innerWindow, "addEventListener").and.callFake((eventName, listener) => {
-                    listener({ changeType: 0 });
-                });
-                win.addListener("move", (e) => {
-                    expect(e.innerEvent.changeType).toEqual(0);
-                    done();
-                });
+            win.addListener("resize", (e) => {
+                expect(e.innerEvent.changeType).toBeGreaterThanOrEqual(1);
+                done();
             });
         });
 
-        describe("removeListener", () => {
-            it("removeListener calls underlying OpenFin window removeEventListener with mapped event name", () => {
-                spyOn(win.innerWindow, "removeEventListener").and.callThrough()
-                win.removeListener("move", () => { });
-                expect(win.innerWindow.removeEventListener).toHaveBeenCalledWith("bounds-changing", jasmine.any(Function));
+        it ("move wraps filtered bounds-changing", (done) => {
+            spyOn(win.innerWindow, "addEventListener").and.callFake((eventName, listener) => {
+                listener({ changeType: 0 });
             });
-
-            it("removeListener calls underlying OpenFin window removeEventListener with unmapped event name", () => {
-                const unmappedEvent = "closed";
-                spyOn(win.innerWindow, "removeEventListener").and.callThrough()
-                win.removeListener(unmappedEvent, () => { });
-                expect(win.innerWindow.removeEventListener).toHaveBeenCalledWith(unmappedEvent, jasmine.any(Function));
+            win.addListener("move", (e) => {
+                expect(e.innerEvent.changeType).toEqual(0);
+                done();
             });
         });
+
+        it ("beforeunload attaches to underlying close-requested ", () => {
+            spyOn(win.innerWindow, "addEventListener").and.stub()
+            win.addListener("beforeunload", (e) => {});
+            expect(win.innerWindow.addEventListener).toHaveBeenCalledWith("close-requested", jasmine.any(Function));
+        });
     });
 
+    describe("removeListener", () => {
+        it("removeListener calls underlying OpenFin window removeEventListener with mapped event name", () => {
+            spyOn(win.innerWindow, "removeEventListener").and.callThrough()
+            win.removeListener("move", () => { });
+            expect(win.innerWindow.removeEventListener).toHaveBeenCalledWith("bounds-changing", jasmine.any(Function));
+        });
+
+        it("removeListener calls underlying OpenFin window removeEventListener with unmapped event name", () => {
+            const unmappedEvent = "closed";
+            spyOn(win.innerWindow, "removeEventListener").and.callThrough()
+            win.removeListener(unmappedEvent, () => { });
+            expect(win.innerWindow.removeEventListener).toHaveBeenCalledWith(unmappedEvent, jasmine.any(Function));
+        });
+    });    
+
     describe("window grouping", () => {
         it("allowGrouping is true", () => {
             expect(win.allowGrouping).toEqual(true);

diff --git a/tests/unit/events.spec.ts b/tests/unit/events.spec.ts
@@ -70,4 +70,12 @@ describe("EventEmitter", () => {
         const wrappedCallback = emitter.registerAndWrapListener("TestEvent", listener);
         expect(emitter.unwrapAndUnRegisterListener(listener)).toEqual(wrappedCallback);
     });
+
+    it ("postProcessArgs copies returnValue to innerEvent", () => {
+        const innerEvent = {};
+        const args = new EventArgs({}, "event", innerEvent);
+        args.returnValue = "Foo";
+        emitter.postProcessArgs(args)
+        expect((<any>innerEvent).returnValue).toEqual("Foo");
+    }); 
 });