Skip to content

Commit 28ea951

Browse files
Improve external ips row (#2576)
* List ephemeral IP last; handle overflow scrolling * Add overflow logic * Add spacing; add tests * oops * refactoring * Fix import * Sort external IPs on instance networking table with ephemeral at end of list * fix test * Tighten copy button and tweak overflow style * update test * Remove unneeded comment * Use Remeda for sorting; memoize IP list ordering for table --------- Co-authored-by: Benjamin Leonard <[email protected]>
1 parent 0878bcc commit 28ea951

File tree

7 files changed

+48
-11
lines changed

7 files changed

+48
-11
lines changed

app/components/ExternalIps.tsx

+30-5
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,22 @@
66
* Copyright Oxide Computer Company
77
*/
88

9-
import { useApiQuery } from '@oxide/api'
9+
import { Link } from 'react-router'
10+
import * as R from 'remeda'
11+
12+
import { useApiQuery, type ExternalIp } from '@oxide/api'
1013

1114
import { EmptyCell, SkeletonCell } from '~/table/cells/EmptyCell'
1215
import { CopyableIp } from '~/ui/lib/CopyableIp'
16+
import { Slash } from '~/ui/lib/Slash'
1317
import { intersperse } from '~/util/array'
18+
import { pb } from '~/util/path-builder'
1419
import type * as PP from '~/util/path-params'
1520

21+
/** Move ephemeral IP (if present) to the end of the list of external IPs */
22+
export const orderIps = (ips: ExternalIp[]) =>
23+
R.sortBy(ips, (a) => (a.kind === 'ephemeral' ? 1 : -1))
24+
1625
export function ExternalIps({ project, instance }: PP.Instance) {
1726
const { data, isPending } = useApiQuery('instanceExternalIpList', {
1827
path: { instance },
@@ -22,11 +31,27 @@ export function ExternalIps({ project, instance }: PP.Instance) {
2231

2332
const ips = data?.items
2433
if (!ips || ips.length === 0) return <EmptyCell />
34+
const orderedIps = orderIps(ips)
35+
const ipsToShow = orderedIps.slice(0, 2)
36+
const overflowCount = orderedIps.length - ipsToShow.length
37+
38+
// create a list of CopyableIp components
39+
const links = ipsToShow.map((eip) => <CopyableIp ip={eip.ip} key={eip.ip} />)
40+
2541
return (
26-
<div className="flex items-center gap-1">
27-
{intersperse(
28-
ips.map((eip) => <CopyableIp ip={eip.ip} key={eip.ip} />),
29-
<span className="text-quaternary"> / </span>
42+
<div className="flex max-w-full items-center">
43+
{intersperse(links, <Slash className="ml-0.5 mr-1.5" />)}
44+
{/* if there are more than 2 ips, add a link to the instance networking page */}
45+
{overflowCount > 0 && (
46+
<>
47+
<Slash className="ml-0.5 mr-1.5" />
48+
<Link
49+
to={pb.instanceNetworking({ project, instance })}
50+
className="hover:link-with-underline -m-2 self-center p-2 text-tertiary"
51+
>
52+
53+
</Link>
54+
</>
3055
)}
3156
</div>
3257
)

app/pages/project/instances/instance/tabs/NetworkingTab.tsx

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { IpGlobal24Icon, Networking24Icon } from '@oxide/design-system/icons/rea
2424

2525
import { AttachEphemeralIpModal } from '~/components/AttachEphemeralIpModal'
2626
import { AttachFloatingIpModal } from '~/components/AttachFloatingIpModal'
27+
import { orderIps } from '~/components/ExternalIps'
2728
import { HL } from '~/components/HL'
2829
import { ListPlusCell } from '~/components/ListPlusCell'
2930
import { CreateNetworkInterfaceForm } from '~/forms/network-interface-create'
@@ -371,7 +372,7 @@ export function Component() {
371372

372373
const ipTableInstance = useReactTable({
373374
columns: useColsWithActions(staticIpCols, makeIpActions),
374-
data: eips?.items || [],
375+
data: useMemo(() => orderIps(eips.items), [eips]),
375376
getCoreRowModel: getCoreRowModel(),
376377
})
377378

app/pages/settings/ProfilePage.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export function ProfilePage() {
4545
<PropertiesTable.Row label="Display name">{me.displayName}</PropertiesTable.Row>
4646
<PropertiesTable.Row label="User ID">
4747
{me.id}
48-
<CopyToClipboard className="ml-2" text={me.id} />
48+
<CopyToClipboard className="ml-1" text={me.id} />
4949
</PropertiesTable.Row>
5050
</PropertiesTable>
5151

app/ui/lib/CopyableIp.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import { CopyToClipboard } from '~/ui/lib/CopyToClipboard'
99

1010
export const CopyableIp = ({ ip, isLinked = true }: { ip: string; isLinked?: boolean }) => (
11-
<span className="flex max-w-full items-center gap-1">
11+
<span className="flex max-w-full items-center gap-0.5">
1212
{isLinked ? (
1313
<a
1414
className="link-with-underline truncate text-sans-md"

app/ui/lib/Slash.tsx

+6-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
*
66
* Copyright Oxide Computer Company
77
*/
8-
export const Slash = () => (
9-
<span className="mx-1 text-quaternary selected:text-accent-disabled">/</span>
8+
import cn from 'classnames'
9+
10+
export const Slash = ({ className }: { className?: string }) => (
11+
<span className={cn('mx-1 text-quaternary selected:text-accent-disabled', className)}>
12+
/
13+
</span>
1014
)

app/ui/lib/Truncate.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export const Truncate = ({
3333
// Only use the tooltip if the text is longer than maxLength
3434
return (
3535
// overflow-hidden required to make inner truncate work
36-
<div className="flex items-center space-x-2 overflow-hidden">
36+
<div className="flex items-center gap-0.5 overflow-hidden">
3737
<Tooltip content={text} delay={tooltipDelay}>
3838
<div aria-label={text} className="truncate">
3939
{truncate(text, maxLength, position)}

test/e2e/instance-networking.e2e.ts

+7
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ test('Instance networking tab — floating IPs', async ({ page }) => {
122122
await expectRowVisible(externalIpTable, { ip: '123.4.56.0', Kind: 'ephemeral' })
123123
await expectRowVisible(externalIpTable, { ip: '123.4.56.5', Kind: 'floating' })
124124

125+
await expect(page.getByText('external IPs123.4.56.5/123.4.56.0')).toBeVisible()
126+
125127
// Attach a new external IP
126128
await attachFloatingIpButton.click()
127129
await expectVisible(page, ['role=heading[name="Attach floating IP"]'])
@@ -143,9 +145,14 @@ test('Instance networking tab — floating IPs', async ({ page }) => {
143145
// Verify that the "Attach floating IP" button is disabled, since there shouldn't be any more IPs to attach
144146
await expect(attachFloatingIpButton).toBeDisabled()
145147

148+
// Verify that the External IPs table row has an ellipsis link in it
149+
await expect(page.getByText('123.4.56.5/…')).toBeVisible()
150+
146151
// Detach one of the external IPs
147152
await clickRowAction(page, 'cola-float', 'Detach')
148153
await page.getByRole('button', { name: 'Confirm' }).click()
154+
await expect(page.getByText('123.4.56.5/…')).toBeHidden()
155+
await expect(page.getByText('external IPs123.4.56.4/123.4.56.0')).toBeVisible()
149156

150157
// Since we detached it, we don't expect to see the row any longer
151158
await expect(externalIpTable.getByRole('cell', { name: 'cola-float' })).toBeHidden()

0 commit comments

Comments
 (0)