From 4b00756c4065ea7f2702e3935433368667f71cab Mon Sep 17 00:00:00 2001 From: David Hartunian Date: Fri, 12 Jul 2024 13:10:49 -0400 Subject: [PATCH] ui: add loading skeletons to databases and tables pages The databases and tables pages can take a long time to load data on clusters with large sizes. The current user experience did not clearly signal which pieces of information were in progress or missing. This change updates each cell to show a skeleton as it waits for data from the backend. If there's no data returned we will show the text "No Data" instead of "(unavailable)". The `checkInfoAvailable` helper function is converted into a functional React component for ease of testing and code uniformity. We add styling to ensure that warning icons within cells are aligned properly and colored the orange color we use elsewhere. Also removes the narrow styling of the "% of Live Data" column. Resolves: #124153 Epic: CRDB-37558 Release note (ui change): The databases and tables pages in the DB Console will show a loading state while loading information for databases and tables including size and range counts. --- .../databaseDetailsPage.module.scss | 9 ++ .../databaseDetailsPage.tsx | 137 +++++++++++------- .../src/databaseDetailsPage/tableCells.tsx | 36 +++-- .../databaseTablePage.module.scss | 9 ++ .../databaseTablePage/databaseTablePage.tsx | 137 +++++++++++------- .../databases/{util.spec.ts => util.spec.tsx} | 75 ++++++++++ .../cluster-ui/src/databases/util.tsx | 49 +++++-- .../src/databasesPage/databaseTableCells.tsx | 27 ++-- .../databasesPage/databasesPage.module.scss | 9 ++ .../src/databasesPage/databasesPage.tsx | 49 ++++--- 10 files changed, 377 insertions(+), 160 deletions(-) rename pkg/ui/workspaces/cluster-ui/src/databases/{util.spec.ts => util.spec.tsx} (71%) diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.module.scss b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.module.scss index 77c1abf2dc2e..6293d67aa9ae 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.module.scss @@ -30,6 +30,15 @@ &__no-result { @include text--body-strong; } + + &__cell-error { + display: inline-flex; + align-items: center; + gap: 10px; + svg { + fill: $colors--warning; + } + } } .sorted-table { diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx index 6777d06c18d8..f0d1bae2da71 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx @@ -59,7 +59,7 @@ import { TableSchemaDetailsRow, TableSpanStatsRow, } from "../api"; -import { checkInfoAvailable } from "../databases"; +import { LoadingCell } from "../databases"; import { InlineAlert } from "@cockroachlabs/ui-components"; const cx = classNames.bind(styles); @@ -563,12 +563,16 @@ export class DatabaseDetailsPage extends React.Component< Ranges ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.spanStats?.error, - table.details.spanStats?.range_count, - ), + cell: table => ( + + {table.details.spanStats?.range_count} + + ), sort: table => table.details.spanStats?.range_count, className: cx("database-table__col-range-count"), name: "rangeCount", @@ -582,12 +586,16 @@ export class DatabaseDetailsPage extends React.Component< Columns ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.schemaDetails?.error, - table.details.schemaDetails?.columns?.length, - ), + cell: table => ( + + {table.details.schemaDetails?.columns?.length} + + ), sort: table => table.details.schemaDetails?.columns?.length, className: cx("database-table__col-column-count"), name: "columnCount", @@ -620,15 +628,19 @@ export class DatabaseDetailsPage extends React.Component< Regions ), - cell: table => - checkInfoAvailable( - table.requestError, - null, - table.details.nodesByRegionString && - table.details.nodesByRegionString.length > 0 + cell: table => ( + + {table.details.nodesByRegionString && + table.details.nodesByRegionString.length > 0 ? table.details.nodesByRegionString - : null, - ), + : null} + + ), sort: table => table.details.nodesByRegionString, className: cx("database-table__col--regions"), name: "regions", @@ -653,16 +665,19 @@ export class DatabaseDetailsPage extends React.Component< % of Live Data ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.spanStats?.error, - table.details.spanStats ? ( + cell: table => ( + + {table.details.spanStats ? ( - ) : null, - ), + ) : null} + + ), sort: table => table.details.spanStats?.live_percentage, - className: cx("database-table__col-column-count"), name: "livePercentage", }, { @@ -674,16 +689,20 @@ export class DatabaseDetailsPage extends React.Component< Table Stats Last Updated ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.statsLastUpdated?.error, + cell: table => ( + , - ), + /> + + ), sort: table => table.details.statsLastUpdated, className: cx("database-table__col--table-stats"), name: "tableStatsUpdated", @@ -722,12 +741,16 @@ export class DatabaseDetailsPage extends React.Component< Users ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.grants?.error, - table.details.grants?.roles.length, - ), + cell: table => ( + + {table.details.grants?.roles.length} + + ), sort: table => table.details.grants?.roles.length, className: cx("database-table__col-user-count"), name: "userCount", @@ -738,12 +761,16 @@ export class DatabaseDetailsPage extends React.Component< Roles ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.grants?.error, - table.details.grants?.roles.join(", "), - ), + cell: table => ( + + {table.details.grants?.roles.join(", ")} + + ), sort: table => table.details.grants?.roles.join(", "), className: cx("database-table__col-roles"), name: "roles", @@ -754,12 +781,16 @@ export class DatabaseDetailsPage extends React.Component< Grants ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.grants?.error, - table.details.grants?.privileges.join(", "), - ), + cell: table => ( + + {table.details.grants?.privileges.join(", ")} + + ), sort: table => table.details.grants?.privileges?.join(", "), className: cx("database-table__col-grants"), name: "grants", diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx index 5cdff39636c6..7b73b5e8a71a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx @@ -33,7 +33,7 @@ import * as format from "../util/format"; import { Breadcrumbs } from "../breadcrumbs"; import { CaretRight } from "../icon/caretRight"; import { CockroachCloudContext } from "../contexts"; -import { checkInfoAvailable, getNetworkErrorMessage } from "../databases"; +import { LoadingCell, getNetworkErrorMessage } from "../databases"; const cx = classNames.bind(styles); @@ -44,13 +44,18 @@ export const DiskSizeCell = ({ }): JSX.Element => { return ( <> - {checkInfoAvailable( - table.requestError, - table.details?.spanStats?.error, - table.details?.spanStats?.approximate_disk_bytes - ? format.Bytes(table.details?.spanStats?.approximate_disk_bytes) - : null, - )} + { + + {table.details?.spanStats?.approximate_disk_bytes + ? format.Bytes(table.details?.spanStats?.approximate_disk_bytes) + : null} + + } ); }; @@ -110,11 +115,16 @@ export const IndexesCell = ({ }): JSX.Element => { const elem = ( <> - {checkInfoAvailable( - table.requestError, - table.details?.schemaDetails?.error, - table.details?.schemaDetails?.indexes?.length, - )} + { + + {table.details?.schemaDetails?.indexes?.length} + + } ); // If index recommendations are not enabled or we don't have any index recommendations, diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.module.scss b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.module.scss index f685f2248f52..af6569d122af 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.module.scss @@ -40,6 +40,15 @@ @include text--body; overflow-wrap: anywhere; } + + &__error-cell{ + display: inline-flex; + align-items: center; + gap: 10px; + svg { + fill: $colors--warning; + } + } } .icon { diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx index bfaa4b3cb30e..1f04108d1b36 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx @@ -68,7 +68,7 @@ import { TableSchemaDetailsRow, TableSpanStatsRow, } from "../api"; -import { checkInfoAvailable } from "../databases"; +import { LoadingCell } from "../databases"; const cx = classNames.bind(styles); const booleanSettingCx = classnames.bind(booleanSettingStyles); @@ -472,55 +472,82 @@ export class DatabaseTablePage extends React.Component< + {details.spanStats?.approximate_disk_bytes + ? format.Bytes( + details.spanStats?.approximate_disk_bytes, + ) + : null} + + } /> + {details.replicaData?.replicaCount} + + } /> + {details.spanStats?.range_count} + + } /> , - )} + value={ + + + + } /> {details.statsLastUpdated && ( , - )} + value={ + + + + } /> )} {this.props.automaticStatsCollectionEnabled != @@ -555,14 +582,21 @@ export class DatabaseTablePage extends React.Component< {this.props.showNodeRegionsSection && ( + {details.nodesByRegionString && details.nodesByRegionString?.length - ? details.nodesByRegionString - : null, - )} + ? details.nodesByRegionString + : null} + + } /> )} + {details.indexData?.indexes?.join(", ")} + + } className={cx( "database-table-page__indexes--value", )} diff --git a/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.ts b/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.tsx similarity index 71% rename from pkg/ui/workspaces/cluster-ui/src/databases/util.spec.ts rename to pkg/ui/workspaces/cluster-ui/src/databases/util.spec.tsx index e9c0c04e528d..7f4ee73954b5 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.tsx @@ -8,6 +8,9 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +import React from "react"; +import { render } from "@testing-library/react"; + import { INodeStatus } from "../util"; import { Nodes, @@ -16,6 +19,7 @@ import { getNodeIdsFromStoreIds, normalizePrivileges, normalizeRoles, + LoadingCell, } from "./util"; describe("Getting nodes by region string", () => { @@ -173,3 +177,74 @@ describe("Normalize roles", () => { expect(result).toEqual(["admin", "public"]); }); }); + +describe("LoadingCell", () => { + it("renders empty data", () => { + const { getByText } = render( + + {null} + , + ); + + expect(getByText("No data")).not.toBeNull(); + }); + it("renders with undefined children", () => { + const { getByText } = render( + , + ); + + expect(getByText("No data")).not.toBeNull(); + }); + it("renders skeleton heading when loading", () => { + const { getByRole } = render( + + {null} + , + ); + + expect(getByRole("heading")).not.toBeNull(); + }); + it("renders error name and status icon", () => { + const { getByRole } = render( + + {null} + , + ); + + // TODO(davidh): rendering of antd Tooltip component doesn't work + // here and hence can't be directly tested to contain the error + // name. + expect(getByRole("status")).not.toBeNull(); + }); + it("renders children with no error", () => { + const { getByText } = render( + +
inner data
+ , + ); + + expect(getByText("inner data")).not.toBeNull(); + }); + it("renders children with error together", () => { + const { getByText, getByRole } = render( + +
inner data
+ , + ); + + expect(getByRole("status")).not.toBeNull(); + expect(getByText("inner data")).not.toBeNull(); + }); +}); diff --git a/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx b/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx index 5a9340ce97f8..d07707717f77 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx @@ -10,8 +10,11 @@ import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import React from "react"; -import { Tooltip } from "antd"; +import { Skeleton, Tooltip } from "antd"; +import "antd/lib/skeleton/style"; import "antd/lib/tooltip/style"; +import { Caution } from "@cockroachlabs/icons"; + import { isMaxSizeError, isPrivilegeError, @@ -240,34 +243,56 @@ export function buildIndexStatToRecommendationsMap( return recommendationsMap; } -export function checkInfoAvailable( - requestError: Error, - queryError: Error, - cell: React.ReactNode, -): React.ReactNode { +interface LoadingCellProps { + requestError: Error; + queryError?: Error; + loading: boolean; + errorClassName: string; +} + +export const LoadingCell: React.FunctionComponent = ({ + loading, + requestError, + queryError, + errorClassName, + children, +}) => { + if (loading) { + return ( + + ); + } + let tooltipMsg = ""; if (requestError) { tooltipMsg = `Encountered a network error fetching data for this cell: ${requestError.name}`; } else if (queryError) { tooltipMsg = getQueryErrorMessage(queryError); - } else if (cell == null) { - tooltipMsg = "Empty result"; } + + let childrenOrNoData = <>{children}; + if (children == null) { + childrenOrNoData = <>{"No data"}; + } + // If we encounter an error gathering data for this cell, - // render it "unavailable" with a tooltip message for the error. + // render a warning icon with a tooltip message for the error. if (tooltipMsg !== "") { return ( - (unavailable) + + {childrenOrNoData} ); + } else { + return childrenOrNoData; } - return cell; -} +}; export const getNetworkErrorMessage = (requestError: Error): string => { return `Encountered a network error: ${requestError.message}`; diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databaseTableCells.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databaseTableCells.tsx index 8dc1d19e748e..5015018937d6 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databaseTableCells.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databaseTableCells.tsx @@ -19,7 +19,7 @@ import { EncodeDatabaseUri } from "../util"; import { Link } from "react-router-dom"; import { StackIcon } from "../icon/stackIcon"; import { CockroachCloudContext } from "../contexts"; -import { checkInfoAvailable, getNetworkErrorMessage } from "../databases"; +import { LoadingCell, getNetworkErrorMessage } from "../databases"; import * as format from "../util/format"; import { Caution } from "@cockroachlabs/icons"; @@ -29,19 +29,18 @@ interface CellProps { database: DatabasesPageDataDatabase; } -export const DiskSizeCell = ({ database }: CellProps): JSX.Element => { - return ( - <> - {checkInfoAvailable( - database.requestError, - database.spanStats?.error, - database.spanStats?.approximate_disk_bytes - ? format.Bytes(database.spanStats?.approximate_disk_bytes) - : null, - )} - - ); -}; +export const DiskSizeCell = ({ database }: CellProps) => ( + + {database.spanStats?.approximate_disk_bytes + ? format.Bytes(database.spanStats?.approximate_disk_bytes) + : null} + +); export const IndexRecCell = ({ database }: CellProps): JSX.Element => { const text = diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.module.scss b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.module.scss index 4e076c6faeb2..f1e8981fd471 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.module.scss @@ -31,6 +31,15 @@ &__no-result { @include text--body-strong; } + + &__cell-error { + display: inline-flex; + align-items: center; + gap: 10px; + svg { + fill: $colors--warning; + } + } } .sorted-table { diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx index 9788df712ffc..50d58fe67377 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx @@ -58,7 +58,7 @@ import { SqlExecutionErrorMessage, } from "../api"; import { InlineAlert } from "@cockroachlabs/ui-components"; -import { checkInfoAvailable } from "../databases"; +import { LoadingCell } from "../databases"; const cx = classNames.bind(styles); const sortableTableCx = classNames.bind(sortableTableStyles); @@ -554,12 +554,16 @@ export class DatabasesPage extends React.Component< Tables ), - cell: database => - checkInfoAvailable( - database.requestError, - database.tables?.error, - database.tables?.tables?.length, - ), + cell: database => ( + + {database.tables?.tables?.length} + + ), sort: database => database.tables?.tables.length ?? 0, className: cx("databases-table__col-table-count"), name: "tableCount", @@ -573,12 +577,16 @@ export class DatabasesPage extends React.Component< Range Count ), - cell: database => - checkInfoAvailable( - database.requestError, - database.spanStats?.error, - database.spanStats?.range_count, - ), + cell: database => ( + + {database.spanStats?.range_count} + + ), sort: database => database.spanStats?.range_count, className: cx("databases-table__col-range-count"), name: "rangeCount", @@ -592,12 +600,15 @@ export class DatabasesPage extends React.Component< {this.props.isTenant ? "Regions" : "Regions/Nodes"} ), - cell: database => - checkInfoAvailable( - database.requestError, - null, - database.nodesByRegionString ? database.nodesByRegionString : null, - ), + cell: database => ( + + {database.nodesByRegionString ? database.nodesByRegionString : null} + + ), sort: database => database.nodesByRegionString, className: cx("databases-table__col-node-regions"), name: "nodeRegions",