Skip to content
This repository has been archived by the owner on Sep 6, 2023. It is now read-only.

Commit

Permalink
Restore notificationclick handler to activate the app. Fixes #16
Browse files Browse the repository at this point in the history
  • Loading branch information
jwheare committed May 16, 2019
1 parent 237dc33 commit 29423f4
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
18 changes: 18 additions & 0 deletions app/render/notification.js
@@ -0,0 +1,18 @@
var remote = require('electron').remote;

function listenNotification() {
remote.getCurrentWindow().webContents.executeJavaScript(
'new Promise((resolve, reject) => { if (SESSION) { console.log("SESSION avail"); SESSION.on("notificationClick", function () { resolve(); }); } });'

This comment has been minimized.

Copy link
@Macil

Macil May 16, 2019

It looks like there's a memory leak here: every time the user clicks on a notification, the existing event listener isn't removed, and a new one is added. Assuming that SESSION follows Node's EventEmitter API, this can be fixed by changing .on( to .once(.

This comment has been minimized.

Copy link
@jwheare

jwheare May 17, 2019

Author Member

Thanks, sounds like a good call, I’ll take a look.

This comment has been minimized.

Copy link
@jwheare

jwheare May 17, 2019

Author Member

Yup, thanks again, fixed in 0246737

).then(() => {
remote.app.emit('activate');
listenNotification();
});
}

function setupNotificationHandler() {
document.addEventListener("DOMContentLoaded", function (event) {
listenNotification();
});
}

module.exports = setupNotificationHandler;
1 change: 1 addition & 0 deletions app/render/preload.js
Expand Up @@ -24,6 +24,7 @@
require('./focus')();
require('./zoom')();
require('./spellcheck')();
require('./notification')();
require('./irc-url')();
require('./user')();

Expand Down

0 comments on commit 29423f4

Please sign in to comment.