Add single select, multi select to QueryTable (#1065)

* WIP

* Wire up selection state changes w/ better click boundaries

* Remove onSelect from disk page

* don't toggle row select on more button click, modify columns array in place

* fix e2e tests

Co-authored-by: David Crespo <[email protected]>

Author: zephraph

app/pages/__tests__/instance/networking.e2e.ts

@@ -10,7 +10,6 @@ test('Instance networking tab', async ({ page }) => {
   // Instance networking tab
   await page.click('role=tab[name="Networking"]')
   await expectRowVisible(page, 'my-nic', [
-    '',
     'my-nic',
     'a network interface',
     '172.30.0.10',
@@ -56,15 +55,14 @@ test('Instance networking tab', async ({ page }) => {
     .click()
   await page.click('role=menuitem[name="Make primary"]')
   await expectRowVisible(page, 'my-nic', [
-    '',
     'my-nic',
     'a network interface',
     '172.30.0.10',
     'mock-vpc',
     'mock-subnet',
     '',
   ])
-  await expectRowVisible(page, 'nic-2', ['', 'nic-2', null, null, null, null, 'primary'])
+  await expectRowVisible(page, 'nic-2', ['nic-2', null, null, null, null, 'primary'])
 
   // Make an edit to the network interface
   await page

app/pages/__tests__/row-select.e2e.ts

@@ -7,7 +7,9 @@ import { forEach } from 'app/util/e2e'
 // usual testing philosophy), but they let us make sure selection is being
 // passed through to the UI Table.
 
-test('Row select works as expected', async ({ page }) => {
+// skipped for now because we no longer have any live multiselect tables to test
+// with. TODO: make it a testing-lib test instead?
+test.skip('Row multiselect works as expected', async ({ page }) => {
   // SETUP
 
   const headCheckbox = page.locator('thead >> role=checkbox')

app/pages/__tests__/ssh-keys.e2e.ts

@@ -18,7 +18,7 @@ test('SSH keys', async ({ page }) => {
 
   // it's there in the table
   await expectNotVisible(page, ['text="No SSH keys"'])
-  await expectRowVisible(page, 'my-key', ['', 'my-key', 'definitely a key'])
+  await expectRowVisible(page, 'my-key', ['my-key', 'definitely a key'])
 
   // now delete it
   await page.click('role=button[name="Row actions"]')

app/pages/project/networking/VpcPage/tabs/VpcFirewallRulesTab.tsx

@@ -11,7 +11,6 @@ import {
   TypeValueListCell,
   createTable,
   getActionsCol,
-  getSelectCol,
 } from '@oxide/table'
 import { Button, EmptyMessage, TableEmptyBox } from '@oxide/ui'
 
@@ -23,7 +22,6 @@ const tableHelper = createTable().setRowType<VpcFirewallRule>()
 
 /** columns that don't depend on anything in `render` */
 const staticColumns = [
-  tableHelper.createDisplayColumn(getSelectCol()),
   tableHelper.createDataColumn('name', { header: 'Name' }),
   tableHelper.createDataColumn('action', { header: 'Action' }),
   // map() fixes the fact that IpNets aren't strings

libs/table/QueryTable.tsx

@@ -3,7 +3,7 @@ import type { UseQueryOptions } from '@tanstack/react-query'
 import { hashQueryKey } from '@tanstack/react-query'
 import type { AccessorFn } from '@tanstack/react-table'
 import { getCoreRowModel, useTableInstance } from '@tanstack/react-table'
-import React from 'react'
+import React, { useEffect } from 'react'
 import { useCallback } from 'react'
 import { useMemo } from 'react'
 import type { ComponentType, ReactElement } from 'react'
@@ -17,7 +17,7 @@ import { isOneOf } from '@oxide/util'
 
 import { Table, createTable } from './Table'
 import { DefaultCell } from './cells'
-import { getActionsCol, getSelectCol } from './columns'
+import { getActionsCol, getMultiSelectCol, getSelectCol } from './columns'
 import type { MakeActions } from './columns'
 
 interface UseQueryTableResult<Item> {
@@ -44,7 +44,7 @@ export const useQueryTable = <A extends ApiListMethods, M extends keyof A>(
   return { Table, Column: QueryTableColumn }
 }
 
-interface QueryTableProps<Item> {
+type QueryTableProps<Item> = {
   /** Prints table data in the console when enabled */
   debug?: boolean
   /** Function that produces a list of actions from a row item */
@@ -53,7 +53,20 @@ interface QueryTableProps<Item> {
   pageSize?: number
   children: React.ReactNode
   emptyState: React.ReactElement
-}
+} & (
+  | {
+      onSelect: (selection: string) => void
+      onMultiSelect?: never
+    }
+  | {
+      onSelect?: never
+      onMultiSelect: (selections: string[]) => void
+    }
+  | {
+      onSelect?: never
+      onMultiSelect?: never
+    }
+)
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const makeQueryTable = <Item,>(
@@ -68,16 +81,25 @@ const makeQueryTable = <Item,>(
     pagination = 'page',
     pageSize = 10,
     emptyState,
+    onSelect,
+    onMultiSelect,
   }: QueryTableProps<Item>) {
     invariant(
       isOneOf(children, [QueryTableColumn]),
       'QueryTable can only have Column as a child'
     )
 
+    const [rowSelection, setRowSelection] = React.useState({})
+    useEffect(() => {
+      const selected = Object.keys(rowSelection)
+      onSelect?.(selected[0])
+      onMultiSelect?.(selected)
+    }, [rowSelection, onSelect, onMultiSelect])
+
     const { currentPage, goToNextPage, goToPrevPage, hasPrev } = usePagination()
     const tableHelper = useMemo(() => createTable().setRowType<Item>(), [])
     const columns = useMemo(() => {
-      let columns = React.Children.toArray(children).map((child) => {
+      const columns = React.Children.toArray(children).map((child) => {
         const column = { ...(child as ReactElement<QueryTableColumnProps<Item>>).props }
 
         // QueryTableColumnProps ensures `id` is passed in if and only if
@@ -101,12 +123,18 @@ const makeQueryTable = <Item,>(
         )
       })
 
+      if (onSelect) {
+        columns.unshift(getSelectCol())
+      } else if (onMultiSelect) {
+        columns.unshift(getMultiSelectCol())
+      }
+
       if (makeActions) {
-        columns = [getSelectCol(), ...columns, getActionsCol(makeActions)]
+        columns.push(getActionsCol(makeActions))
       }
 
       return columns
-    }, [children, tableHelper, makeActions])
+    }, [children, tableHelper, makeActions, onSelect, onMultiSelect])
 
     const { data, isLoading } = useApiQuery(
       query,
@@ -116,14 +144,20 @@ const makeQueryTable = <Item,>(
 
     const tableData: any[] = useMemo(() => (data as any)?.items || [], [data])
 
-    const getRowId = useCallback((row) => row.id, [])
+    const getRowId = useCallback((row) => row.name, [])
 
     const table = useTableInstance(tableHelper, {
       columns,
       data: tableData,
       getRowId,
       getCoreRowModel: getCoreRowModel(),
+      state: {
+        rowSelection,
+      },
       manualPagination: true,
+      enableRowSelection: !!onSelect,
+      enableMultiRowSelection: !!onMultiSelect,
+      onRowSelectionChange: setRowSelection,
     })
 
     if (debug) console.table((data as { items?: any[] })?.items || data)

libs/table/Table.tsx

@@ -1,5 +1,6 @@
 import type { TableInstance } from '@tanstack/react-table'
 import { createTable as _createTable } from '@tanstack/react-table'
+import cn from 'classnames'
 
 import { Table as UITable } from '@oxide/ui'
 
@@ -47,13 +48,36 @@ export const Table = <TGenerics extends OurTableGenerics>({
       ))}
     </UITable.Header>
     <UITable.Body>
-      {table.getRowModel().rows.map((row) => (
-        <UITable.Row className={rowClassName} selected={row.getIsSelected()} key={row.id}>
-          {row.getAllCells().map((cell) => (
-            <UITable.Cell key={cell.column.id}>{cell.renderCell()}</UITable.Cell>
-          ))}
-        </UITable.Row>
-      ))}
+      {table.getRowModel().rows.map((row) => {
+        const onSingleSelect = row.getCanSelect()
+          ? () => {
+              table.resetRowSelection()
+              row.toggleSelected()
+            }
+          : undefined
+        const onMultiSelect = row.getCanMultiSelect()
+          ? () => row.toggleSelected()
+          : undefined
+        const [firstCell, ...cells] = row.getAllCells()
+        return (
+          <UITable.Row
+            className={cn(rowClassName, { 'cursor-pointer': !!onSingleSelect })}
+            selected={row.getIsSelected()}
+            key={row.id}
+            onClick={onSingleSelect}
+          >
+            <UITable.Cell
+              onClick={onMultiSelect}
+              className={cn({ 'cursor-pointer': !!onMultiSelect })}
+            >
+              {firstCell.renderCell()}
+            </UITable.Cell>
+            {cells.map((cell) => (
+              <UITable.Cell key={cell.column.id}>{cell.renderCell()}</UITable.Cell>
+            ))}
+          </UITable.Row>
+        )
+      })}
     </UITable.Body>
   </UITable>
 )

libs/table/columns/action-col.tsx

@@ -29,7 +29,8 @@ export const getActionsCol = <TGenerics extends TableGenerics>(
         <div className="flex justify-center">
           <Menu>
             {/* TODO: This name should not suck; future us, make it so! */}
-            <MenuButton aria-label="Row actions">
+            {/* stopPropagation prevents clicks from toggling row select in a single select table */}
+            <MenuButton aria-label="Row actions" onClick={(e) => e.stopPropagation()}>
               <More12Icon className="text-tertiary" />
             </MenuButton>
             <MenuList>

libs/table/columns/select-col.tsx

@@ -1,9 +1,19 @@
 import type { Row, TableInstance } from '@tanstack/react-table'
 
-import { Checkbox } from '@oxide/ui'
+import { Checkbox, Radio } from '@oxide/ui'
 
 // only needs to be a function because of the generic params
 export const getSelectCol = <TGenerics,>() => ({
+  id: 'select',
+  meta: { thClassName: 'w-10' },
+  header: '',
+  cell: ({ row }: { row: Row<TGenerics>; instance: TableInstance<TGenerics> }) => {
+    // `onChange` is empty to suppress react warning. Actual trigger happens in `Table.tsx`
+    return <Radio checked={row.getIsSelected()} onChange={() => {}} />
+  },
+})
+
+export const getMultiSelectCol = <TGenerics,>() => ({
   id: 'select',
   meta: { thClassName: 'w-10' },
   header: ({ instance }: { instance: TableInstance<TGenerics> }) => (
@@ -16,6 +26,7 @@ export const getSelectCol = <TGenerics,>() => ({
     </div>
   ),
   cell: ({ row }: { row: Row<TGenerics> }) => (
-    <Checkbox checked={row.getIsSelected()} onChange={row.getToggleSelectedHandler()} />
+    // `onChange` is empty to suppress react warning. Actual trigger happens in `Table.tsx`
+    <Checkbox checked={row.getIsSelected()} onChange={() => {}} />
   ),
 })

libs/ui/lib/radio/Radio.tsx

@@ -28,7 +28,7 @@ export const Radio = ({ children, className, ...inputProps }: RadioProps) => (
       <div className="absolute left-1 top-1 hidden h-2 w-2 rounded-full bg-accent peer-checked:block" />
     </span>
 
-    <span className="ml-2.5 text-sans-md text-secondary">{children}</span>
+    {children && <span className="ml-2.5 text-sans-md text-secondary">{children}</span>}
   </label>
 )