diff --git a/enterprise/frontend/src/metabase-enterprise/upload_management/UploadManagement.unit.spec.tsx b/enterprise/frontend/src/metabase-enterprise/upload_management/UploadManagement.unit.spec.tsx index 4bd1f2f5d3845b8784a109f06e939d9782380b77..5b115316207b45b95e25ffd65a77c12cddfbe43f 100644 --- a/enterprise/frontend/src/metabase-enterprise/upload_management/UploadManagement.unit.spec.tsx +++ b/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/); }); diff --git a/enterprise/frontend/src/metabase-enterprise/upload_management/UploadManagementTable.tsx b/enterprise/frontend/src/metabase-enterprise/upload_management/UploadManagementTable.tsx index fda6807c464a9c967fe0e3e6078b5017069589ad..eb8057636a7164712c6110e237ce6c6fbed63ced 100644 --- a/enterprise/frontend/src/metabase-enterprise/upload_management/UploadManagementTable.tsx +++ b/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} diff --git a/frontend/src/metabase/admin/settings/components/ApiKeys/ManageApiKeys.tsx b/frontend/src/metabase/admin/settings/components/ApiKeys/ManageApiKeys.tsx index a14e52aba309f86bb9efb3674b9a520c79a47361..eabc596d18dca6f1c62f7eb0ba6847a721792129 100644 --- a/frontend/src/metabase/admin/settings/components/ApiKeys/ManageApiKeys.tsx +++ b/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} diff --git a/frontend/src/metabase/common/components/Table/ClientSortableTable.tsx b/frontend/src/metabase/common/components/Table/ClientSortableTable.tsx new file mode 100644 index 0000000000000000000000000000000000000000..2ffe7ba30fadfad502fcda76bd8af84f568be32e --- /dev/null +++ b/frontend/src/metabase/common/components/Table/ClientSortableTable.tsx @@ -0,0 +1,80 @@ +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" }); +} diff --git a/frontend/src/metabase/common/components/Table/Table.styled.tsx b/frontend/src/metabase/common/components/Table/Table.module.css similarity index 58% rename from frontend/src/metabase/common/components/Table/Table.styled.tsx rename to frontend/src/metabase/common/components/Table/Table.module.css index 07adcfaa5a380c9aa21b8b1565389077631ebca3..d5cbc28edb2b06c2156ff095e08575b5a29f58cd 100644 --- a/frontend/src/metabase/common/components/Table/Table.styled.tsx +++ b/frontend/src/metabase/common/components/Table/Table.module.css @@ -1,8 +1,4 @@ -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 +} diff --git a/frontend/src/metabase/common/components/Table/Table.tsx b/frontend/src/metabase/common/components/Table/Table.tsx index 6851c57a18eb90111addbd5eb7c1753c317c11af..245477bf9b7eccf03cf39a025082cbee2f52ee8d 100644 --- a/frontend/src/metabase/common/components/Table/Table.tsx +++ b/frontend/src/metabase/common/components/Table/Table.tsx @@ -1,90 +1,115 @@ 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"; -} diff --git a/frontend/src/metabase/common/components/Table/Table.unit.spec.tsx b/frontend/src/metabase/common/components/Table/Table.unit.spec.tsx index e09de78e80007e45f7769d448d7adec95fef350e..5f28e726d86e13fb3453d583b5ad5b31e23f4ff1 100644 --- a/frontend/src/metabase/common/components/Table/Table.unit.spec.tsx +++ b/frontend/src/metabase/common/components/Table/Table.unit.spec.tsx @@ -1,8 +1,9 @@ -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) { diff --git a/frontend/src/metabase/common/components/Table/index.ts b/frontend/src/metabase/common/components/Table/index.ts index 7a821f023bec35db0e5f827b8ffae326a1e67268..81c997d2efb5a9b856f6e4f07170eae1aeaeb43a 100644 --- a/frontend/src/metabase/common/components/Table/index.ts +++ b/frontend/src/metabase/common/components/Table/index.ts @@ -1,2 +1,2 @@ +export * from "./ClientSortableTable"; export * from "./Table"; -export * from "./Table.styled"; diff --git a/frontend/src/metabase/components/PaginationControls/index.tsx b/frontend/src/metabase/components/PaginationControls/index.tsx index f0b87e6be95ea5e89a10cfd0a8515ca7acdb2348..a259a62e0430dcc97dba9c99ecbf5d2fd4887436 100644 --- a/frontend/src/metabase/components/PaginationControls/index.tsx +++ b/frontend/src/metabase/components/PaginationControls/index.tsx @@ -1 +1,4 @@ -export { PaginationControls } from "./PaginationControls"; +export { + PaginationControls, + type PaginationControlsProps, +} from "./PaginationControls";