From 5d7acd41b601a7ffca6cb1dbd10847e9751b56df Mon Sep 17 00:00:00 2001 From: bingenito Date: Wed, 15 Aug 2018 08:56:47 -0400 Subject: [PATCH 1/2] Add beforeunload as first class window event to allow providing custom implementation --- src/Electron/electron.ts | 15 ++++- src/OpenFin/openfin.ts | 12 +++- src/events.ts | 18 +++++- src/window.ts | 3 +- tests/unit/Electron/electron.spec.ts | 29 ++++++++-- tests/unit/OpenFin/openfin.spec.ts | 86 +++++++++++++++------------- tests/unit/events.spec.ts | 8 +++ 7 files changed, 122 insertions(+), 49 deletions(-) diff --git a/src/Electron/electron.ts b/src/Electron/electron.ts index 473d8af4..2a181c16 100644 --- 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 index c21c38b1..d7b8f39d 100644 --- 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/events.ts b/src/events.ts index a1c186b2..08dc7427 100644 --- 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") { + (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 index 2586134c..a2f99e85 100644 --- 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 index 88c074b3..8e95ad2f 100644 --- 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 index c135a235..5cbc2019 100644 --- 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 index 98f83a42..12348963 100644 --- 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((innerEvent).returnValue).toEqual("Foo"); + }); }); From a3cb97491cd504019eae252c35f7d3dead6a3421 Mon Sep 17 00:00:00 2001 From: bingenito Date: Wed, 15 Aug 2018 09:08:59 -0400 Subject: [PATCH 2/2] Fix tslint console usage error --- src/container.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/container.ts b/src/container.ts index a75dbce1..a3b00baa 100644 --- 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 } }