From cd409e40ac009bedac510f2196f331f176fdef1e Mon Sep 17 00:00:00 2001 From: Jose Marino Date: Thu, 6 Sep 2018 11:45:47 -0600 Subject: [PATCH 1/4] use "static" variable to detect startup Add a property to checkUnreads function that behaves like a static variable. It can be used to detect the first time this function is called. The reason for this change is to avoid relying on an empty seenMessages to detect startup. --- app/renderer/unreads.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/renderer/unreads.js b/app/renderer/unreads.js index 8ae3316..ed18514 100644 --- a/app/renderer/unreads.js +++ b/app/renderer/unreads.js @@ -68,18 +68,20 @@ function getUnreadMessages() { } function checkUnreads(period = 2000) { + if ( typeof checkUnreads.startingUp == 'undefined' ) { + checkUnreads.startingUp = true; + } + const unreads = getUnreadMessages(); ipc.send('update-unreads-count', unreads.length); - const startingUp = seenMessages.size === 0; - unreads.filter(message => !seenMessages.has(keyByMessage(message))).forEach((message) => { const { element, subject, sender, avatar, } = message; // do not show the same notification every time on start up - if (!startingUp) { + if (!checkUnreads.startingUp) { sendNotification({ title: sender, body: subject, @@ -92,6 +94,9 @@ function checkUnreads(period = 2000) { // mark message as seen seenMessages.set(keyByMessage(message), true); }); + if (checkUnreads.startingUp) { + checkUnreads.startingUp = false; + } setTimeout(checkUnreads, period); } From ad276f7ccb8b5a2b8d134d19409ae3ecd13d0c4b Mon Sep 17 00:00:00 2001 From: Jose Marino Date: Wed, 10 Oct 2018 15:16:36 -0600 Subject: [PATCH 2/4] clean up seenMessages as new messages are read seenMessages keeps track of previously seen unread messages so we don't continuously notify the user about them. As these unread messages are read, they should be deleted from seenMessages. This stops seenMessages from growing indefinitely and fixes the following issue: Inboxer fails to generate a new email notification when: - Alice and I have an email thread going - Alice sends me a new email in the thread -> a new email notification is generated - I read Alice's email but don't reply - Alice sends me another email in the thread -> NO new email notification is generated because the new email looks like the old one already stored in seenMessages. --- app/renderer/unreads.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/app/renderer/unreads.js b/app/renderer/unreads.js index ed18514..396459d 100644 --- a/app/renderer/unreads.js +++ b/app/renderer/unreads.js @@ -76,12 +76,18 @@ function checkUnreads(period = 2000) { ipc.send('update-unreads-count', unreads.length); - unreads.filter(message => !seenMessages.has(keyByMessage(message))).forEach((message) => { + // mark all previously seen messages as false + for(const key of seenMessages.keys()) { + seenMessages.set(key, false); + } + + unreads.forEach((message) => { const { element, subject, sender, avatar, } = message; + const key = keyByMessage(message); // do not show the same notification every time on start up - if (!checkUnreads.startingUp) { + if (!checkUnreads.startingUp && !seenMessages.has(key)) { sendNotification({ title: sender, body: subject, @@ -92,8 +98,16 @@ function checkUnreads(period = 2000) { }); } // mark message as seen - seenMessages.set(keyByMessage(message), true); + seenMessages.set(key, true); }); + + // clean up old entries in seenMessages + for(const key of seenMessages.keys()) { + if (seenMessages.get(key) == false) { + seenMessages.delete(key); + } + } + if (checkUnreads.startingUp) { checkUnreads.startingUp = false; } From 8af18ff02114a09297500e0b93c239e3803bd1b3 Mon Sep 17 00:00:00 2001 From: Jose Marino Date: Wed, 10 Oct 2018 14:16:08 -0600 Subject: [PATCH 3/4] bail out early looking for unread messages when not in inbox getUnreadMessages returns [] for two very different situations: not in inbox & zero unread messages. Moving the in-inbox check to function checkUnreads makes it possible to differentiate between these two situations and addresses these two issues: - Leaving the inbox causes a reset of seenMessages. When we return to the inbox, all the previous unseen messages will be counted as new and generate new notifications. - Leaving the inbox will update unread count to zero, even though we still have unread emails. --- app/renderer/unreads.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/renderer/unreads.js b/app/renderer/unreads.js index 396459d..da76e64 100644 --- a/app/renderer/unreads.js +++ b/app/renderer/unreads.js @@ -42,13 +42,6 @@ function extractSender(el, message) { } function getUnreadMessages() { - // not inside the inbox - const isInbox = $('.hA [title=Inbox]'); - - if (!isInbox) { - return []; - } - return Array.from($$('.ss')) .map((message) => { const ancestorEl = ancestor(message, '.jS'); @@ -68,6 +61,13 @@ function getUnreadMessages() { } function checkUnreads(period = 2000) { + // skip if we're not inside the inbox + const isInbox = $('.hA [title=Inbox]'); + if (!isInbox) { + setTimeout(checkUnreads, period); + return; + } + if ( typeof checkUnreads.startingUp == 'undefined' ) { checkUnreads.startingUp = true; } From eab08be2a16960110249f21cee523af6a5546f0f Mon Sep 17 00:00:00 2001 From: Jose Marino Date: Wed, 10 Oct 2018 16:51:38 -0600 Subject: [PATCH 4/4] fix some Travis build errors Fix spaces that Travis doesn't like. Use === instead of ==. Replace for loop with forEach. Travis complains about for loops with iterators being too haveyweight. --- app/renderer/unreads.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/renderer/unreads.js b/app/renderer/unreads.js index da76e64..efe9f9e 100644 --- a/app/renderer/unreads.js +++ b/app/renderer/unreads.js @@ -68,7 +68,7 @@ function checkUnreads(period = 2000) { return; } - if ( typeof checkUnreads.startingUp == 'undefined' ) { + if (typeof checkUnreads.startingUp === 'undefined') { checkUnreads.startingUp = true; } @@ -77,9 +77,9 @@ function checkUnreads(period = 2000) { ipc.send('update-unreads-count', unreads.length); // mark all previously seen messages as false - for(const key of seenMessages.keys()) { - seenMessages.set(key, false); - } + seenMessages.forEach((value, key, map) => { + map.set(key, false); + }); unreads.forEach((message) => { const { @@ -102,11 +102,11 @@ function checkUnreads(period = 2000) { }); // clean up old entries in seenMessages - for(const key of seenMessages.keys()) { - if (seenMessages.get(key) == false) { - seenMessages.delete(key); + seenMessages.forEach((value, key, map) => { + if (value === false) { + map.delete(key); } - } + }); if (checkUnreads.startingUp) { checkUnreads.startingUp = false;