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

Add beforeunload as first class window event #185

Merged
merged 2 commits into from
Aug 16, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/Electron/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 11 additions & 1 deletion src/OpenFin/openfin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This callback is not covered in the test and is the resulting loss of coverage

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 {
Expand Down
8 changes: 4 additions & 4 deletions src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
18 changes: 17 additions & 1 deletion src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Copy link
Member Author

@bingenito bingenito Aug 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

desktopJS events wrap the internal container events for usage consistency. The returnValue on the djs event args needs to be propagated back to the inner event for the specific container.

}
}

protected unwrapAndUnRegisterListener(listener: (event: EventArgs) => void): (event: EventArgs) => void {
const callback = this.wrappedListeners.get(listener);
if (callback) {
Expand Down
3 changes: 2 additions & 1 deletion src/window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ export type WindowEventType =
"blur" |
"maximize" |
"minimize" |
"restore"
"restore" |
"beforeunload"
;

export class WindowEventArgs extends EventArgs {
Expand Down
29 changes: 25 additions & 4 deletions tests/unit/Electron/electron.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
86 changes: 46 additions & 40 deletions tests/unit/OpenFin/openfin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/events.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});