From a78bbb5f6cbbfb60c6840fdd1fa7904565edae00 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 10 Nov 2020 10:10:19 -0500 Subject: [PATCH] Fix rendering of boolean values in CRDs (#1087) * Fix rendering of boolean values in CRDs - add optional special casing for boolean values in DrawerItems and TableRows since React (imo annoying fashion) does not render boolean values by default. - add a spinner on the sidebar for when the CRD menu is expeanded but the entries have not been loaded yet. - Add ability to double click a Badge to expand, also make it so that Badges highligh all text on first click. Signed-off-by: Sebastian Malton --- src/common/utils/saveToAppFiles.ts | 3 +- src/renderer/api/api-manager.ts | 2 +- src/renderer/api/endpoints/crd.api.ts | 7 +- .../crd-resource-details.tsx | 102 ++++++++++-------- .../+custom-resources/crd-resources.tsx | 13 +-- .../components/drawer/drawer-item.tsx | 15 +-- .../item-object-list/item-list-layout.tsx | 18 ++-- src/renderer/components/layout/sidebar.tsx | 35 +++--- src/renderer/components/spinner/spinner.scss | 8 +- src/renderer/components/spinner/spinner.tsx | 14 +-- src/renderer/components/table/table-cell.tsx | 11 +- .../utils/__tests__/display-booleans.test.tsx | 18 ++++ src/renderer/utils/display-booleans.ts | 15 +++ src/renderer/utils/index.ts | 1 + tsconfig.json | 6 +- 15 files changed, 167 insertions(+), 101 deletions(-) create mode 100644 src/renderer/utils/__tests__/display-booleans.test.tsx create mode 100644 src/renderer/utils/display-booleans.ts diff --git a/src/common/utils/saveToAppFiles.ts b/src/common/utils/saveToAppFiles.ts index 9092767ccf..e6fab1cfa9 100644 --- a/src/common/utils/saveToAppFiles.ts +++ b/src/common/utils/saveToAppFiles.ts @@ -1,7 +1,8 @@ // Save file to electron app directory (e.g. "/Users/$USER/Library/Application Support/Lens" for MacOS) import path from "path"; import { app, remote } from "electron"; -import { ensureDirSync, writeFileSync, WriteFileOptions } from "fs-extra"; +import { ensureDirSync, writeFileSync } from "fs-extra"; +import { WriteFileOptions } from "fs" export function saveToAppFiles(filePath: string, contents: any, options?: WriteFileOptions): string { const absPath = path.resolve((app || remote.app).getPath("userData"), filePath); diff --git a/src/renderer/api/api-manager.ts b/src/renderer/api/api-manager.ts index 0b066f37e1..a0d90f4a13 100644 --- a/src/renderer/api/api-manager.ts +++ b/src/renderer/api/api-manager.ts @@ -14,7 +14,7 @@ export class ApiManager { return this.apis.get(pathOrCallback) || this.apis.get(KubeApi.parseApi(pathOrCallback).apiBase); } - return Array.from(this.apis.values()).find(pathOrCallback); + return Array.from(this.apis.values()).find(pathOrCallback ?? ((api: KubeApi) => true)); } registerApi(apiBase: string, api: KubeApi) { diff --git a/src/renderer/api/endpoints/crd.api.ts b/src/renderer/api/endpoints/crd.api.ts index 7744276e74..7b561041f7 100644 --- a/src/renderer/api/endpoints/crd.api.ts +++ b/src/renderer/api/endpoints/crd.api.ts @@ -9,7 +9,7 @@ type AdditionalPrinterColumnsCommon = { description: string; } -type AdditionalPrinterColumnsV1 = AdditionalPrinterColumnsCommon & { +export type AdditionalPrinterColumnsV1 = AdditionalPrinterColumnsCommon & { jsonPath: string; } @@ -120,9 +120,9 @@ export class CustomResourceDefinition extends KubeObject { return JSON.stringify(this.spec.conversion); } - getPrinterColumns(ignorePriority = true) { + getPrinterColumns(ignorePriority = true): AdditionalPrinterColumnsV1[] { const columns = this.spec.versions.find(a => this.getVersion() == a.name)?.additionalPrinterColumns - ?? this.spec.additionalPrinterColumns?.map(({JSONPath, ...rest}) => ({ ...rest, jsonPath: JSONPath })) // map to V1 shape + ?? this.spec.additionalPrinterColumns?.map(({ JSONPath, ...rest }) => ({ ...rest, jsonPath: JSONPath })) // map to V1 shape ?? []; return columns .filter(column => column.name != "Age") @@ -149,4 +149,3 @@ export class CustomResourceDefinition extends KubeObject { export const crdApi = new VersionedKubeApi({ objectConstructor: CustomResourceDefinition }); - diff --git a/src/renderer/components/+custom-resources/crd-resource-details.tsx b/src/renderer/components/+custom-resources/crd-resource-details.tsx index 4525240396..883fba2fd4 100644 --- a/src/renderer/components/+custom-resources/crd-resource-details.tsx +++ b/src/renderer/components/+custom-resources/crd-resource-details.tsx @@ -12,67 +12,83 @@ import { KubeObjectDetailsProps } from "../kube-object"; import { crdStore } from "./crd.store"; import { KubeObjectMeta } from "../kube-object/kube-object-meta"; import { Input } from "../input"; -import { CustomResourceDefinition } from "../../api/endpoints/crd.api"; +import { AdditionalPrinterColumnsV1, CustomResourceDefinition } from "../../api/endpoints/crd.api"; interface Props extends KubeObjectDetailsProps { } -function CrdColumnValue({ value }: { value: any[] | {} | string }) { +function convertSpecValue(value: any): any { if (Array.isArray(value)) { - return <>{value.map((item, index) => )} + return value.map(convertSpecValue) } - if (typeof(value) === 'object') return ( - - ); - return {value}; + + if (typeof value === "object") { + return ( + + ) + } + + return value } + @observer export class CrdResourceDetails extends React.Component { @computed get crd() { return crdStore.getByObject(this.props.object); } + renderAdditionalColumns(crd: CustomResourceDefinition, columns: AdditionalPrinterColumnsV1[]) { + return columns.map(({ name, jsonPath: jp }) => ( + + {convertSpecValue(jsonPath.value(crd, jp.slice(1)))} + + )) + } + + renderStatus(crd: CustomResourceDefinition, columns: AdditionalPrinterColumnsV1[]) { + const showStatus = !columns.find(column => column.name == "Status") && crd.status?.conditions; + if (!showStatus) { + return null + } + + const conditions = crd.status.conditions + .filter(({ type, reason }) => type || reason) + .map(({ type, reason, message, status }) => ({ kind: type || reason, message, status })) + .map(({ kind, message, status }, index) => ( + + )) + + return ( + Status} className="status" labelsOnly> + {conditions} + + ) + } + render() { - const { object } = this.props; - const { crd } = this; - if (!object || !crd) return null; + const { props: { object }, crd } = this; + if (!object || !crd) { + return null; + } + const className = cssNames("CrdResourceDetails", crd.getResourceKind()); const extraColumns = crd.getPrinterColumns(); - const showStatus = !extraColumns.find(column => column.name == "Status") && object.status?.conditions; + return (
- - {extraColumns.map(column => { - const { name } = column; - const value = jsonPath.query(object, (column.jsonPath).slice(1)); - return ( - - - - ) - })} - {showStatus && ( - Status} className="status" labelsOnly> - {object.status.conditions.map((condition, index) => { - const { type, reason, message, status } = condition; - const kind = type || reason; - if (!kind) return null; - return ( - - ); - })} - - )} + + {this.renderAdditionalColumns(object, extraColumns)} + {this.renderStatus(object, extraColumns)}
) } diff --git a/src/renderer/components/+custom-resources/crd-resources.tsx b/src/renderer/components/+custom-resources/crd-resources.tsx index 3a7d726b59..e36fef532d 100644 --- a/src/renderer/components/+custom-resources/crd-resources.tsx +++ b/src/renderer/components/+custom-resources/crd-resources.tsx @@ -56,11 +56,11 @@ export class CrdResources extends React.Component { [sortBy.age]: (item: KubeObject) => item.metadata.creationTimestamp, } extraColumns.forEach(column => { - sortingCallbacks[column.name] = (item: KubeObject) => jsonPath.query(item, column.jsonPath.slice(1)) + sortingCallbacks[column.name] = (item: KubeObject) => jsonPath.value(item, column.jsonPath.slice(1)) }) - const ListView = KubeObjectListLayout; + return ( - { renderTableContents={(crdInstance: KubeObject) => [ crdInstance.getName(), isNamespaced && crdInstance.getNs(), - ...extraColumns.map(column => { - return jsonPath.query(crdInstance, (column.jsonPath).slice(1)) - }), + ...extraColumns.map(column => ({ + renderBoolean: true, + children: jsonPath.value(crdInstance, column.jsonPath.slice(1)), + })), crdInstance.getAge(), ]} /> diff --git a/src/renderer/components/drawer/drawer-item.tsx b/src/renderer/components/drawer/drawer-item.tsx index da38dd4a46..e50619fe5d 100644 --- a/src/renderer/components/drawer/drawer-item.tsx +++ b/src/renderer/components/drawer/drawer-item.tsx @@ -1,6 +1,6 @@ import "./drawer-item.scss"; import React from "react"; -import { cssNames } from "../../utils"; +import { cssNames, displayBooleans } from "../../utils"; export interface DrawerItemProps extends React.HTMLAttributes { name: React.ReactNode; @@ -8,18 +8,21 @@ export interface DrawerItemProps extends React.HTMLAttributes { title?: string; labelsOnly?: boolean; hidden?: boolean; + renderBoolean?: boolean; // show "true" or "false" for all of the children elements are "typeof boolean" } export class DrawerItem extends React.Component { render() { - const { name, title, labelsOnly, children, hidden, ...elemProps } = this.props - let { className } = this.props; + const { name, title, labelsOnly, children, hidden, className, renderBoolean, ...elemProps } = this.props if (hidden) return null - className = cssNames("DrawerItem", className, { labelsOnly }); + + const classNames = cssNames("DrawerItem", className, { labelsOnly }); + const content = displayBooleans(renderBoolean, children) + return ( -
+
{name} - {children} + {content}
) } diff --git a/src/renderer/components/item-object-list/item-list-layout.tsx b/src/renderer/components/item-object-list/item-list-layout.tsx index 0ff8201e05..fa99b3405b 100644 --- a/src/renderer/components/item-object-list/item-list-layout.tsx +++ b/src/renderer/components/item-object-list/item-list-layout.tsx @@ -235,7 +235,7 @@ export class ItemListLayout extends React.Component { cellProps.className = cssNames(cellProps.className, headCell.className); } } - return + return }) } {renderItemMenu && ( @@ -277,7 +277,7 @@ export class ItemListLayout extends React.Component { if (!isReady || !filters.length || hideFilters || !userSettings.showAppliedFilters) { return; } - return + return } renderNoItems() { @@ -297,7 +297,7 @@ export class ItemListLayout extends React.Component { ) } - return + return } renderHeaderContent(placeholders: IHeaderPlaceholders): ReactNode { @@ -344,12 +344,12 @@ export class ItemListLayout extends React.Component { title:
{title}
, info: this.renderInfo(), filters: <> - {!isClusterScoped && } + {!isClusterScoped && } + }} /> , - search: , + search: , } let header = this.renderHeaderContent(placeholders); if (customizeHeader) { @@ -381,7 +381,7 @@ export class ItemListLayout extends React.Component { return (
{!isReady && ( - + )} {isReady && ( { onClick={prevDefault(() => store.toggleSelectionAll(items))} /> )} - {renderTableHeader.map((cellProps, index) => )} - {renderItemMenu && } + {renderTableHeader.map((cellProps, index) => )} + {renderItemMenu && } )} { diff --git a/src/renderer/components/layout/sidebar.tsx b/src/renderer/components/layout/sidebar.tsx index 4619e48de7..8bce1a32c8 100644 --- a/src/renderer/components/layout/sidebar.tsx +++ b/src/renderer/components/layout/sidebar.tsx @@ -27,8 +27,9 @@ import { crdStore } from "../+custom-resources/crd.store"; import { CrdList, crdResourcesRoute, crdRoute, crdURL } from "../+custom-resources"; import { CustomResources } from "../+custom-resources/custom-resources"; import { navigation } from "../../navigation"; -import { isAllowedResource } from "../../../common/rbac" import { clusterPageRegistry } from "../../../extensions/registries/page-registry"; +import { isAllowedResource } from "../../../common/rbac"; +import { Spinner } from "../spinner"; const SidebarContext = React.createContext({ pinned: false }); type SidebarContextValue = { @@ -50,6 +51,10 @@ export class Sidebar extends React.Component { } renderCustomResources() { + if (crdStore.isLoading) { + return + } + return Object.entries(crdStore.groups).map(([group, crds]) => { const submenus = crds.map((crd) => { return { @@ -80,7 +85,7 @@ export class Sidebar extends React.Component {
- +
Lens
{ isHidden={!isAllowedResource("nodes")} url={clusterURL()} text={Cluster} - icon={} + icon={} /> Nodes} - icon={} + icon={} /> { routePath={workloadsRoute.path} subMenus={Workloads.tabRoutes} text={Workloads} - icon={} + icon={} /> { routePath={configRoute.path} subMenus={Config.tabRoutes} text={Configuration} - icon={} + icon={} /> { routePath={networkRoute.path} subMenus={Network.tabRoutes} text={Network} - icon={} + icon={} /> { url={storageURL({ query })} routePath={storageRoute.path} subMenus={Storage.tabRoutes} - icon={} + icon={} text={Storage} /> } + icon={} text={Namespaces} /> { isHidden={!isAllowedResource("events")} url={eventsURL({ query })} routePath={eventRoute.path} - icon={} + icon={} text={Events} /> { url={appsURL({ query })} subMenus={Apps.tabRoutes} routePath={appsRoute.path} - icon={} + icon={} text={Apps} /> { url={usersManagementURL({ query })} routePath={usersManagementRoute.path} subMenus={UserManagement.tabRoutes} - icon={} + icon={} text={Access Control} /> { url={crdURL()} subMenus={CustomResources.tabRoutes} routePath={crdRoute.path} - icon={} + icon={} text={Custom Resources} > {this.renderCustomResources()} @@ -194,7 +199,7 @@ export class Sidebar extends React.Component { url={url} routePath={path} text={title} - icon={} + icon={} /> ) })} @@ -257,7 +262,7 @@ class SidebarNavItem extends React.Component {
{icon} {text} - +
    {subMenus.map(({ title, url }) => ( diff --git a/src/renderer/components/spinner/spinner.scss b/src/renderer/components/spinner/spinner.scss index 802ef27bfc..b8843b542d 100644 --- a/src/renderer/components/spinner/spinner.scss +++ b/src/renderer/components/spinner/spinner.scss @@ -34,6 +34,12 @@ margin-top: calc(var(--spinner-size) / -2); } + &.centerHorizontal { + position: absolute; + left: 50%; + margin-left: calc(var(--spinner-size) / -2); + } + @keyframes rotate { 0% { transform: rotate(0deg); @@ -60,4 +66,4 @@ @include spinner-color(#4285F4); } } -} \ No newline at end of file +} diff --git a/src/renderer/components/spinner/spinner.tsx b/src/renderer/components/spinner/spinner.tsx index 771784a905..0edb2665b1 100644 --- a/src/renderer/components/spinner/spinner.tsx +++ b/src/renderer/components/spinner/spinner.tsx @@ -6,23 +6,19 @@ import { cssNames } from "../../utils"; export interface SpinnerProps extends React.HTMLProps { singleColor?: boolean; center?: boolean; + centerHorizontal?: boolean; } export class Spinner extends React.Component { - private elem: HTMLElement; - static defaultProps = { singleColor: true, center: false, }; render() { - const { center, singleColor, ...props } = this.props; - let { className } = this.props; - className = cssNames('Spinner', className, { - singleColor: singleColor, - center: center, - }); - return
    this.elem = e}/>; + const { center, singleColor, centerHorizontal, className, ...props } = this.props; + const classNames = cssNames('Spinner', className, { singleColor, center, centerHorizontal }); + + return
    ; } } diff --git a/src/renderer/components/table/table-cell.tsx b/src/renderer/components/table/table-cell.tsx index a56f144bc0..d278ad41d8 100644 --- a/src/renderer/components/table/table-cell.tsx +++ b/src/renderer/components/table/table-cell.tsx @@ -2,7 +2,7 @@ import "./table-cell.scss"; import type { TableSortBy, TableSortParams } from "./table"; import React, { ReactNode } from "react"; -import { autobind, cssNames } from "../../utils"; +import { autobind, cssNames, displayBooleans } from "../../utils"; import { Icon } from "../icon"; import { Checkbox } from "../checkbox"; @@ -13,6 +13,7 @@ export interface TableCellProps extends React.DOMAttributes { title?: ReactNode; checkbox?: boolean; // render cell with a checkbox isChecked?: boolean; // mark checkbox as checked or not + renderBoolean?: boolean; // show "true" or "false" for all of the children elements are "typeof boolean" sortBy?: TableSortBy; // column name, must be same as key in sortable object
_sorting?: Partial; //
sorting state, don't use this prop outside (!) _sort?(sortBy: TableSortBy): void; //
sort function, don't use this prop outside (!) @@ -52,20 +53,20 @@ export class TableCell extends React.Component { const { checkbox, isChecked } = this.props; const showCheckbox = isChecked !== undefined; if (checkbox && showCheckbox) { - return + return } } render() { - const { className, checkbox, isChecked, sortBy, _sort, _sorting, _nowrap, children, title, ...cellProps } = this.props; + const { className, checkbox, isChecked, sortBy, _sort, _sorting, _nowrap, children, title, renderBoolean: displayBoolean, ...cellProps } = this.props; const classNames = cssNames("TableCell", className, { checkbox: checkbox, nowrap: _nowrap, sorting: this.isSortable, }); - const content = title || children; + const content = displayBooleans(displayBoolean, title || children) return ( -
+
{this.renderCheckbox()} {_nowrap ?
{content}
: content} {this.renderSortIcon()} diff --git a/src/renderer/utils/__tests__/display-booleans.test.tsx b/src/renderer/utils/__tests__/display-booleans.test.tsx new file mode 100644 index 0000000000..29fdd0574b --- /dev/null +++ b/src/renderer/utils/__tests__/display-booleans.test.tsx @@ -0,0 +1,18 @@ +import React from "react" +import { displayBooleans } from "../display-booleans" + +describe("displayBooleans tests", () => { + it("should not do anything to div's if shouldShow is false", () => { + expect(displayBooleans(false,
)).toStrictEqual(
) + }) + + it("should not do anything to booleans's if shouldShow is false", () => { + expect(displayBooleans(false, true)).toStrictEqual(true) + expect(displayBooleans(false, false)).toStrictEqual(false) + }) + + it("should stringify booleans when shouldShow is true", () => { + expect(displayBooleans(true, true)).toStrictEqual("true") + expect(displayBooleans(true, false)).toStrictEqual("false") + }) +}) diff --git a/src/renderer/utils/display-booleans.ts b/src/renderer/utils/display-booleans.ts new file mode 100644 index 0000000000..c50b7cdf9b --- /dev/null +++ b/src/renderer/utils/display-booleans.ts @@ -0,0 +1,15 @@ +import React from "react" + +export function displayBooleans(shouldShow: boolean, from: React.ReactNode): React.ReactNode { + if (shouldShow) { + if (typeof from === "boolean") { + return from.toString() + } + + if (Array.isArray(from)) { + return from.map(node => displayBooleans(shouldShow, node)) + } + } + + return from +} diff --git a/src/renderer/utils/index.ts b/src/renderer/utils/index.ts index d8c9d692c4..0c2df22c91 100755 --- a/src/renderer/utils/index.ts +++ b/src/renderer/utils/index.ts @@ -18,3 +18,4 @@ export * from "./isReactNode" export * from "./convertMemory" export * from "./convertCpu" export * from "./metricUnitsToNumber" +export * from "./display-booleans" diff --git a/tsconfig.json b/tsconfig.json index 5a61e2ca3e..9cfbd543a8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,11 @@ "jsx": "react", "target": "ES2017", "module": "ESNext", - "lib": ["ESNext", "DOM", "DOM.Iterable"], + "lib": [ + "ESNext", + "DOM", + "DOM.Iterable" + ], "moduleResolution": "Node", "sourceMap": true, "strict": false,