extending table component features (#47030)

* extending table component features

* adding unit tests

* PR Feedback

* PR Feedback

* unit test adjustment

enterprise/frontend/src/metabase-enterprise/upload_management/UploadManagement.unit.spec.tsx

@@ -80,11 +80,11 @@ describe("uploadManagementTable", () => {
     const dateColumn = screen.getByText("Created at");
 
     await userEvent.click(dateColumn);
-    expect(screen.getByLabelText("chevrondown icon")).toBeInTheDocument();
+    expect(screen.getByLabelText("chevronup icon")).toBeInTheDocument();
     expect(getFirstRow()).toHaveTextContent(/Uploaded Table 99/);
 
     await userEvent.click(dateColumn);
-    expect(screen.getByLabelText("chevronup icon")).toBeInTheDocument();
+    expect(screen.getByLabelText("chevrondown icon")).toBeInTheDocument();
     expect(getFirstRow()).toHaveTextContent(/Uploaded Table 2/);
   });
 

enterprise/frontend/src/metabase-enterprise/upload_management/UploadManagementTable.tsx

@@ -2,7 +2,7 @@ import { useCallback, useMemo, useState } from "react";
 import { msgid, ngettext, t } from "ttag";
 
 import SettingHeader from "metabase/admin/settings/components/SettingHeader";
-import { StyledTable } from "metabase/common/components/Table";
+import { ClientSortableTable } from "metabase/common/components/Table";
 import {
   BulkActionBar,
   BulkActionButton,
@@ -39,7 +39,7 @@ export function UploadManagementTable() {
   // TODO: once we have uploads running through RTK Query, we can remove the force update
   // because we can properly invalidate the tables tag
   const {
-    data: uploadTables,
+    data: uploadTables = [],
     error,
     isLoading,
   } = useListUploadTablesQuery(undefined, {
@@ -149,7 +149,7 @@ export function UploadManagementTable() {
       <Text fw="bold" color="text-medium">
         {t`Uploaded Tables`}
       </Text>
-      <StyledTable
+      <ClientSortableTable
         data-testid="upload-tables-table"
         columns={columns}
         rows={uploadTables}

frontend/src/metabase/admin/settings/components/ApiKeys/ManageApiKeys.tsx

@@ -2,7 +2,7 @@ import { useMemo, useState } from "react";
 import { t } from "ttag";
 
 import { useListApiKeysQuery } from "metabase/api";
-import { StyledTable } from "metabase/common/components/Table";
+import { ClientSortableTable } from "metabase/common/components/Table";
 import { DelayedLoadingAndErrorWrapper } from "metabase/components/LoadingAndErrorWrapper/DelayedLoadingAndErrorWrapper";
 import { Ellipsified } from "metabase/core/components/Ellipsified";
 import CS from "metabase/css/core/index.css";
@@ -76,7 +76,7 @@ function ApiKeysTable({
   }
 
   return (
-    <StyledTable
+    <ClientSortableTable
       data-testid="api-keys-table"
       columns={columns}
       rows={flatApiKeys}

frontend/src/metabase/common/components/Table/ClientSortableTable.tsx

+import React, { useMemo } from "react";
+
+import { SortDirection } from "metabase-types/api/sorting";
+
+import { type BaseRow, Table, type TableProps } from "./Table";
+
+const compareNumbers = (a: number, b: number) => a - b;
+
+export type ClientSortableTableProps<T extends BaseRow> = TableProps<T> & {
+  locale?: string;
+  formatValueForSorting?: (row: T, columnName: string) => any;
+};
+
+/**
+ * A basic reusable table component that supports client-side sorting by a column
+ *
+ * @param props.columns               - an array of objects with name and key properties
+ * @param props.rows                  - an array of objects with keys that match the column keys
+ * @param props.rowRenderer           - a function that takes a row object and returns a <tr> element
+ * @param props.formatValueForSorting - a function that is passed the row and column and returns a value to be used for sorting. Defaults to row[column]
+ * @param props.locale                - a locale used for string comparisons
+ * @param props.emptyBody             - content to be displayed when the row count is 0
+ * @param props.cols                  - a ReactNode that is inserted in the table element before <thead>. Useful for defining <colgroups> and <cols>
+ */
+export function ClientSortableTable<Row extends BaseRow>({
+  columns,
+  rows,
+  rowRenderer,
+  formatValueForSorting = (row: Row, columnName: string) => row[columnName],
+  locale,
+  ...rest
+}: ClientSortableTableProps<Row>) {
+  const [sortColumn, setSortColumn] = React.useState<string | null>(null);
+  const [sortDirection, setSortDirection] = React.useState<SortDirection>(
+    SortDirection.Asc,
+  );
+
+  const sortedRows = useMemo(() => {
+    if (sortColumn) {
+      return [...rows].sort((rowA, rowB) => {
+        const a = formatValueForSorting(rowA, sortColumn);
+        const b = formatValueForSorting(rowB, sortColumn);
+
+        if (!isSortableValue(a) || !isSortableValue(b)) {
+          return 0;
+        }
+
+        const result =
+          typeof a === "string"
+            ? compareStrings(a, b as string, locale)
+            : compareNumbers(a, b as number);
+        return sortDirection === SortDirection.Asc ? result : -result;
+      });
+    }
+    return rows;
+  }, [rows, sortColumn, sortDirection, locale, formatValueForSorting]);
+
+  return (
+    <Table
+      rows={sortedRows}
+      columns={columns}
+      rowRenderer={rowRenderer}
+      onSort={(name, direction) => {
+        setSortColumn(name);
+        setSortDirection(direction);
+      }}
+      sortColumnName={sortColumn}
+      sortDirection={sortDirection}
+      {...rest}
+    />
+  );
+}
+
+function isSortableValue(value: unknown): value is string | number {
+  return typeof value === "string" || typeof value === "number";
+}
+
+function compareStrings(a: string, b: string, locale?: string) {
+  return a.localeCompare(b, locale, { sensitivity: "base" });
+}

frontend/src/metabase/common/components/Table/Table.styled.tsx → frontend/src/metabase/common/components/Table/Table.module.css

-import styled from "@emotion/styled";
-
-import { Table } from "./Table";
-
-export const StyledTable = styled(Table)`
+.Table {
   width: 100%;
   border-collapse: unset;
   border-spacing: 0;
@@ -15,6 +11,7 @@ export const StyledTable = styled(Table)`
     text-align: left;
     padding: 0.5rem;
     border-bottom: 1px solid var(--mb-color-border);
+    padding-inline: 0.75rem;
   }
 
   tbody {
@@ -29,13 +26,6 @@ export const StyledTable = styled(Table)`
 
   td {
     border-bottom: 1px solid var(--mb-color-border);
-    padding-inline: 0.5rem;
-  }
-
-  &:first-of-type td,
-  th {
-    padding-inline-start: 1rem;
+    padding-inline: 0.75rem;
   }
-` as typeof Table;
-// we have to cast this because emotion messes up the generic types here
-// see https://github.com/emotion-js/emotion/issues/2342
+}

frontend/src/metabase/common/components/Table/Table.tsx

 import React from "react";
 
-import { color } from "metabase/lib/colors";
+import {
+  PaginationControls,
+  type PaginationControlsProps,
+} from "metabase/components/PaginationControls";
 import { Box, Flex, Icon } from "metabase/ui";
+import { SortDirection } from "metabase-types/api/sorting";
 
-type BaseRow = Record<string, unknown> & { id: number | string };
+import CS from "./Table.module.css";
+
+export type BaseRow = Record<string, unknown> & { id: number | string };
 
 type ColumnItem = {
   name: string;
   key: string;
+  sortable?: boolean;
 };
 
 export type TableProps<Row extends BaseRow> = {
   columns: ColumnItem[];
   rows: Row[];
   rowRenderer: (row: Row) => React.ReactNode;
-  tableProps?: React.HTMLAttributes<HTMLTableElement>;
+  sortColumnName?: string | null;
+  sortDirection?: SortDirection;
+  onSort?: (columnName: string, direction: SortDirection) => void;
+  paginationProps?: Pick<
+    PaginationControlsProps,
+    "page" | "pageSize" | "total"
+  > & { onPageChange: (page: number) => void };
+  emptyBody?: React.ReactNode;
+  cols?: React.ReactNode;
 };
 
 /**
- * A basic reusable table component that supports client-side sorting by a column
+ * A basic reusable table component
  *
- * @param columns     - an array of objects with name and key properties
- * @param rows        - an array of objects with keys that match the column keys
- * @param rowRenderer - a function that takes a row object and returns a <tr> element
- * @param tableProps  - additional props to pass to the <table> element
+ * @param props.columns         - an array of objects with name and key properties
+ * @param props.rows            - an array of objects with keys that match the column keys
+ * @param props.rowRenderer     - a function that takes a row object and returns a <tr> element
+ * @param props.emptyBody       - content to be displayed when the row count is 0
+ * @param props.cols            - a ReactNode that is inserted in the table element before <thead>. Useful for defining <colgroups> and <cols>
+ * @param props.sortColumnName  - ID of the column currently used in row sorting
+ * @param props.sortDirection   - The direction of the sort. Can be "asc" or "desc"
+ * @param props.onSort          - a callback containing updated sort info for when a header is clicked
+ * @param props.paginationProps - a map of information used to render pagination controls.
  */
+
 export function Table<Row extends BaseRow>({
   columns,
   rows,
   rowRenderer,
-  ...tableProps
+  sortColumnName,
+  sortDirection,
+  onSort,
+  paginationProps,
+  emptyBody,
+  cols,
+  ...rest
 }: TableProps<Row>) {
-  const [sortColumn, setSortColumn] = React.useState<string | null>(null);
-  const [sortDirection, setSortDirection] = React.useState<"asc" | "desc">(
-    "asc",
-  );
-
-  const sortedRows = React.useMemo(() => {
-    if (sortColumn) {
-      return [...rows].sort((a, b) => {
-        const aValue = a[sortColumn];
-        const bValue = b[sortColumn];
-        if (
-          aValue === bValue ||
-          !isSortableValue(aValue) ||
-          !isSortableValue(bValue)
-        ) {
-          return 0;
-        }
-        if (sortDirection === "asc") {
-          return aValue < bValue ? -1 : 1;
-        } else {
-          return aValue > bValue ? -1 : 1;
-        }
-      });
-    }
-    return rows;
-  }, [rows, sortColumn, sortDirection]);
-
   return (
-    <table {...tableProps}>
-      <thead>
-        <tr>
-          {columns.map(column => (
-            <th key={String(column.key)}>
-              <ColumnHeader
-                column={column}
-                sortColumn={sortColumn}
-                sortDirection={sortDirection}
-                onSort={(columnKey: string, direction: "asc" | "desc") => {
-                  setSortColumn(columnKey);
-                  setSortDirection(direction);
-                }}
-              />
-            </th>
-          ))}
-        </tr>
-      </thead>
-      <tbody>
-        {sortedRows.map((row, index) => (
-          <React.Fragment key={String(row.id) || index}>
-            {rowRenderer(row)}
-          </React.Fragment>
-        ))}
-      </tbody>
-    </table>
+    <>
+      <table {...rest} className={CS.Table}>
+        {cols && <colgroup>{cols}</colgroup>}
+        <thead>
+          <tr>
+            {columns.map(column => (
+              <th key={String(column.key)}>
+                {onSort && column.sortable !== false ? (
+                  <ColumnHeader
+                    column={column}
+                    sortColumn={sortColumnName}
+                    sortDirection={sortDirection}
+                    onSort={(columnKey: string, direction: SortDirection) => {
+                      onSort(columnKey, direction);
+                    }}
+                  />
+                ) : (
+                  column.name
+                )}
+              </th>
+            ))}
+          </tr>
+        </thead>
+        <tbody>
+          {rows.length > 0
+            ? rows.map((row, index) => (
+                <React.Fragment key={String(row.id) || index}>
+                  {rowRenderer(row)}
+                </React.Fragment>
+              ))
+            : emptyBody}
+        </tbody>
+      </table>
+
+      {paginationProps && (
+        <Flex justify="end">
+          <PaginationControls
+            page={paginationProps.page}
+            pageSize={paginationProps.pageSize}
+            total={paginationProps.total}
+            itemsLength={rows.length}
+            onNextPage={() =>
+              paginationProps.onPageChange(paginationProps.page + 1)
+            }
+            onPreviousPage={() =>
+              paginationProps.onPageChange(paginationProps.page - 1)
+            }
+          />
+        </Flex>
+      )}
+    </>
   );
 }
 
@@ -95,9 +120,9 @@ function ColumnHeader({
   onSort,
 }: {
   column: ColumnItem;
-  sortColumn: string | null;
-  sortDirection: "asc" | "desc";
-  onSort: (column: string, direction: "asc" | "desc") => void;
+  sortColumn?: string | null;
+  sortDirection?: SortDirection;
+  onSort: (column: string, direction: SortDirection) => void;
 }) {
   return (
     <Flex
@@ -107,7 +132,9 @@ function ColumnHeader({
       onClick={() =>
         onSort(
           String(column.key),
-          sortColumn === column.key && sortDirection === "asc" ? "desc" : "asc",
+          sortColumn === column.key && sortDirection === SortDirection.Asc
+            ? SortDirection.Desc
+            : SortDirection.Asc,
         )
       }
     >
@@ -115,8 +142,10 @@ function ColumnHeader({
       {
         column.name && column.key === sortColumn ? (
           <Icon
-            name={sortDirection === "desc" ? "chevronup" : "chevrondown"}
-            color={color("text-medium")}
+            name={
+              sortDirection === SortDirection.Asc ? "chevronup" : "chevrondown"
+            }
+            color="var(--mb-color-text-medium)"
             style={{ flexShrink: 0 }}
             size={8}
           />
@@ -127,7 +156,3 @@ function ColumnHeader({
     </Flex>
   );
 }
-
-function isSortableValue(value: unknown): value is string | number {
-  return typeof value === "string" || typeof value === "number";
-}

frontend/src/metabase/common/components/Table/Table.unit.spec.tsx

-import { render, screen } from "@testing-library/react";
+import { render, screen, within } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 
 import { getIcon, queryIcon } from "__support__/ui";
 
+import { ClientSortableTable } from "./ClientSortableTable";
 import { Table } from "./Table";
 
 type Pokemon = {
@@ -45,6 +46,22 @@ const sampleData: Pokemon[] = [
   },
 ];
 
+/** The Japanese words for blue and green are sorted differently in the ja-JP locale vs. the en-US locale */
+const sampleJapaneseData: Pokemon[] = [
+  {
+    id: 1,
+    name: "青いゼニガメ (Blue Squirtle)",
+    type: "Water",
+    generation: 1,
+  },
+  {
+    id: 2,
+    name: "緑のフシギダネ (Green Bulbasaur)",
+    type: "Grass",
+    generation: 1,
+  },
+];
+
 const sampleColumns = [
   {
     key: "name",
@@ -70,10 +87,10 @@ const renderRow = (row: Pokemon) => {
   );
 };
 
-describe("common > components > Table", () => {
+describe("common > components > ClientSortableTable", () => {
   it("should render table headings", () => {
     render(
-      <Table
+      <ClientSortableTable
         columns={sampleColumns}
         rows={sampleData}
         rowRenderer={renderRow}
@@ -87,7 +104,7 @@ describe("common > components > Table", () => {
 
   it("should render table row data", () => {
     render(
-      <Table
+      <ClientSortableTable
         columns={sampleColumns}
         rows={sampleData}
         rowRenderer={renderRow}
@@ -104,7 +121,7 @@ describe("common > components > Table", () => {
 
   it("should sort the table", async () => {
     render(
-      <Table
+      <ClientSortableTable
         columns={sampleColumns}
         rows={sampleData}
         rowRenderer={renderRow}
@@ -117,17 +134,57 @@ describe("common > components > Table", () => {
     firstRowShouldHaveText("Charmander");
 
     await userEvent.click(sortButton);
-    expect(getIcon("chevrondown")).toBeInTheDocument();
+    expect(getIcon("chevronup")).toBeInTheDocument();
     firstRowShouldHaveText("Bulbasaur");
 
     await userEvent.click(sortButton);
-    expect(getIcon("chevronup")).toBeInTheDocument();
+    expect(getIcon("chevrondown")).toBeInTheDocument();
     firstRowShouldHaveText("Squirtle");
   });
 
+  it("should respect locales when sorting tables", async () => {
+    render(
+      <>
+        <ClientSortableTable
+          data-testid="japanese-table"
+          columns={sampleColumns}
+          rows={sampleJapaneseData}
+          rowRenderer={renderRow}
+          locale="ja-JP"
+        />
+        <ClientSortableTable
+          data-testid="english-table"
+          columns={sampleColumns}
+          rows={sampleJapaneseData}
+          rowRenderer={renderRow}
+          locale="en-US"
+        />
+      </>,
+    );
+
+    expect(queryIcon("chevrondown")).not.toBeInTheDocument();
+    expect(queryIcon("chevronup")).not.toBeInTheDocument();
+
+    const japaneseTable = await screen.findByTestId("japanese-table");
+    const englishTable = await screen.findByTestId("english-table");
+
+    // Sort both tables
+    await userEvent.click(await within(japaneseTable).findByText("Name"));
+    await userEvent.click(await within(englishTable).findByText("Name"));
+
+    // The locales affect the order of the rows:
+    const englishRows = within(englishTable).getAllByRole("row");
+    expect(englishRows[1]).toHaveTextContent("Green");
+    expect(englishRows[2]).toHaveTextContent("Blue");
+
+    const japaneseRows = within(japaneseTable).getAllByRole("row");
+    expect(japaneseRows[1]).toHaveTextContent("Blue");
+    expect(japaneseRows[2]).toHaveTextContent("Green");
+  });
+
   it("should sort on multiple columns", async () => {
     render(
-      <Table
+      <ClientSortableTable
         columns={sampleColumns}
         rows={sampleData}
         rowRenderer={renderRow}
@@ -141,17 +198,112 @@ describe("common > components > Table", () => {
     firstRowShouldHaveText("Charmander");
 
     await userEvent.click(sortNameButton);
-    expect(getIcon("chevrondown")).toBeInTheDocument();
+    expect(getIcon("chevronup")).toBeInTheDocument();
     firstRowShouldHaveText("Bulbasaur");
 
     await userEvent.click(sortGenButton);
-    expect(getIcon("chevrondown")).toBeInTheDocument();
+    expect(getIcon("chevronup")).toBeInTheDocument();
     firstRowShouldHaveText("1");
 
     await userEvent.click(sortGenButton);
-    expect(getIcon("chevronup")).toBeInTheDocument();
+    expect(getIcon("chevrondown")).toBeInTheDocument();
     firstRowShouldHaveText("8");
   });
+
+  it("should present the empty component if no rows are given", async () => {
+    render(
+      <ClientSortableTable
+        columns={sampleColumns}
+        rows={[]}
+        rowRenderer={renderRow}
+        emptyBody={
+          <tr>
+            <td colSpan={3}>No Results</td>
+          </tr>
+        }
+      />,
+    );
+    expect(screen.getByText("Name")).toBeInTheDocument();
+    expect(screen.getByText("Type")).toBeInTheDocument();
+    expect(screen.getByText("Generation")).toBeInTheDocument();
+    expect(screen.getByText("No Results")).toBeInTheDocument();
+  });
+
+  it("should allow you provide a format values when sorting", async () => {
+    render(
+      <ClientSortableTable
+        columns={sampleColumns}
+        rows={sampleData}
+        rowRenderer={renderRow}
+        formatValueForSorting={(row, colName) => {
+          if (colName === "type") {
+            if (row.type === "Water") {
+              return 10;
+            }
+            return 1;
+          }
+          return row[colName as keyof Pokemon];
+        }}
+      />,
+    );
+    expect(screen.getByText("Name")).toBeInTheDocument();
+    expect(screen.getByText("Type")).toBeInTheDocument();
+    expect(screen.getByText("Generation")).toBeInTheDocument();
+
+    const sortNameButton = screen.getByText("Type");
+    // Ascending
+    await userEvent.click(sortNameButton);
+    // Descending
+    await userEvent.click(sortNameButton);
+    firstRowShouldHaveText("Squirtle");
+  });
+});
+
+describe("common > components > Table", () => {
+  it("should call the onSort handler with values when a row is clicked", async () => {
+    const onSort = jest.fn();
+
+    render(
+      <Table
+        columns={sampleColumns}
+        rows={sampleData}
+        rowRenderer={renderRow}
+        onSort={onSort}
+      />,
+    );
+    expect(screen.getByText("Name")).toBeInTheDocument();
+    expect(screen.getByText("Type")).toBeInTheDocument();
+    expect(screen.getByText("Generation")).toBeInTheDocument();
+
+    await userEvent.click(screen.getByText("Type"));
+
+    expect(onSort).toHaveBeenCalledWith("type", "asc");
+  });
+
+  it("if pageination props are passed, it should render the pagination controller.", async () => {
+    const onPageChange = jest.fn();
+
+    render(
+      <Table
+        columns={sampleColumns}
+        rows={sampleData}
+        rowRenderer={renderRow}
+        paginationProps={{
+          onPageChange,
+          page: 0,
+          total: sampleData.length,
+          pageSize: 3,
+        }}
+      />,
+    );
+    expect(screen.getByText("Name")).toBeInTheDocument();
+    expect(screen.getByText("Type")).toBeInTheDocument();
+    expect(screen.getByText("Generation")).toBeInTheDocument();
+
+    expect(
+      screen.getByRole("navigation", { name: /pagination/ }),
+    ).toBeInTheDocument();
+  });
 });
 
 function firstRowShouldHaveText(text: string) {

frontend/src/metabase/common/components/Table/index.ts

+export * from "./ClientSortableTable";
 export * from "./Table";
-export * from "./Table.styled";

frontend/src/metabase/components/PaginationControls/index.tsx

-export { PaginationControls } from "./PaginationControls";
+export {
+  PaginationControls,
+  type PaginationControlsProps,
+} from "./PaginationControls";