diff --git a/app/components/form/fields/DisksTableField.tsx b/app/components/form/fields/DisksTableField.tsx index 6a78ce9f6a..e5af57ff36 100644 --- a/app/components/form/fields/DisksTableField.tsx +++ b/app/components/form/fields/DisksTableField.tsx @@ -13,16 +13,15 @@ import type { DiskCreate } from '@oxide/api' import { AttachDiskModalForm } from '~/forms/disk-attach' import { CreateDiskSideModalForm } from '~/forms/disk-create' import type { InstanceCreateInput } from '~/forms/instance-create' -import { EmptyCell } from '~/table/cells/EmptyCell' +import { sizeCellInner } from '~/table/columns/common' import { Badge } from '~/ui/lib/Badge' import { Button } from '~/ui/lib/Button' -import * as MiniTable from '~/ui/lib/MiniTable' +import { MiniTable } from '~/ui/lib/MiniTable' import { Truncate } from '~/ui/lib/Truncate' -import { bytesToGiB } from '~/util/units' export type DiskTableItem = | (DiskCreate & { type: 'create' }) - | { name: string; type: 'attach' } + | { name: string; type: 'attach'; size: number } /** * Designed less for reuse, more to encapsulate logic that would otherwise @@ -47,54 +46,28 @@ export function DisksTableField({ return ( <>
- - - Name - Type - Size - {/* For remove button */} - - - - {items.length ? ( - items.map((item, index) => ( - - - - - - {item.type} - - - {item.type === 'attach' ? ( - - ) : ( - <> - {bytesToGiB(item.size)} - GiB - - )} - - onChange(items.filter((i) => i.name !== item.name))} - label={`remove disk ${item.name}`} - /> - - )) - ) : ( - - )} - - + , + }, + { + header: 'Type', + cell: (item) => {item.type}, + }, + { + header: 'Size', + cell: (item) => sizeCellInner(item.size), + }, + ]} + rowKey={(item) => item.name} + onRemoveItem={(item) => onChange(items.filter((i) => i.name !== item.name))} + removeLabel={(item) => `Remove disk ${item.name}`} + emptyState={{ title: 'No disks', body: 'Add a disk to see it here' }} + />
- portRangeForm.reset()} @@ -458,24 +436,16 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = />
{ports.value.length > 0 && ( - - - Port ranges - {/* For remove button */} - - - - {ports.value.map((p) => ( - - {p} - ports.onChange(ports.value.filter((p1) => p1 !== p))} - label={`remove port ${p}`} - /> - - ))} - - + p }]} + rowKey={(port) => port} + emptyState={{ title: 'No ports', body: 'Add a port to see it here' }} + onRemoveItem={(p) => ports.onChange(ports.value.filter((p1) => p1 !== p))} + removeLabel={(port) => `remove port ${port}`} + /> )}
diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 281aabc484..5415e5de07 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -67,7 +67,7 @@ import { FormDivider } from '~/ui/lib/Divider' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { Listbox } from '~/ui/lib/Listbox' import { Message } from '~/ui/lib/Message' -import * as MiniTable from '~/ui/lib/MiniTable' +import { MiniTable } from '~/ui/lib/MiniTable' import { Modal } from '~/ui/lib/Modal' import { PageHeader, PageTitle } from '~/ui/lib/PageHeader' import { RadioCard } from '~/ui/lib/Radio' @@ -694,8 +694,6 @@ const AdvancedAccordion = ({ ) } - const isFloatingIpAttached = attachedFloatingIps.some((ip) => ip.floatingIp !== '') - const selectedFloatingIpMessage = ( <> This instance will be reachable at{' '} @@ -786,40 +784,18 @@ const AdvancedAccordion = ({ /> ) : ( -
- - - Name - IP - {/* For remove button */} - - - - {isFloatingIpAttached ? ( - attachedFloatingIpsData.map((item, index) => ( - - {item.name} - {item.ip} - detachFloatingIp(item.name)} - label={`remove floating IP ${item.name}`} - /> - - )) - ) : ( - - )} - - +
+ item.name }, + { header: 'IP', cell: (item) => item.ip }, + ]} + rowKey={(item) => item.name} + onRemoveItem={(item) => detachFloatingIp(item.name)} + removeLabel={(item) => `remove floating IP ${item.name}`} + />
- transitIpsForm.reset()} onSubmit={submitTransitIp} />
- {transitIps.length > 0 && ( - - - Transit IPs - {/* For remove button */} - - - - {transitIps.map((ip, index) => ( - - {ip} - { - form.setValue( - 'transitIps', - transitIps.filter((item) => item !== ip) - ) - }} - /> - - ))} - - - )} + ip }]} + rowKey={(ip) => ip} + onRemoveItem={(ip) => { + form.setValue( + 'transitIps', + transitIps.filter((item) => item !== ip) + ) + }} + removeLabel={(ip) => `remove IP ${ip}`} + /> ) } diff --git a/app/table/columns/common.tsx b/app/table/columns/common.tsx index 21c368f3cb..e6cb831039 100644 --- a/app/table/columns/common.tsx +++ b/app/table/columns/common.tsx @@ -38,8 +38,9 @@ function instanceStateCell(info: Info) { return } -function sizeCell(info: Info) { - const size = filesize(info.getValue(), { base: 2, output: 'object' }) +// not using Info so this can also be used for minitables +export function sizeCellInner(value: number) { + const size = filesize(value, { base: 2, output: 'object' }) return ( {size.value} {size.unit} @@ -55,7 +56,7 @@ export const Columns = { }, id: { header: 'ID', cell: idCell }, instanceState: { header: 'state', cell: instanceStateCell }, - size: { cell: sizeCell }, + size: { cell: (info: Info) => sizeCellInner(info.getValue()) }, timeCreated: { header: 'created', cell: dateCell }, timeModified: { header: 'modified', cell: dateCell }, } diff --git a/app/ui/lib/MiniTable.tsx b/app/ui/lib/MiniTable.tsx index 601fe99d7b..18e31fc0f4 100644 --- a/app/ui/lib/MiniTable.tsx +++ b/app/ui/lib/MiniTable.tsx @@ -15,21 +15,21 @@ import { Table as BigTable } from './Table' type Children = { children: React.ReactNode } -export const Table = classed.table`ox-mini-table w-full border-separate text-sans-md` +const Table = classed.table`ox-mini-table w-full border-separate text-sans-md` -export const Header = ({ children }: Children) => ( +const Header = ({ children }: Children) => ( {children} ) -export const HeadCell = BigTable.HeadCell +const HeadCell = BigTable.HeadCell -export const Body = classed.tbody`` +const Body = classed.tbody`` -export const Row = classed.tr`is-selected children:border-default first:children:border-l children:last:border-b last:children:border-r` +const Row = classed.tr`is-selected children:border-default first:children:border-l children:last:border-b last:children:border-r` -export const Cell = ({ children }: Children) => { +const Cell = ({ children }: Children) => { return (
{children}
@@ -37,7 +37,7 @@ export const Cell = ({ children }: Children) => { ) } -export const EmptyState = (props: { title: string; body: string; colSpan: number }) => ( +const EmptyState = (props: { title: string; body: string; colSpan: number }) => (
@@ -70,7 +70,7 @@ export const InputCell = ({ // followed this for icon in button best practices // https://www.sarasoueidan.com/blog/accessible-icon-buttons/ -export const RemoveCell = ({ onClick, label }: { onClick: () => void; label: string }) => ( +const RemoveCell = ({ onClick, label }: { onClick: () => void; label: string }) => (
) + +type Column = { + header: string + cell: (item: T, index: number) => React.ReactNode +} + +type MiniTableProps = { + ariaLabel: string + items: T[] + columns: Column[] + rowKey: (item: T, index: number) => string + onRemoveItem: (item: T) => void + removeLabel?: (item: T) => string + /** + * If empty state is not provided, the entire table will disappear when items + * is empty + */ + emptyState?: { title: string; body: string } + className?: string +} + +/** If `emptyState` is left out, `MiniTable` renders null when `items` is empty. */ +export function MiniTable({ + ariaLabel, + items, + columns, + rowKey, + onRemoveItem, + removeLabel, + emptyState, + className, +}: MiniTableProps) { + if (!emptyState && items.length === 0) return null + + return ( + +
+ {columns.map((column, index) => ( + {column.header} + ))} + {/* For remove button */} +
+ + + {items.length ? ( + items.map((item, index) => ( + + {columns.map((column, colIndex) => ( + {column.cell(item, index)} + ))} + + onRemoveItem(item)} + label={removeLabel?.(item) || `Remove item ${index + 1}`} + /> + + )) + ) : emptyState ? ( + + ) : null} + +
+ ) +} diff --git a/app/ui/styles/components/mini-table.css b/app/ui/styles/components/mini-table.css index 5bbcc717bc..3bf03222fa 100644 --- a/app/ui/styles/components/mini-table.css +++ b/app/ui/styles/components/mini-table.css @@ -2,7 +2,7 @@ * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * + * * Copyright Oxide Computer Company */ @@ -28,6 +28,10 @@ content: ' '; } + & tr td:last-child:before { + @apply hidden; + } + & tr:last-child td + td:before { @apply bottom-[calc(0.5rem+2px)]; } diff --git a/package-lock.json b/package-lock.json index 74dd734ce7..73524f7779 100644 --- a/package-lock.json +++ b/package-lock.json @@ -57,7 +57,7 @@ "@ianvs/prettier-plugin-sort-imports": "^4.4.1", "@mswjs/http-middleware": "^0.10.3", "@oxide/openapi-gen-ts": "~0.7.0", - "@playwright/test": "^1.52.0", + "@playwright/test": "^1.53.1", "@testing-library/dom": "^10.4.0", "@testing-library/jest-dom": "^6.6.3", "@testing-library/react": "^16.2.0", @@ -1758,13 +1758,13 @@ } }, "node_modules/@playwright/test": { - "version": "1.52.0", - "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.52.0.tgz", - "integrity": "sha512-uh6W7sb55hl7D6vsAeA+V2p5JnlAqzhqFyF0VcJkKZXkgnFcVG9PziERRHQfPLfNGx1C292a4JqbWzhR8L4R1g==", + "version": "1.53.1", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.53.1.tgz", + "integrity": "sha512-Z4c23LHV0muZ8hfv4jw6HngPJkbbtZxTkxPNIg7cJcTc9C28N/p2q7g3JZS2SiKBBHJ3uM1dgDye66bB7LEk5w==", "dev": true, "license": "Apache-2.0", "dependencies": { - "playwright": "1.52.0" + "playwright": "1.53.1" }, "bin": { "playwright": "cli.js" @@ -11298,13 +11298,13 @@ } }, "node_modules/playwright": { - "version": "1.52.0", - "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.52.0.tgz", - "integrity": "sha512-JAwMNMBlxJ2oD1kce4KPtMkDeKGHQstdpFPcPH3maElAXon/QZeTvtsfXmTMRyO9TslfoYOXkSsvao2nE1ilTw==", + "version": "1.53.1", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.53.1.tgz", + "integrity": "sha512-LJ13YLr/ocweuwxyGf1XNFWIU4M2zUSo149Qbp+A4cpwDjsxRPj7k6H25LBrEHiEwxvRbD8HdwvQmRMSvquhYw==", "dev": true, "license": "Apache-2.0", "dependencies": { - "playwright-core": "1.52.0" + "playwright-core": "1.53.1" }, "bin": { "playwright": "cli.js" @@ -11317,9 +11317,9 @@ } }, "node_modules/playwright-core": { - "version": "1.52.0", - "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.52.0.tgz", - "integrity": "sha512-l2osTgLXSMeuLZOML9qYODUQoPPnUsKsb5/P6LJ2e6uPKXUdPK5WYhN4z03G+YNbWmGDY4YENauNu4ZKczreHg==", + "version": "1.53.1", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.53.1.tgz", + "integrity": "sha512-Z46Oq7tLAyT0lGoFx4DOuB1IA9D1TPj0QkYxpPVUnGDqHHvDpCftu1J2hM2PiWsNMoZh8+LQaarAWcDfPBc6zg==", "dev": true, "license": "Apache-2.0", "bin": { diff --git a/package.json b/package.json index f86d570968..65dde430ea 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "@ianvs/prettier-plugin-sort-imports": "^4.4.1", "@mswjs/http-middleware": "^0.10.3", "@oxide/openapi-gen-ts": "~0.7.0", - "@playwright/test": "^1.52.0", + "@playwright/test": "^1.53.1", "@testing-library/dom": "^10.4.0", "@testing-library/jest-dom": "^6.6.3", "@testing-library/react": "^16.2.0", diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index 9ad4af8fd4..e62761ca4b 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -506,7 +506,7 @@ test('create from existing rule', async ({ page }) => { await expect(portFilters).toBeHidden() const targets = modal.getByRole('table', { name: 'Targets' }) - await expect(targets.getByRole('row', { name: 'Name: default, Type: vpc' })).toBeVisible() + await expect(targets.getByRole('row', { name: 'vpc default' })).toBeVisible() // close the modal await page.keyboard.press('Escape') @@ -526,7 +526,7 @@ test('create from existing rule', async ({ page }) => { await expect(modal.getByRole('checkbox', { name: 'UDP' })).not.toBeChecked() await expect(modal.getByRole('checkbox', { name: 'ICMP' })).not.toBeChecked() - await expect(targets.getByRole('row', { name: 'Name: default, Type: vpc' })).toBeVisible() + await expect(targets.getByRole('row', { name: 'vpc default' })).toBeVisible() }) const rulePath = '/projects/mock-project/vpcs/mock-vpc/firewall-rules/allow-icmp/edit' diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index 9d9357b381..33bba89cca 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -583,7 +583,7 @@ test('create instance with additional disks', async ({ page }) => { const disksTable = page.getByRole('table', { name: 'Disks' }) await expect(disksTable.getByText('disk-6')).toBeHidden() - await expectRowVisible(disksTable, { Name: 'new-disk-1', Type: 'create', Size: '5GiB' }) + await expectRowVisible(disksTable, { Name: 'new-disk-1', Type: 'create', Size: '5 GiB' }) // now that name is taken too, so disk create disallows it await page.getByRole('button', { name: 'Create new disk' }).click() @@ -597,7 +597,7 @@ test('create instance with additional disks', async ({ page }) => { await selectOption(page, 'Disk name', 'disk-3') await page.getByRole('button', { name: 'Attach disk' }).click() - await expectRowVisible(disksTable, { Name: 'disk-3', Type: 'attach', Size: '—' }) + await expectRowVisible(disksTable, { Name: 'disk-3', Type: 'attach', Size: '6 GiB' }) // Create the instance await page.getByRole('button', { name: 'Create instance' }).click()