From fa23b5cd3f53e462718dc6045ca4645adf4f4f8d Mon Sep 17 00:00:00 2001 From: Janne Savolainen Date: Wed, 22 Jun 2022 15:37:13 +0300 Subject: [PATCH] Fix download percentages not updating in tray (#5697) Co-authored-by: Mikko Aspiala Signed-off-by: Janne Savolainen --- .../installing-update-using-tray.test.ts | 31 ++++++++----------- .../electron-tray/electron-tray.injectable.ts | 13 ++++++-- .../reactive-tray-menu-items/converters.ts | 10 +++--- .../reactive-tray-menu-items.injectable.ts | 26 ++++++++++++---- .../test-utils/get-application-builder.tsx | 26 ++++++++++------ 5 files changed, 66 insertions(+), 40 deletions(-) diff --git a/src/behaviours/application-update/installing-update-using-tray.test.ts b/src/behaviours/application-update/installing-update-using-tray.test.ts index 0cf326d79d..f0e46b448f 100644 --- a/src/behaviours/application-update/installing-update-using-tray.test.ts +++ b/src/behaviours/application-update/installing-update-using-tray.test.ts @@ -14,7 +14,6 @@ import asyncFn from "@async-fn/jest"; import type { DownloadPlatformUpdate } from "../../main/application-update/download-platform-update/download-platform-update.injectable"; import downloadPlatformUpdateInjectable from "../../main/application-update/download-platform-update/download-platform-update.injectable"; import showApplicationWindowInjectable from "../../main/start-main-application/lens-window/show-application-window.injectable"; -import progressOfUpdateDownloadInjectable from "../../common/application-update/progress-of-update-download/progress-of-update-download.injectable"; import type { TrayIconPaths } from "../../main/tray/tray-icon-path.injectable"; import trayIconPathsInjectable from "../../main/tray/tray-icon-path.injectable"; @@ -87,13 +86,13 @@ describe("installing update using tray", () => { it("user cannot check for updates again", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.enabled.get(), + applicationBuilder.tray.get("check-for-updates")?.enabled, ).toBe(false); }); it("name of tray item for checking updates indicates that checking is happening", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.label?.get(), + applicationBuilder.tray.get("check-for-updates")?.label, ).toBe("Checking for updates..."); }); @@ -128,13 +127,13 @@ describe("installing update using tray", () => { it("user can check for updates again", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.enabled.get(), + applicationBuilder.tray.get("check-for-updates")?.enabled, ).toBe(true); }); it("name of tray item for checking updates no longer indicates that checking is happening", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.label?.get(), + applicationBuilder.tray.get("check-for-updates")?.label, ).toBe("Check for updates"); }); @@ -163,25 +162,21 @@ describe("installing update using tray", () => { it("user cannot check for updates again yet", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.enabled.get(), + applicationBuilder.tray.get("check-for-updates")?.enabled, ).toBe(false); }); it("name of tray item for checking updates indicates that downloading is happening", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.label?.get(), + applicationBuilder.tray.get("check-for-updates")?.label, ).toBe("Downloading update some-version (0%)..."); }); it("when download progresses with decimals, percentage increases as integers", () => { - const progressOfUpdateDownload = applicationBuilder.dis.mainDi.inject( - progressOfUpdateDownloadInjectable, - ); - - progressOfUpdateDownload.set({ percentage: 42.424242 }); + downloadPlatformUpdateMock.mock.calls[0][0]({ percentage: 42.424242 }); expect( - applicationBuilder.tray.get("check-for-updates")?.label?.get(), + applicationBuilder.tray.get("check-for-updates")?.label, ).toBe("Downloading update some-version (42%)..."); }); @@ -210,13 +205,13 @@ describe("installing update using tray", () => { it("user can check for updates again", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.enabled.get(), + applicationBuilder.tray.get("check-for-updates")?.enabled, ).toBe(true); }); it("name of tray item for checking updates no longer indicates that downloading is happening", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.label?.get(), + applicationBuilder.tray.get("check-for-updates")?.label, ).toBe("Check for updates"); }); @@ -232,7 +227,7 @@ describe("installing update using tray", () => { it("user can install update", () => { expect( - applicationBuilder.tray.get("install-update")?.label?.get(), + applicationBuilder.tray.get("install-update")?.label, ).toBe("Install update some-version"); }); @@ -242,13 +237,13 @@ describe("installing update using tray", () => { it("user can check for updates again", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.enabled.get(), + applicationBuilder.tray.get("check-for-updates")?.enabled, ).toBe(true); }); it("name of tray item for checking updates no longer indicates that downloading is happening", () => { expect( - applicationBuilder.tray.get("check-for-updates")?.label?.get(), + applicationBuilder.tray.get("check-for-updates")?.label, ).toBe("Check for updates"); }); diff --git a/src/main/tray/electron-tray/electron-tray.injectable.ts b/src/main/tray/electron-tray/electron-tray.injectable.ts index a96104f047..d7547ca57a 100644 --- a/src/main/tray/electron-tray/electron-tray.injectable.ts +++ b/src/main/tray/electron-tray/electron-tray.injectable.ts @@ -9,15 +9,24 @@ import showApplicationWindowInjectable from "../../start-main-application/lens-w import isWindowsInjectable from "../../../common/vars/is-windows.injectable"; import loggerInjectable from "../../../common/logger.injectable"; import trayIconPathsInjectable from "../tray-icon-path.injectable"; -import type { TrayMenuItem } from "../tray-menu-item/tray-menu-item-injection-token"; import { convertToElectronMenuTemplate } from "../reactive-tray-menu-items/converters"; const TRAY_LOG_PREFIX = "[TRAY]"; +export interface MinimalTrayMenuItem { + id: string; + parentId: string | null; + enabled: boolean; + label?: string; + click?: () => Promise | void; + tooltip?: string; + separator?: boolean; +} + export interface ElectronTray { start(): void; stop(): void; - setMenuItems(menuItems: TrayMenuItem[]): void; + setMenuItems(menuItems: MinimalTrayMenuItem[]): void; setIconPath(iconPath: string): void; } diff --git a/src/main/tray/reactive-tray-menu-items/converters.ts b/src/main/tray/reactive-tray-menu-items/converters.ts index 42add7481e..a3007ee045 100644 --- a/src/main/tray/reactive-tray-menu-items/converters.ts +++ b/src/main/tray/reactive-tray-menu-items/converters.ts @@ -2,13 +2,13 @@ * Copyright (c) OpenLens Authors. All rights reserved. * Licensed under MIT License. See LICENSE in root directory for more information. */ -import type { TrayMenuItem } from "../tray-menu-item/tray-menu-item-injection-token"; +import type { MinimalTrayMenuItem } from "../electron-tray/electron-tray.injectable"; -export function convertToElectronMenuTemplate(trayMenuItems: TrayMenuItem[]): Electron.MenuItemConstructorOptions[] { +export function convertToElectronMenuTemplate(trayMenuItems: MinimalTrayMenuItem[]): Electron.MenuItemConstructorOptions[] { const toTrayMenuOptions = (parentId: string | null) => ( trayMenuItems .filter((item) => item.parentId === parentId) - .map((trayMenuItem: TrayMenuItem): Electron.MenuItemConstructorOptions => { + .map((trayMenuItem): Electron.MenuItemConstructorOptions => { if (trayMenuItem.separator) { return { id: trayMenuItem.id, type: "separator" }; } @@ -17,8 +17,8 @@ export function convertToElectronMenuTemplate(trayMenuItems: TrayMenuItem[]): El return { id: trayMenuItem.id, - label: trayMenuItem.label?.get(), - enabled: trayMenuItem.enabled.get(), + label: trayMenuItem.label, + enabled: trayMenuItem.enabled, toolTip: trayMenuItem.tooltip, ...(childItems.length === 0 diff --git a/src/main/tray/reactive-tray-menu-items/reactive-tray-menu-items.injectable.ts b/src/main/tray/reactive-tray-menu-items/reactive-tray-menu-items.injectable.ts index 22c3d29399..145cc1e870 100644 --- a/src/main/tray/reactive-tray-menu-items/reactive-tray-menu-items.injectable.ts +++ b/src/main/tray/reactive-tray-menu-items/reactive-tray-menu-items.injectable.ts @@ -5,26 +5,40 @@ import { getInjectable } from "@ogre-tools/injectable"; import { getStartableStoppable } from "../../../common/utils/get-startable-stoppable"; import { reaction } from "mobx"; +import type { MinimalTrayMenuItem } from "../electron-tray/electron-tray.injectable"; import electronTrayInjectable from "../electron-tray/electron-tray.injectable"; import trayMenuItemsInjectable from "../tray-menu-item/tray-menu-items.injectable"; +import type { TrayMenuItem } from "../tray-menu-item/tray-menu-item-injection-token"; const reactiveTrayMenuItemsInjectable = getInjectable({ id: "reactive-tray-menu-items", instantiate: (di) => { const electronTray = di.inject(electronTrayInjectable); - const trayMenuItems = di.inject(trayMenuItemsInjectable); + const reactiveItems = di.inject(trayMenuItemsInjectable); - return getStartableStoppable("reactive-tray-menu-items", () => ( + return getStartableStoppable("reactive-tray-menu-items", () => reaction( - () => trayMenuItems.get(), - electronTray.setMenuItems, + (): MinimalTrayMenuItem[] => reactiveItems.get().map(toNonReactiveItem), + + (nonReactiveItems) => electronTray.setMenuItems(nonReactiveItems), + { fireImmediately: true, }, - ) - )); + ), + ); }, }); export default reactiveTrayMenuItemsInjectable; + +const toNonReactiveItem = (item: TrayMenuItem): MinimalTrayMenuItem => ({ + id: item.id, + parentId: item.parentId, + click: item.click, + tooltip: item.tooltip, + separator: item.separator, + enabled: item.enabled.get(), + label: item.label?.get(), +}); diff --git a/src/renderer/components/test-utils/get-application-builder.tsx b/src/renderer/components/test-utils/get-application-builder.tsx index 42401e1288..0274bb7d6d 100644 --- a/src/renderer/components/test-utils/get-application-builder.tsx +++ b/src/renderer/components/test-utils/get-application-builder.tsx @@ -23,7 +23,7 @@ import type { ClusterStore } from "../../../common/cluster-store/cluster-store"; import mainExtensionsInjectable from "../../../extensions/main-extensions.injectable"; import currentRouteComponentInjectable from "../../routes/current-route-component.injectable"; import { pipeline } from "@ogre-tools/fp"; -import { flatMap, compact, join, get, filter, map, matches } from "lodash/fp"; +import { flatMap, compact, join, get, filter, map, matches, last } from "lodash/fp"; import preferenceNavigationItemsInjectable from "../+preferences/preferences-navigation/preference-navigation-items.injectable"; import navigateToPreferencesInjectable from "../../../common/front-end-routing/routes/preferences/navigate-to-preferences.injectable"; import type { MenuItemOpts } from "../../../main/menu/application-menu-items.injectable"; @@ -41,6 +41,7 @@ import startFrameInjectable from "../../start-frame/start-frame.injectable"; import type { NamespaceStore } from "../+namespaces/store"; import namespaceStoreInjectable from "../+namespaces/store.injectable"; import historyInjectable from "../../navigation/history.injectable"; +import type { MinimalTrayMenuItem } from "../../../main/tray/electron-tray/electron-tray.injectable"; import electronTrayInjectable from "../../../main/tray/electron-tray/electron-tray.injectable"; import applicationWindowInjectable from "../../../main/start-main-application/lens-window/application-window/application-window.injectable"; import { Notifications } from "../notifications/notifications"; @@ -48,7 +49,6 @@ import broadcastThatRootFrameIsRenderedInjectable from "../../frames/root-frame/ import { getDiForUnitTesting as getRendererDi } from "../../getDiForUnitTesting"; import { getDiForUnitTesting as getMainDi } from "../../../main/getDiForUnitTesting"; import { overrideChannels } from "../../../test-utils/channel-fakes/override-channels"; -import type { TrayMenuItem } from "../../../main/tray/tray-menu-item/tray-menu-item-injection-token"; import trayIconPathsInjectable from "../../../main/tray/tray-icon-path.injectable"; import assert from "assert"; import { openMenu } from "react-select-event"; @@ -59,7 +59,6 @@ import type { Route } from "../../../common/front-end-routing/front-end-route-in import type { NavigateToRouteOptions } from "../../../common/front-end-routing/navigate-to-route-injection-token"; import { navigateToRouteInjectionToken } from "../../../common/front-end-routing/navigate-to-route-injection-token"; import type { LensMainExtension } from "../../../extensions/lens-main-extension"; -import trayMenuItemsInjectable from "../../../main/tray/tray-menu-item/tray-menu-items.injectable"; import type { LensExtension } from "../../../extensions/lens-extension"; import extensionInjectable from "../../../extensions/extension-loader/extension/extension.injectable"; @@ -92,7 +91,7 @@ export interface ApplicationBuilder { tray: { click: (id: string) => Promise; - get: (id: string) => TrayMenuItem | null; + get: (id: string) => MinimalTrayMenuItem | null; getIconPath: () => string; }; @@ -207,6 +206,8 @@ export const getApplicationBuilder = () => { let trayMenuIconPath: string; + const traySetMenuItemsMock = jest.fn(); + mainDi.override(electronTrayInjectable, () => ({ start: () => { const iconPaths = mainDi.inject(trayIconPathsInjectable); @@ -214,7 +215,7 @@ export const getApplicationBuilder = () => { trayMenuIconPath = iconPaths.normal; }, stop: () => {}, - setMenuItems: () => {}, + setMenuItems: traySetMenuItemsMock, setIconPath: (path) => { trayMenuIconPath = path; }, @@ -293,14 +294,21 @@ export const getApplicationBuilder = () => { tray: { get: (id: string) => { - const trayMenuItems = mainDi.inject(trayMenuItemsInjectable).get(); + const lastCall = last(traySetMenuItemsMock.mock.calls); - return trayMenuItems.find(matches({ id })) ?? null; + assert(lastCall); + + return lastCall[0].find(matches({ id })) ?? null; }, + getIconPath: () => trayMenuIconPath, click: async (id: string) => { - const trayMenuItems = mainDi.inject(trayMenuItemsInjectable).get(); + const lastCall = last(traySetMenuItemsMock.mock.calls); + + assert(lastCall); + + const trayMenuItems = lastCall[0]; const menuItem = trayMenuItems.find(matches({ id })) ?? null; @@ -315,7 +323,7 @@ export const getApplicationBuilder = () => { throw new Error(`Tried to click tray menu item with ID ${id} which does not exist. Available IDs are: "${availableIds}"`); } - if (!menuItem.enabled.get()) { + if (!menuItem.enabled) { throw new Error(`Tried to click tray menu item with ID ${id} which is disabled.`); }