From e7150f92bfd50e5d46ba65e85163bdac808a2229 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 21 Sep 2020 12:47:52 -0700 Subject: [PATCH 1/7] Add better way to access visibility since document.visibilityState does not work in Electron --- main.js | 6 +++++- src/lib/actions/Report.js | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/main.js b/main.js index ad1c5ebabee39..c071f169e5ded 100644 --- a/main.js +++ b/main.js @@ -1,4 +1,4 @@ -const {app, BrowserWindow, shell} = require('electron'); +const {app, BrowserWindow, shell, ipcMain} = require('electron'); const serve = require('electron-serve'); const contextMenu = require('electron-context-menu'); const {autoUpdater} = require('electron-updater'); @@ -57,6 +57,10 @@ const mainWindow = (() => { return shell.openExternal(url); }); + ipcMain.on('request-visibility', (event) => { + event.returnValue = browserWindow.isFocused(); + }); + return browserWindow; }) diff --git a/src/lib/actions/Report.js b/src/lib/actions/Report.js index a86f0bc72c80e..90c311a88dbdc 100644 --- a/src/lib/actions/Report.js +++ b/src/lib/actions/Report.js @@ -11,6 +11,7 @@ import ExpensiMark from '../ExpensiMark'; import Notification from '../Notification'; import * as PersonalDetails from './PersonalDetails'; import {redirect} from './App'; +import Visibility from '../Visibility'; let currentUserEmail; let currentUserAccountID; @@ -200,7 +201,7 @@ function updateReportWithNewAction(reportID, reportAction) { const currentReportID = Number(lodashGet(currentURL.split('/'), [1], 0)); // If we are currently viewing this report do not show a notification. - if (reportID === currentReportID) { + if (reportID === currentReportID && Visibility.isVisible()) { console.debug('[NOTIFICATION] No notification because it was a comment for the current report'); return; } From 617c17995b45439f4f9a98061a3102c86c6a4eb6 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 21 Sep 2020 13:00:19 -0700 Subject: [PATCH 2/7] add the visibility lib --- src/lib/Visibility/index.js | 13 +++++++++++++ src/lib/Visibility/index.native.js | 5 +++++ 2 files changed, 18 insertions(+) create mode 100644 src/lib/Visibility/index.js create mode 100644 src/lib/Visibility/index.native.js diff --git a/src/lib/Visibility/index.js b/src/lib/Visibility/index.js new file mode 100644 index 0000000000000..7de536f7e944e --- /dev/null +++ b/src/lib/Visibility/index.js @@ -0,0 +1,13 @@ +const ipcRenderer = window.require ? window.require('electron').ipcRenderer : null; + +function isVisible() { + if (ipcRenderer) { + return ipcRenderer.sendSync('request-visibility'); + } + + return document.visibilityState === 'visible'; +} + +export default { + isVisible, +}; diff --git a/src/lib/Visibility/index.native.js b/src/lib/Visibility/index.native.js new file mode 100644 index 0000000000000..4fcbd85216404 --- /dev/null +++ b/src/lib/Visibility/index.native.js @@ -0,0 +1,5 @@ +const isVisible = () => true; + +export default { + isVisible, +} From 2df98130bd9d05bb249640687bff5840f5fe93b9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 21 Sep 2020 13:11:10 -0700 Subject: [PATCH 3/7] add docs --- src/lib/Visibility/index.js | 18 +++++++++++++----- src/lib/Visibility/index.native.js | 6 ++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/lib/Visibility/index.js b/src/lib/Visibility/index.js index 7de536f7e944e..95067099d484c 100644 --- a/src/lib/Visibility/index.js +++ b/src/lib/Visibility/index.js @@ -1,11 +1,19 @@ +// We conditionally import the ipcRenderer here so that we can +// communicate with the main Electron process in main.js const ipcRenderer = window.require ? window.require('electron').ipcRenderer : null; +/** + * Detects whether the app is visible or not. Electron supports + * document.visibilityState, but switching to another app does not + * trigger a state of hidden so we ask the main process synchronously + * whether the BrowserWindow.isFocused() + * + * @returns {Boolean} + */ function isVisible() { - if (ipcRenderer) { - return ipcRenderer.sendSync('request-visibility'); - } - - return document.visibilityState === 'visible'; + return ipcRenderer + ? ipcRenderer.sendSync('request-visibility') + : document.visibilityState === 'visible'; } export default { diff --git a/src/lib/Visibility/index.native.js b/src/lib/Visibility/index.native.js index 4fcbd85216404..2636b7aaa5d24 100644 --- a/src/lib/Visibility/index.native.js +++ b/src/lib/Visibility/index.native.js @@ -1,3 +1,9 @@ +// Mobile apps do not require this check for visibility as +// they do not use the Notification lib. + +/** + * @return {Boolean} + */ const isVisible = () => true; export default { From 4960dbd4f01aea3de365a537ebe96f54f5210281 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 21 Sep 2020 13:42:09 -0700 Subject: [PATCH 4/7] Fix style --- main.js | 9 ++++++++- src/lib/Visibility/index.js | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/main.js b/main.js index c071f169e5ded..3714f8a9c85e1 100644 --- a/main.js +++ b/main.js @@ -1,4 +1,9 @@ -const {app, BrowserWindow, shell, ipcMain} = require('electron'); +const { + app, + BrowserWindow, + shell, + ipcMain +} = require('electron'); const serve = require('electron-serve'); const contextMenu = require('electron-context-menu'); const {autoUpdater} = require('electron-updater'); @@ -58,6 +63,8 @@ const mainWindow = (() => { }); ipcMain.on('request-visibility', (event) => { + // This is how synchronous messages work in Electron + // eslint-disable-next-line no-param-reassign event.returnValue = browserWindow.isFocused(); }); diff --git a/src/lib/Visibility/index.js b/src/lib/Visibility/index.js index 95067099d484c..a375b7c7b3db5 100644 --- a/src/lib/Visibility/index.js +++ b/src/lib/Visibility/index.js @@ -5,9 +5,11 @@ const ipcRenderer = window.require ? window.require('electron').ipcRenderer : nu /** * Detects whether the app is visible or not. Electron supports * document.visibilityState, but switching to another app does not - * trigger a state of hidden so we ask the main process synchronously + * always trigger a state of hidden so we ask the main process synchronously * whether the BrowserWindow.isFocused() * + * See: https://github.com/electron/electron/issues/8664 + * * @returns {Boolean} */ function isVisible() { From 4e40f58f8dd98c4c87f94369b6e93a942e176cb3 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 21 Sep 2020 13:42:38 -0700 Subject: [PATCH 5/7] fix style --- src/lib/Visibility/index.native.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Visibility/index.native.js b/src/lib/Visibility/index.native.js index 2636b7aaa5d24..ba878d4ef7d07 100644 --- a/src/lib/Visibility/index.native.js +++ b/src/lib/Visibility/index.native.js @@ -8,4 +8,4 @@ const isVisible = () => true; export default { isVisible, -} +}; From ad8c730c6dee3173ff31dd09b39c5fdbeb76775a Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 21 Sep 2020 14:01:30 -0700 Subject: [PATCH 6/7] add extra info about occlusion --- src/lib/Visibility/index.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/lib/Visibility/index.js b/src/lib/Visibility/index.js index a375b7c7b3db5..3831a3868af01 100644 --- a/src/lib/Visibility/index.js +++ b/src/lib/Visibility/index.js @@ -4,11 +4,10 @@ const ipcRenderer = window.require ? window.require('electron').ipcRenderer : nu /** * Detects whether the app is visible or not. Electron supports - * document.visibilityState, but switching to another app does not - * always trigger a state of hidden so we ask the main process synchronously - * whether the BrowserWindow.isFocused() - * - * See: https://github.com/electron/electron/issues/8664 + * document.visibilityState, but switching to another app while + * Electron is partially occluded will not trigger a state of hidden + * so we ask the main process synchronously whether the + * BrowserWindow.isFocused() * * @returns {Boolean} */ @@ -20,4 +19,4 @@ function isVisible() { export default { isVisible, -}; +}; \ No newline at end of file From 4651260b8ca763aaeb05584d84b7c4ad1b6d8a0f Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 21 Sep 2020 14:03:39 -0700 Subject: [PATCH 7/7] add newline --- src/lib/Visibility/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Visibility/index.js b/src/lib/Visibility/index.js index 3831a3868af01..7929d824a5f27 100644 --- a/src/lib/Visibility/index.js +++ b/src/lib/Visibility/index.js @@ -19,4 +19,4 @@ function isVisible() { export default { isVisible, -}; \ No newline at end of file +};