Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
a1cf1bc
Add step to attach floating IPs to instance creation form
charliepark May 17, 2024
1a02de0
Slight refactoring of const names
charliepark May 17, 2024
76001bf
Smoother handling of unchecking Floating IP box
charliepark May 17, 2024
2a98024
Add noItemsPlaceholder to Listbox
charliepark May 17, 2024
c38c181
add type guard to clean up code
charliepark May 17, 2024
597ab34
Remove checkbox; button-only to open modal
charliepark May 17, 2024
9f4b7f6
Merge branch 'main' into add_floating_ips_to_instance_create
charliepark May 20, 2024
a920aea
only show message when IPs are available to attach
charliepark May 20, 2024
7e840b0
Simplify floating IP label, sans-pool
charliepark May 20, 2024
6d211ea
revert removal of api call
charliepark May 20, 2024
de80c5c
Add header to new file
charliepark May 20, 2024
cc66203
Merge branch 'main' into add_floating_ips_to_instance_create
charliepark May 21, 2024
6679cf9
Merge main; disable modal trigger button if no floating IPs available
charliepark May 21, 2024
0074c40
Merge branch 'main' into add_floating_ips_to_instance_create
charliepark May 22, 2024
d6eebce
Add empty state for when no floatingIPs exist
charliepark May 22, 2024
09adbcc
Update app/forms/instance-create.tsx
charliepark May 22, 2024
9c519ce
Refactor deeply-nested JSX
charliepark May 22, 2024
37912dd
Extract FloatingIpLabel, update TipIcon copy
charliepark May 22, 2024
59b27d1
Extract Listbox labels
charliepark May 22, 2024
ecbf0c5
Revert "Extract Listbox labels"
charliepark May 22, 2024
64cb620
Refactor to use useState instead of weird form entry for state manage…
charliepark May 22, 2024
3f1296a
small refactor on onAction
charliepark May 22, 2024
779d301
another small refactor
charliepark May 23, 2024
497f186
refactor to use FloatingIp in useState, rather than a string
charliepark May 23, 2024
4de9b85
simplify and future-proof logic
charliepark May 23, 2024
cdcd2b2
Add test for attaching a floating IP to instance
charliepark May 23, 2024
9c63149
include assertion regarding IP address in modal message
charliepark May 23, 2024
9b6bcfe
Add test for empty floating IPs state
charliepark May 23, 2024
5a45161
Update mock serviceworker to attach IPs to instance; update tests
charliepark May 23, 2024
da18dcc
Move ipFromPool to util function
charliepark May 24, 2024
8e41cf7
refactor
charliepark May 24, 2024
14a2e74
fix the button!
david-crespo May 24, 2024
9a535c9
Merge branch 'main' into add_floating_ips_to_instance_create
david-crespo May 24, 2024
c1af1d1
use RemoveCell in floating IPs MiniTable
david-crespo May 24, 2024
5de89a4
Add test to remove row from form
charliepark May 24, 2024
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
5 changes: 1 addition & 4 deletions app/components/form/fields/ImageSelectField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { Image } from '@oxide/api'

import type { InstanceCreateInput } from '~/forms/instance-create'
import type { ListboxItem } from '~/ui/lib/Listbox'
import { Slash } from '~/ui/lib/Slash'
import { nearest10 } from '~/util/math'
import { bytesToGiB, GiB } from '~/util/units'

Expand Down Expand Up @@ -50,10 +51,6 @@ export function BootDiskImageSelectField({
)
}

const Slash = () => (
<span className="mx-1 text-quinary selected:text-accent-disabled">/</span>
)

export function toListboxItem(i: Image, includeProjectSiloIndicator = false): ListboxItem {
const { name, os, projectId, size, version } = i
const formattedSize = `${bytesToGiB(size, 1)} GiB`
Expand Down
6 changes: 3 additions & 3 deletions app/forms/disk-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { FormDivider } from '~/ui/lib/Divider'
import { FieldLabel } from '~/ui/lib/FieldLabel'
import { Radio } from '~/ui/lib/Radio'
import { RadioGroup } from '~/ui/lib/RadioGroup'
import { Slash } from '~/ui/lib/Slash'
import { toLocaleDateString } from '~/util/date'
import { bytesToGiB, GiB } from '~/util/units'

Expand Down Expand Up @@ -259,9 +260,8 @@ const SnapshotSelectField = ({ control }: { control: Control<DiskCreate> }) => {
<div>{i.name}</div>
<div className="text-tertiary selected:text-accent-secondary">
Created on {toLocaleDateString(i.timeCreated)}
<DiskNameFromId disk={i.diskId} />{' '}
<span className="mx-1 text-quinary selected:text-accent-disabled">/</span>{' '}
{formattedSize.value} {formattedSize.unit}
<DiskNameFromId disk={i.diskId} /> <Slash /> {formattedSize.value}{' '}
{formattedSize.unit}
</div>
</>
),
Expand Down
182 changes: 182 additions & 0 deletions app/forms/instance-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ import {
useApiMutation,
useApiQueryClient,
usePrefetchedApiQuery,
type ExternalIpCreate,
type FloatingIp,
type InstanceCreate,
type InstanceDiskAttachment,
type NameOrId,
} from '@oxide/api'
import {
Images16Icon,
Instances16Icon,
Instances24Icon,
IpGlobal16Icon,
Storage16Icon,
} from '@oxide/design-system/icons/react'

Expand All @@ -50,19 +54,25 @@ import { SshKeysField } from '~/components/form/fields/SshKeysField'
import { TextField } from '~/components/form/fields/TextField'
import { Form } from '~/components/form/Form'
import { FullPageForm } from '~/components/form/FullPageForm'
import { HL } from '~/components/HL'
import { getProjectSelector, useForm, useProjectSelector } from '~/hooks'
import { addToast } from '~/stores/toast'
import { Badge } from '~/ui/lib/Badge'
import { Button } from '~/ui/lib/Button'
import { Checkbox } from '~/ui/lib/Checkbox'
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 { Modal } from '~/ui/lib/Modal'
import { PageHeader, PageTitle } from '~/ui/lib/PageHeader'
import { RadioCard } from '~/ui/lib/Radio'
import { Slash } from '~/ui/lib/Slash'
import { Tabs } from '~/ui/lib/Tabs'
import { TextInputHint } from '~/ui/lib/TextInput'
import { TipIcon } from '~/ui/lib/TipIcon'
import { isTruthy } from '~/util/array'
import { readBlobAsBase64 } from '~/util/file'
import { docLinks, links } from '~/util/links'
import { nearest10 } from '~/util/math'
Expand Down Expand Up @@ -153,6 +163,7 @@ CreateInstanceForm.loader = async ({ params }: LoaderFunctionArgs) => {
}),
apiQueryClient.prefetchQuery('currentUserSshKeyList', {}),
apiQueryClient.prefetchQuery('projectIpPoolList', { query: { limit: 1000 } }),
apiQueryClient.prefetchQuery('floatingIpList', { query: { project, limit: 1000 } }),
])
return null
}
Expand Down Expand Up @@ -573,6 +584,28 @@ export function CreateInstanceForm() {
)
}

// `ip is …` guard is necessary until we upgrade to 5.5, which handles this automatically
const isFloating = (
ip: ExternalIpCreate
): ip is { type: 'floating'; floatingIp: NameOrId } => ip.type === 'floating'

const FloatingIpLabel = ({ ip }: { ip: FloatingIp }) => (
<div>
<div>{ip.name}</div>
<div className="flex gap-0.5 text-tertiary selected:text-accent-secondary">
<div>{ip.ip}</div>
{ip.description && (
<>
<Slash />
<div className="grow overflow-hidden overflow-ellipsis whitespace-pre text-left">
{ip.description}
</div>
</>
)}
</div>
</div>
)

const AdvancedAccordion = ({
control,
isSubmitting,
Expand All @@ -586,11 +619,65 @@ const AdvancedAccordion = ({
// tell, inside AccordionItem, when an accordion is opened so we can scroll its
// contents into view
const [openItems, setOpenItems] = useState<string[]>([])
const [floatingIpModalOpen, setFloatingIpModalOpen] = useState(false)
const [selectedFloatingIp, setSelectedFloatingIp] = useState<FloatingIp | undefined>()
const externalIps = useController({ control, name: 'externalIps' })
const ephemeralIp = externalIps.field.value?.find((ip) => ip.type === 'ephemeral')
const assignEphemeralIp = !!ephemeralIp
const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
const defaultPool = siloPools.find((pool) => pool.isDefault)?.name
const attachedFloatingIps = (externalIps.field.value || []).filter(isFloating)

const { project } = useProjectSelector()
const { data: floatingIpList } = usePrefetchedApiQuery('floatingIpList', {
query: { project, limit: 1000 },
})

// Filter out the IPs that are already attached to an instance
const attachableFloatingIps = useMemo(
() => floatingIpList.items.filter((ip) => !ip.instanceId),
[floatingIpList]
)

// To find available floating IPs, we remove the ones that are already committed to this instance
const availableFloatingIps = attachableFloatingIps.filter(
(ip) => !attachedFloatingIps.find((attachedIp) => attachedIp.floatingIp === ip.name)
)
const attachedFloatingIpsData = attachedFloatingIps
.map((ip) => attachableFloatingIps.find((fip) => fip.name === ip.floatingIp))
.filter(isTruthy)

const closeFloatingIpModal = () => {
setFloatingIpModalOpen(false)
setSelectedFloatingIp(undefined)
}

const attachFloatingIp = () => {
if (selectedFloatingIp) {
externalIps.field.onChange([
...(externalIps.field.value || []),
{ type: 'floating', floatingIp: selectedFloatingIp.name },
])
}
closeFloatingIpModal()
}

const detachFloatingIp = (name: string) => {
externalIps.field.onChange(
externalIps.field.value?.filter(
(ip) => !(ip.type === 'floating' && ip.floatingIp === name)
)
)
}

const isFloatingIpAttached = attachedFloatingIps.some((ip) => ip.floatingIp !== '')

const selectedFloatingIpMessage = (
<>
This instance will be reachable at{' '}
{selectedFloatingIp ? <HL>{selectedFloatingIp.ip}</HL> : 'the selected IP'}
</>
)

return (
<Accordion.Root
Expand Down Expand Up @@ -669,6 +756,101 @@ const AdvancedAccordion = ({
/>
)}
</div>

<div className="flex flex-1 flex-col gap-4">
<h2 className="text-sans-md">
Floating IPs{' '}
<TipIcon>
Floating IPs exist independently of instances and can be attached to and
detached from them as needed.
</TipIcon>
</h2>
{isFloatingIpAttached && (
<MiniTable.Table>
<MiniTable.Header>
<MiniTable.HeadCell>Name</MiniTable.HeadCell>
<MiniTable.HeadCell>IP</MiniTable.HeadCell>
{/* For remove button */}
<MiniTable.HeadCell className="w-12" />
</MiniTable.Header>
<MiniTable.Body>
{attachedFloatingIpsData.map((item, index) => (
<MiniTable.Row
tabIndex={0}
aria-rowindex={index + 1}
aria-label={`Name: ${item.name}, IP: ${item.ip}`}
key={item.name}
>
<MiniTable.Cell>{item.name}</MiniTable.Cell>
<MiniTable.Cell>{item.ip}</MiniTable.Cell>
<MiniTable.RemoveCell
onClick={() => detachFloatingIp(item.name)}
label={`remove floating IP ${item.name}`}
/>
</MiniTable.Row>
))}
</MiniTable.Body>
</MiniTable.Table>
)}
{floatingIpList.items.length === 0 ? (
Copy link
Collaborator

@david-crespo david-crespo May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be available IPs? You still can't do anything if floating IPs exist but are taken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is that if the list of floating IPs coming in from the API query has 0 items, we show them the empty message; if there are any items in that list at all (attached or not), we show them the Attach floating IP button. That button then is either disabled (no available floating IPs) or enabled (at least one available floating IP). But the disabled message on the button would indicate that no floating IPs are available. Let me know if that didn't make sense, or if there's an alternate flow you would prefer to see there?

Copy link
Collaborator

@david-crespo david-crespo May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what felt funny to me is that within this form, I'm not sure the distinction between "there are no floating IPs at all" and "there are floating IPs but they're all taken" is meaningful to the user. We are treating it as meaningful by representing those two states differently. Here are all the states:

State UI representation
no floating IPs at all empty state like the tables
floating IPs exist but are attached to other instances disabled attach button
floating IPs exist, none available, 1+ attached to this instance mini table + disabled attach button
floating IPs available, 1+ attached to this instance mini table + enabled attach button
floating IPs available, none attached to this instance enabled attach button

I'm wondering if we should give the first two rows the same representation. This can't matter that much because the user is never going to see both versions next to each other and wonder why they're different, but I'm finding it helpful to think through it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we consolidated them, which of the two treatments feels better to you? Just the disabled button (with no empty state)? That would certainly be less "involved" on an already pretty-lengthy page. Though the empty state does call your eye to it, and for something as foundational as a floating IP, I could see it making sense to have it called out like that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree there is something nice about the full empty state almost inviting the user to create one. @benjaminleonard since you weighed in before, what do you think?

<div className="flex max-w-lg items-center justify-center rounded-lg border p-6 border-default">
<EmptyMessage
icon={<IpGlobal16Icon />}
title="No floating IPs found"
body="Create a floating IP to attach it to this instance"
/>
</div>
) : (
<div>
<Button
size="sm"
className="shrink-0"
disabled={availableFloatingIps.length === 0}
disabledReason="No floating IPs available"
onClick={() => setFloatingIpModalOpen(true)}
>
Attach floating IP
</Button>
</div>
)}

<Modal
isOpen={floatingIpModalOpen}
onDismiss={closeFloatingIpModal}
title="Attach floating IP"
>
<Modal.Body>
<Modal.Section>
<Message variant="info" content={selectedFloatingIpMessage} />
<form>
<Listbox
name="floatingIp"
items={availableFloatingIps.map((i) => ({
value: i.name,
label: <FloatingIpLabel ip={i} />,
selectedLabel: `${i.name} (${i.ip})`,
}))}
label="Floating IP"
onChange={(name) => {
setSelectedFloatingIp(
availableFloatingIps.find((i) => i.name === name)
)
}}
required
placeholder="Select floating IP"
selected={selectedFloatingIp?.name || ''}
/>
</form>
</Modal.Section>
</Modal.Body>
<Modal.Footer
actionText="Attach"
disabled={!selectedFloatingIp}
onAction={attachFloatingIp}
onDismiss={closeFloatingIpModal}
></Modal.Footer>
</Modal>
</div>
</AccordionItem>
<AccordionItem
value="configuration"
Expand Down
3 changes: 2 additions & 1 deletion app/pages/project/floating-ips/AttachFloatingIpModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { ListboxField } from '~/components/form/fields/ListboxField'
import { addToast } from '~/stores/toast'
import { Message } from '~/ui/lib/Message'
import { Modal } from '~/ui/lib/Modal'
import { Slash } from '~/ui/lib/Slash'

function FloatingIpLabel({ fip }: { fip: FloatingIp }) {
return (
Expand All @@ -22,7 +23,7 @@ function FloatingIpLabel({ fip }: { fip: FloatingIp }) {
<div>{fip.ip}</div>
{fip.description && (
<>
<span className="mx-1 text-quinary selected:text-accent-disabled">/</span>
<Slash />
<div className="grow overflow-hidden overflow-ellipsis whitespace-pre text-left">
{fip.description}
</div>
Expand Down
4 changes: 3 additions & 1 deletion app/ui/lib/Listbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface ListboxProps<Value extends string = string> {
onChange: (value: Value) => void
items: ListboxItem<Value>[]
placeholder?: string
noItemsPlaceholder?: string
className?: string
disabled?: boolean
hasError?: boolean
Expand All @@ -48,6 +49,7 @@ export const Listbox = <Value extends string = string>({
selected,
items,
placeholder = 'Select an option',
noItemsPlaceholder = 'No items',
className,
onChange,
hasError = false,
Expand Down Expand Up @@ -107,7 +109,7 @@ export const Listbox = <Value extends string = string>({
selectedItem.selectedLabel || selectedItem.label
) : (
<span className="text-quaternary">
{noItems ? 'No items' : placeholder}
{noItems ? noItemsPlaceholder : placeholder}
</span>
)}
</div>
Expand Down
10 changes: 10 additions & 0 deletions app/ui/lib/Slash.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* 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
*/
export const Slash = () => (
<span className="mx-1 text-quinary selected:text-accent-disabled">/</span>
)
12 changes: 12 additions & 0 deletions mock-api/msw/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ export const lookupById = <T extends { id: string }>(table: T[], id: string) =>
return item
}

export const getIpFromPool = (poolName: string | undefined) => {
const pool = lookup.ipPool({ pool: poolName })
const ipPoolRange = db.ipPoolRanges.find((range) => range.ip_pool_id === pool.id)
if (!ipPoolRange) throw notFoundErr

// right now, we're just using the first address in the range, but we'll
// want to filter the list of available IPs for the first unused address
// also: think through how calling code might want to handle various issues
// and what appropriate error codes would be: no ranges? pool is exhausted? etc.
return ipPoolRange.range.first
}

export const lookup = {
project({ project: id }: PP.Project): Json<Api.Project> {
if (!id) throw notFoundErr
Expand Down
Loading