Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve external ips row #2576

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions app/components/ExternalIps.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,22 @@
* Copyright Oxide Computer Company
*/

import { useApiQuery } from '@oxide/api'
import { Link } from 'react-router'
import * as R from 'remeda'

import { useApiQuery, type ExternalIp } from '@oxide/api'

import { EmptyCell, SkeletonCell } from '~/table/cells/EmptyCell'
import { CopyableIp } from '~/ui/lib/CopyableIp'
import { Slash } from '~/ui/lib/Slash'
import { intersperse } from '~/util/array'
import { pb } from '~/util/path-builder'
import type * as PP from '~/util/path-params'

/** Move ephemeral IP (if present) to the end of the list of external IPs */
export const orderIps = (ips: ExternalIp[]) =>
R.sortBy(ips, (a) => (a.kind === 'ephemeral' ? 1 : -1))

export function ExternalIps({ project, instance }: PP.Instance) {
const { data, isPending } = useApiQuery('instanceExternalIpList', {
path: { instance },
Expand All @@ -22,11 +31,27 @@ export function ExternalIps({ project, instance }: PP.Instance) {

const ips = data?.items
if (!ips || ips.length === 0) return <EmptyCell />
const orderedIps = orderIps(ips)
const ipsToShow = orderedIps.slice(0, 2)
const overflowCount = orderedIps.length - ipsToShow.length

// create a list of CopyableIp components
const links = ipsToShow.map((eip) => <CopyableIp ip={eip.ip} key={eip.ip} />)

return (
<div className="flex items-center gap-1">
{intersperse(
ips.map((eip) => <CopyableIp ip={eip.ip} key={eip.ip} />),
<span className="text-quaternary"> / </span>
<div className="flex max-w-full items-center">
{intersperse(links, <Slash className="ml-0.5 mr-1.5" />)}
{/* if there are more than 2 ips, add a link to the instance networking page */}
{overflowCount > 0 && (
<>
<Slash className="ml-0.5 mr-1.5" />
<Link
to={pb.instanceNetworking({ project, instance })}
className="hover:link-with-underline -m-2 self-center p-2 text-tertiary"
>
</Link>
</>
)}
</div>
)
Expand Down
3 changes: 2 additions & 1 deletion app/pages/project/instances/instance/tabs/NetworkingTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { IpGlobal24Icon, Networking24Icon } from '@oxide/design-system/icons/rea

import { AttachEphemeralIpModal } from '~/components/AttachEphemeralIpModal'
import { AttachFloatingIpModal } from '~/components/AttachFloatingIpModal'
import { orderIps } from '~/components/ExternalIps'
import { HL } from '~/components/HL'
import { ListPlusCell } from '~/components/ListPlusCell'
import { CreateNetworkInterfaceForm } from '~/forms/network-interface-create'
Expand Down Expand Up @@ -371,7 +372,7 @@ export function Component() {

const ipTableInstance = useReactTable({
columns: useColsWithActions(staticIpCols, makeIpActions),
data: eips?.items || [],
data: useMemo(() => orderIps(eips.items), [eips]),
getCoreRowModel: getCoreRowModel(),
})

Expand Down
2 changes: 1 addition & 1 deletion app/pages/settings/ProfilePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function ProfilePage() {
<PropertiesTable.Row label="Display name">{me.displayName}</PropertiesTable.Row>
<PropertiesTable.Row label="User ID">
{me.id}
<CopyToClipboard className="ml-2" text={me.id} />
<CopyToClipboard className="ml-1" text={me.id} />
</PropertiesTable.Row>
</PropertiesTable>

Expand Down
2 changes: 1 addition & 1 deletion app/ui/lib/CopyableIp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { CopyToClipboard } from '~/ui/lib/CopyToClipboard'

export const CopyableIp = ({ ip, isLinked = true }: { ip: string; isLinked?: boolean }) => (
<span className="flex max-w-full items-center gap-1">
<span className="flex max-w-full items-center gap-0.5">
{isLinked ? (
<a
className="link-with-underline truncate text-sans-md"
Expand Down
8 changes: 6 additions & 2 deletions app/ui/lib/Slash.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
*
* Copyright Oxide Computer Company
*/
export const Slash = () => (
<span className="mx-1 text-quaternary selected:text-accent-disabled">/</span>
import cn from 'classnames'

export const Slash = ({ className }: { className?: string }) => (
<span className={cn('mx-1 text-quaternary selected:text-accent-disabled', className)}>
/
</span>
)
2 changes: 1 addition & 1 deletion app/ui/lib/Truncate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const Truncate = ({
// Only use the tooltip if the text is longer than maxLength
return (
// overflow-hidden required to make inner truncate work
<div className="flex items-center space-x-2 overflow-hidden">
<div className="flex items-center gap-0.5 overflow-hidden">
<Tooltip content={text} delay={tooltipDelay}>
<div aria-label={text} className="truncate">
{truncate(text, maxLength, position)}
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/instance-networking.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ test('Instance networking tab — floating IPs', async ({ page }) => {
await expectRowVisible(externalIpTable, { ip: '123.4.56.0', Kind: 'ephemeral' })
await expectRowVisible(externalIpTable, { ip: '123.4.56.5', Kind: 'floating' })

await expect(page.getByText('external IPs123.4.56.5/123.4.56.0')).toBeVisible()

// Attach a new external IP
await attachFloatingIpButton.click()
await expectVisible(page, ['role=heading[name="Attach floating IP"]'])
Expand All @@ -143,9 +145,14 @@ test('Instance networking tab — floating IPs', async ({ page }) => {
// Verify that the "Attach floating IP" button is disabled, since there shouldn't be any more IPs to attach
await expect(attachFloatingIpButton).toBeDisabled()

// Verify that the External IPs table row has an ellipsis link in it
await expect(page.getByText('123.4.56.5/…')).toBeVisible()

// Detach one of the external IPs
await clickRowAction(page, 'cola-float', 'Detach')
await page.getByRole('button', { name: 'Confirm' }).click()
await expect(page.getByText('123.4.56.5/…')).toBeHidden()
await expect(page.getByText('external IPs123.4.56.4/123.4.56.0')).toBeVisible()

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