From 6248f7bde81b6ffa88473272e79c10b0120ba59c 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 | 47 ++++-- .../src/databasesPage/databaseTableCells.tsx | 27 ++-- .../databasesPage/databasesPage.module.scss | 9 ++ .../src/databasesPage/databasesPage.tsx | 49 ++++--- 10 files changed, 375 insertions(+), 160 deletions(-) rename pkg/ui/workspaces/cluster-ui/src/databases/{util.spec.ts => util.spec.tsx} (72%) 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 52147407a229..940221186204 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx @@ -46,7 +46,7 @@ import { TableSchemaDetailsRow, TableSpanStatsRow, } from "../api"; -import { checkInfoAvailable } from "../databases"; +import { LoadingCell } from "../databases"; import { Timestamp, Timezone } from "../timestamp"; import { Search } from "../search"; import { Loading } from "../loading"; @@ -562,12 +562,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", @@ -581,12 +585,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", @@ -619,15 +627,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", @@ -652,16 +664,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", }, { @@ -673,16 +688,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", @@ -721,12 +740,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", @@ -737,12 +760,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", @@ -753,12 +780,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 2016352e9ed4..70d84d2c1aea 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx @@ -25,7 +25,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"; import { DatabaseIcon } from "../icon/databaseIcon"; import styles from "./databaseDetailsPage.module.scss"; @@ -45,13 +45,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} + + } ); }; @@ -111,11 +116,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 630f1ed70c00..f88834c1409e 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx @@ -52,7 +52,7 @@ import { TableSchemaDetailsRow, TableSpanStatsRow, } from "../api"; -import { checkInfoAvailable } from "../databases"; +import { LoadingCell } from "../databases"; import { ActionCell, @@ -469,55 +469,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 != @@ -552,14 +579,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 72% 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 9e3a6ba2c31c..234a8a37be51 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 { @@ -17,6 +20,7 @@ import { getNodeIdsFromStoreIds, normalizePrivileges, normalizeRoles, + LoadingCell, } from "./util"; describe("Getting nodes by region string", () => { @@ -174,3 +178,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 734f7113ea24..e7be98df2d33 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx @@ -10,7 +10,8 @@ import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import React from "react"; -import { Tooltip } from "antd"; +import { Skeleton, Tooltip } from "antd"; +import { Caution } from "@cockroachlabs/icons"; import { isMaxSizeError, @@ -240,34 +241,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 3e8c4a1e1c72..7fb06cad6c38 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 { StackIcon } from "../icon/stackIcon"; import { CockroachCloudContext } from "../contexts"; import { - checkInfoAvailable, + LoadingCell, getNetworkErrorMessage, getQueryErrorMessage, } from "../databases"; @@ -34,19 +34,18 @@ interface CellProps { database: DatabasesPageDataDatabase; } -export const DiskSizeCell = ({ database }: CellProps): JSX.Element => { - return ( - <> - {checkInfoAvailable( - database.spanStatsRequestError, - 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 b9362859d1e0..852f8ea57e53 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 9e5d8ee010ae..fb004c1fcde5 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx @@ -51,7 +51,7 @@ import { SqlApiQueryResponse, SqlExecutionErrorMessage, } from "../api"; -import { checkInfoAvailable } from "../databases"; +import { LoadingCell } from "../databases"; import { DatabaseNameCell, @@ -562,12 +562,16 @@ export class DatabasesPage extends React.Component< Tables ), - cell: database => - checkInfoAvailable( - database.detailsRequestError, - 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", @@ -581,12 +585,16 @@ export class DatabasesPage extends React.Component< Range Count ), - cell: database => - checkInfoAvailable( - database.spanStatsRequestError, - 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", @@ -600,12 +608,15 @@ export class DatabasesPage extends React.Component< {this.props.isTenant ? "Regions" : "Regions/Nodes"} ), - cell: database => - checkInfoAvailable( - database.detailsRequestError, - 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",