From a5928d09cda42f97dcc9a1b876c13f9512b04b93 Mon Sep 17 00:00:00 2001 From: Janne Savolainen Date: Fri, 10 Mar 2023 15:34:43 +0200 Subject: [PATCH 1/2] Fix update button being visible when download for update fails (#7336) Signed-off-by: Janne Savolainen --- .../installing-update.test.ts.snap | 595 ++++++++++++++++++ .../installing-update-using-tray.test.ts.snap | 25 - .../installing-update.test.ts | 83 ++- .../download-update.injectable.ts | 8 +- 4 files changed, 676 insertions(+), 35 deletions(-) diff --git a/packages/core/src/features/application-update/__snapshots__/installing-update.test.ts.snap b/packages/core/src/features/application-update/__snapshots__/installing-update.test.ts.snap index 0b32c94831..cd0d55e25f 100644 --- a/packages/core/src/features/application-update/__snapshots__/installing-update.test.ts.snap +++ b/packages/core/src/features/application-update/__snapshots__/installing-update.test.ts.snap @@ -856,6 +856,601 @@ exports[`installing update when started when user checks for updates when new up `; exports[`installing update when started when user checks for updates when new update is discovered when download fails renders 1`] = ` + +
+
+
+
+
+ + + home + + +
+
+
+ + + arrow_back + + +
+
+
+ + + arrow_forward + + +
+
+
+
+
+
+
+
+ +
+
+

+ Welcome to some-product-name! +

+

+ To get you started we have auto-detected your clusters in your + + kubeconfig file and added them to the catalog, your centralized + + view for managing all your cloud-native resources. +
+
+ If you have any questions or feedback, please join our + + Lens Forums + + . +

+ +
+
+
+
+
+
+
+
+
+
+
+ Ca +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + arrow_left + + +
+
+ 1 +
+
+ + + arrow_right + + +
+
+
+
+
+
+
+
+
+ +`; + +exports[`installing update when started when user checks for updates when new update is discovered when download succeeds given checking for updates again when check resolves with different update that was previously downloaded when download resolves successfully renders 1`] = ` + +
+
+
+
+
+ + + home + + +
+
+
+ + + arrow_back + + +
+
+
+ + + arrow_forward + + +
+
+
+ +
+
+
+
+
+
+
+
+ +
+
+

+ Welcome to some-product-name! +

+

+ To get you started we have auto-detected your clusters in your + + kubeconfig file and added them to the catalog, your centralized + + view for managing all your cloud-native resources. +
+
+ If you have any questions or feedback, please join our + + Lens Forums + + . +

+ +
+
+
+
+
+
+
+
+
+
+
+ Ca +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + arrow_left + + +
+
+ 1 +
+
+ + + arrow_right + + +
+
+
+
+
+
+
+
+
+ +`; + +exports[`installing update when started when user checks for updates when new update is discovered when download succeeds given checking for updates again when check resolves with same update that is already downloaded renders 1`] = `
-
-
- -
diff --git a/packages/core/src/features/application-update/installing-update.test.ts b/packages/core/src/features/application-update/installing-update.test.ts index 719bb879bf..88ccb8ce0f 100644 --- a/packages/core/src/features/application-update/installing-update.test.ts +++ b/packages/core/src/features/application-update/installing-update.test.ts @@ -18,10 +18,11 @@ import setUpdateOnQuitInjectable from "../../main/electron-app/features/set-upda import processCheckingForUpdatesInjectable from "./main/process-checking-for-updates.injectable"; import { testUsingFakeTime } from "../../test-utils/use-fake-time"; import staticFilesDirectoryInjectable from "../../common/vars/static-files-directory.injectable"; +import electronQuitAndInstallUpdateInjectable from "../../main/electron-app/features/electron-quit-and-install-update.injectable"; describe("installing update", () => { let builder: ApplicationBuilder; - let quitAndInstallUpdateMock: jest.Mock; + let electronQuitAndInstallUpdateMock: jest.Mock; let checkForPlatformUpdatesMock: AsyncFnMock; let downloadPlatformUpdateMock: AsyncFnMock; let setUpdateOnQuitMock: jest.Mock; @@ -32,7 +33,7 @@ describe("installing update", () => { builder = getApplicationBuilder(); builder.beforeApplicationStart((mainDi) => { - quitAndInstallUpdateMock = jest.fn(); + electronQuitAndInstallUpdateMock = jest.fn(); checkForPlatformUpdatesMock = asyncFn(); downloadPlatformUpdateMock = asyncFn(); setUpdateOnQuitMock = jest.fn(); @@ -52,8 +53,8 @@ describe("installing update", () => { ); mainDi.override( - quitAndInstallUpdateInjectable, - () => quitAndInstallUpdateMock, + electronQuitAndInstallUpdateInjectable, + () => electronQuitAndInstallUpdateMock, ); mainDi.override(electronUpdaterIsActiveInjectable, () => true); @@ -64,6 +65,7 @@ describe("installing update", () => { describe("when started", () => { let rendered: RenderResult; let processCheckingForUpdates: (source: string) => Promise<{ updateIsReadyToBeInstalled: boolean }>; + let quitAndInstallUpdate: () => void; beforeEach(async () => { rendered = await builder.render(); @@ -71,6 +73,10 @@ describe("installing update", () => { processCheckingForUpdates = builder.mainDi.inject( processCheckingForUpdatesInjectable, ); + + quitAndInstallUpdate = builder.mainDi.inject( + quitAndInstallUpdateInjectable, + ); }); it("renders", () => { @@ -155,7 +161,7 @@ describe("installing update", () => { }); it("does not quit and install update yet", () => { - expect(quitAndInstallUpdateMock).not.toHaveBeenCalled(); + expect(electronQuitAndInstallUpdateMock).not.toHaveBeenCalled(); }); it("still shows normal tray icon", () => { @@ -167,6 +173,12 @@ describe("installing update", () => { it("renders", () => { expect(rendered.baseElement).toMatchSnapshot(); }); + + it("does not show the update button", () => { + const button = rendered.queryByTestId("update-button"); + + expect(button).not.toBeInTheDocument(); + }); }); describe("when download succeeds", () => { @@ -175,7 +187,7 @@ describe("installing update", () => { }); it("does not quit and install update yet", () => { - expect(quitAndInstallUpdateMock).not.toHaveBeenCalled(); + expect(electronQuitAndInstallUpdateMock).not.toHaveBeenCalled(); }); it("shows tray icon for update being available", () => { @@ -218,6 +230,26 @@ describe("installing update", () => { "/some-static-files-directory/build/tray/trayIconUpdateAvailableTemplate.png", ); }); + + it("does not quit and install update yet", () => { + expect(electronQuitAndInstallUpdateMock).not.toHaveBeenCalled(); + }); + + it("renders", () => { + expect(rendered.baseElement).toMatchSnapshot(); + }); + + it("shows the update button", () => { + const button = rendered.getByTestId("update-button"); + + expect(button).toBeInTheDocument(); + }); + + it("when triggering the update, quits and installs the update", () => { + quitAndInstallUpdate(); + + expect(electronQuitAndInstallUpdateMock).toHaveBeenCalled(); + }); }); describe("when check resolves with different update that was previously downloaded", () => { @@ -237,6 +269,45 @@ describe("installing update", () => { "/some-static-files-directory/build/tray/trayIconCheckingForUpdatesTemplate.png", ); }); + + it("still shows the update button", () => { + const button = rendered.getByTestId("update-button"); + + expect(button).toBeInTheDocument(); + }); + + describe("when download resolves successfully", () => { + beforeEach(async () => { + await downloadPlatformUpdateMock.resolve({ downloadWasSuccessful: true }); + }); + + it("still shows the update button", () => { + const button = + rendered.getByTestId("update-button"); + + expect(button).toBeInTheDocument(); + }); + + it("does not quit and install update yet", () => { + expect(electronQuitAndInstallUpdateMock).not.toHaveBeenCalled(); + }); + + it("shows tray icon for update being available", () => { + expect(builder.tray.getIconPath()).toBe( + "/some-static-files-directory/build/tray/trayIconUpdateAvailableTemplate.png", + ); + }); + + it("renders", () => { + expect(rendered.baseElement).toMatchSnapshot(); + }); + + it("when triggering the update, quits and installs the update", () => { + quitAndInstallUpdate(); + + expect(electronQuitAndInstallUpdateMock).toHaveBeenCalled(); + }); + }); }); }); }); diff --git a/packages/core/src/features/application-update/main/download-update/download-update.injectable.ts b/packages/core/src/features/application-update/main/download-update/download-update.injectable.ts index e9019dc7b2..662a2b272d 100644 --- a/packages/core/src/features/application-update/main/download-update/download-update.injectable.ts +++ b/packages/core/src/features/application-update/main/download-update/download-update.injectable.ts @@ -40,12 +40,12 @@ const downloadUpdateInjectable = getInjectable({ if (!downloadWasSuccessful) { progressOfUpdateDownload.set({ percentage: 0, failed: "Download of update failed" }); discoveredVersionState.set(null); + } else { + const currentDateTime = getCurrentDateTime(); + + updateDownloadedDate.set(currentDateTime); } - const currentDateTime = getCurrentDateTime(); - - updateDownloadedDate.set(currentDateTime); - downloadingUpdateState.set(false); }); From 34aed58ff0818c7db994d7f5c977807196702bd5 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 10 Mar 2023 15:34:52 +0200 Subject: [PATCH 2/2] Better displaying container's info for Pods (#7331) * Better displaying container's info for Pods and all tooltips with `formatters={{tableView: true}}` Signed-off-by: Roman * fix: keep containerID on a single line Signed-off-by: Roman * nope, i can :P (make it perfect) Signed-off-by: Roman --------- Signed-off-by: Roman --- .../src/common/k8s-api/endpoints/pod.api.ts | 4 +- .../components/+workloads-pods/pods.tsx | 67 ++++++++++--------- .../renderer/components/tooltip/tooltip.scss | 23 +++++-- 3 files changed, 53 insertions(+), 41 deletions(-) diff --git a/packages/core/src/common/k8s-api/endpoints/pod.api.ts b/packages/core/src/common/k8s-api/endpoints/pod.api.ts index d0c0178d8d..1db41957b2 100644 --- a/packages/core/src/common/k8s-api/endpoints/pod.api.ts +++ b/packages/core/src/common/k8s-api/endpoints/pod.api.ts @@ -80,6 +80,8 @@ export interface ContainerState { terminated?: ContainerStateTerminated; } +export type ContainerStateValues = Partial; + export interface PodContainerStatus { name: string; state?: ContainerState; @@ -649,7 +651,7 @@ export class Pod extends KubeObject< .filter(({ name }) => runningContainerNames.has(name)); } - getContainerStatuses(includeInitContainers = true) { + getContainerStatuses(includeInitContainers = true): PodContainerStatus[] { const { containerStatuses = [], initContainerStatuses = [] } = this.status ?? {}; if (includeInitContainers) { diff --git a/packages/core/src/renderer/components/+workloads-pods/pods.tsx b/packages/core/src/renderer/components/+workloads-pods/pods.tsx index 3f310c2b76..849a101ae1 100644 --- a/packages/core/src/renderer/components/+workloads-pods/pods.tsx +++ b/packages/core/src/renderer/components/+workloads-pods/pods.tsx @@ -5,11 +5,11 @@ import "./pods.scss"; -import React, { Fragment } from "react"; +import React from "react"; import { observer } from "mobx-react"; import { Link } from "react-router-dom"; import { KubeObjectListLayout } from "../kube-object-list-layout"; -import type { NodeApi, Pod } from "../../../common/k8s-api/endpoints"; +import type { ContainerStateValues, NodeApi, Pod } from "../../../common/k8s-api/endpoints"; import { StatusBrick } from "../status-brick"; import { cssNames, getConvertedParts, object, stopPropagation } from "@k8slens/utilities"; import startCase from "lodash/startCase"; @@ -21,8 +21,8 @@ import { SiblingsInTabLayout } from "../layout/siblings-in-tab-layout"; import { KubeObjectAge } from "../kube-object/age"; import { withInjectables } from "@ogre-tools/injectable-react"; import type { GetDetailsUrl } from "../kube-detail-params/get-details-url.injectable"; -import apiManagerInjectable from "../../../common/k8s-api/api-manager/manager.injectable"; import getDetailsUrlInjectable from "../kube-detail-params/get-details-url.injectable"; +import apiManagerInjectable from "../../../common/k8s-api/api-manager/manager.injectable"; import type { EventStore } from "../+events/store"; import type { PodStore } from "./store"; import nodeApiInjectable from "../../../common/k8s-api/endpoints/node.api.injectable"; @@ -52,8 +52,12 @@ interface Dependencies { @observer class NonInjectedPods extends React.Component { - renderState(name: string, ready: boolean, key: string, data: Partial> | undefined) { - return data && ( + renderState(name: string, ready: boolean, key: string, data?: ContainerStateValues) { + if (!data) { + return; + } + + return ( <>
{name} @@ -64,40 +68,37 @@ class NonInjectedPods extends React.Component {
{object.entries(data).map(([name, value]) => ( -
-
- {startCase(name)} -
-
- {value} -
-
+ +
{startCase(name)}
+
{value}
+ ))} ); } renderContainersStatus(pod: Pod) { - return pod.getContainerStatuses() - .map(({ name, state = {}, ready }) => ( - - - {this.renderState(name, ready, "running", state.running)} - {this.renderState(name, ready, "waiting", state.waiting)} - {this.renderState(name, ready, "terminated", state.terminated)} - - ), - }} - /> - - )); + return pod.getContainerStatuses().map(({ name, state, ready }) => { + return ( + + {this.renderState(name, ready, "running", state?.running)} + {this.renderState(name, ready, "waiting", state?.waiting)} + {this.renderState(name, ready, "terminated", state?.terminated)} + + ), + }} + /> + ); + }); } render() { diff --git a/packages/core/src/renderer/components/tooltip/tooltip.scss b/packages/core/src/renderer/components/tooltip/tooltip.scss index 10b7669679..d90d91f52e 100644 --- a/packages/core/src/renderer/components/tooltip/tooltip.scss +++ b/packages/core/src/renderer/components/tooltip/tooltip.scss @@ -63,27 +63,36 @@ } &.tableView { - min-width: 200px; + display: grid; + gap: var(--padding); + grid-template-columns: max-content 1fr; + grid-template-rows: repeat(2, 1fr); - > :not(:last-child) { - margin-bottom: var(--flex-gap); + // backward compatibility: skips element in DOM to consider only children in grid-flow + > .flex { + display: contents; + } + + > * { + white-space: pre-wrap; + word-break: break-word; } .title { + grid-column: 1 / 3; // merge color: var(--textColorAccent); text-align: center; + font-weight: bold; } .name { - color: var(--textColorAccent); text-align: right; - flex: 0 0 35%; + color: var(--textColorAccent); } .value { text-align: left; - max-width: 300px; - word-break: break-word; + color: var(--textColorSecondary); } } }