From 1d17f52b2e718b3bfec9d9b6ba8483e82e1b1208 Mon Sep 17 00:00:00 2001 From: Major-Mayer Date: Mon, 14 Oct 2024 17:16:22 +0200 Subject: [PATCH 1/2] Use isFocused() instead of isVisible() in tray service closes \#6429 --- app/SystemTrayService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/SystemTrayService.ts b/app/SystemTrayService.ts index db38ed98529..160ef9e08f1 100644 --- a/app/SystemTrayService.ts +++ b/app/SystemTrayService.ts @@ -157,7 +157,7 @@ export class SystemTrayService { Menu.buildFromTemplate([ { id: 'toggleWindowVisibility', - ...(browserWindow?.isVisible() + ...(browserWindow?.isFocused() ? { label: this.i18n('icu:hide'), click: () => { @@ -223,7 +223,7 @@ export class SystemTrayService { if (!browserWindow) { return; } - if (browserWindow.isVisible()) { + if (browserWindow.isFocused()) { browserWindow.hide(); } else { browserWindow.show(); From cff73375f57aba896a2b032ddf68b97fdd588336 Mon Sep 17 00:00:00 2001 From: Major-Mayer Date: Wed, 16 Oct 2024 19:26:19 +0200 Subject: [PATCH 2/2] Fix test case, change tray menu label, and listen to focus/ blur events for correct labels --- _locales/en/messages.json | 4 + app/SystemTrayService.ts | 102 ++++++++++++--------- ts/test-node/app/SystemTrayService_test.ts | 12 +-- 3 files changed, 69 insertions(+), 49 deletions(-) diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 88ef501c837..7dd8e58246d 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -903,6 +903,10 @@ "messageformat": "Show", "description": "Command under Window menu, to show the window" }, + "icu:showOrFocus": { + "messageformat": "Show/Focus", + "description": "Command in the tray icon menu, to show or focus the window, depending on the current visibility" + }, "icu:hide": { "messageformat": "Hide", "description": "Command in the tray icon menu, to hide the window" diff --git a/app/SystemTrayService.ts b/app/SystemTrayService.ts index 160ef9e08f1..e783fa9a94f 100644 --- a/app/SystemTrayService.ts +++ b/app/SystemTrayService.ts @@ -129,6 +129,50 @@ export class SystemTrayService { this.renderDisabled(); } + private createContextMenu() { + const { browserWindow } = this; + return Menu.buildFromTemplate([ + { + id: 'toggleWindowVisibility', + ...(browserWindow?.isFocused() + ? { + label: this.i18n('icu:hide'), + click: () => { + log.info( + 'System tray service: hiding the window from the context menu' + ); + // We re-fetch `this.browserWindow` here just in case the browser window + // has changed while the context menu was open. Same applies in the + // "show" case below. + browserWindow?.hide(); + }, + } + : { + label: this.i18n('icu:showOrFocus'), + click: () => { + log.info( + 'System tray service: showing the window from the context menu' + ); + if (browserWindow) { + browserWindow.show(); + focusAndForceToTop(browserWindow); + } + }, + }), + }, + { + id: 'quit', + label: this.i18n('icu:quit'), + click: () => { + log.info( + 'System tray service: quitting the app from the context menu' + ); + app.quit(); + }, + }, + ]); + } + private renderEnabled() { if (this.isQuitting) { log.info('System tray service: not rendering the tray, quitting'); @@ -138,7 +182,7 @@ export class SystemTrayService { log.info('System tray service: rendering the tray'); this.tray = this.tray || this.createTray(); - const { browserWindow, tray } = this; + const { tray } = this; try { tray.setImage(getIcon(this.unreadCount)); @@ -153,48 +197,7 @@ export class SystemTrayService { // context menu, since the 'click' event may not work on all platforms. // For details please refer to: // https://github.com/electron/electron/blob/master/docs/api/tray.md. - tray.setContextMenu( - Menu.buildFromTemplate([ - { - id: 'toggleWindowVisibility', - ...(browserWindow?.isFocused() - ? { - label: this.i18n('icu:hide'), - click: () => { - log.info( - 'System tray service: hiding the window from the context menu' - ); - // We re-fetch `this.browserWindow` here just in case the browser window - // has changed while the context menu was open. Same applies in the - // "show" case below. - this.browserWindow?.hide(); - }, - } - : { - label: this.i18n('icu:show'), - click: () => { - log.info( - 'System tray service: showing the window from the context menu' - ); - if (this.browserWindow) { - this.browserWindow.show(); - focusAndForceToTop(this.browserWindow); - } - }, - }), - }, - { - id: 'quit', - label: this.i18n('icu:quit'), - click: () => { - log.info( - 'System tray service: quitting the app from the context menu' - ); - app.quit(); - }, - }, - ]) - ); + tray.setContextMenu(this.createContextMenu()); } private renderDisabled() { @@ -233,6 +236,19 @@ export class SystemTrayService { result.setToolTip(this.i18n('icu:signalDesktop')); + const { browserWindow } = this; + browserWindow?.on('focus', () => { + // Need to recreate the context menu every time, since we cannot dynamically replace MenuItem's + // or change the label/ click handler + // Ref: https://github.com/electron/electron/issues/12633 + + this.tray?.setContextMenu(this.createContextMenu()); + }); + + browserWindow?.on('blur', () => { + this.tray?.setContextMenu(this.createContextMenu()); + }); + return result; } diff --git a/ts/test-node/app/SystemTrayService_test.ts b/ts/test-node/app/SystemTrayService_test.ts index c2ced014ee4..a6f7b4bbedc 100644 --- a/ts/test-node/app/SystemTrayService_test.ts +++ b/ts/test-node/app/SystemTrayService_test.ts @@ -73,15 +73,15 @@ describe('SystemTrayService', function (this: Mocha.Suite) { // We don't actually want to show a browser window. It's disruptive when you're // running tests and can introduce test-only flakiness. We jump through some hoops // to fake the behavior. - let fakeIsVisible = false; - const browserWindow = new BrowserWindow({ show: fakeIsVisible }); - sinon.stub(browserWindow, 'isVisible').callsFake(() => fakeIsVisible); + let fakeIsFocused = false; + const browserWindow = new BrowserWindow({ show: fakeIsFocused }); + sinon.stub(browserWindow, 'isFocused').callsFake(() => fakeIsFocused); sinon.stub(browserWindow, 'show').callsFake(() => { - fakeIsVisible = true; + fakeIsFocused = true; browserWindow.emit('show'); }); sinon.stub(browserWindow, 'hide').callsFake(() => { - fakeIsVisible = false; + fakeIsFocused = false; browserWindow.emit('hide'); }); @@ -107,7 +107,7 @@ describe('SystemTrayService', function (this: Mocha.Suite) { assert.strictEqual(getToggleMenuItem()?.label, 'Hide'); getToggleMenuItem()?.click(); - assert.strictEqual(getToggleMenuItem()?.label, 'Show'); + assert.strictEqual(getToggleMenuItem()?.label, 'Show/Focus'); getToggleMenuItem()?.click(); assert.strictEqual(getToggleMenuItem()?.label, 'Hide');