From d59ccb66f310ad0b547b7e3f4f53081f624c20a6 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 14 Nov 2023 23:15:47 -0500 Subject: [PATCH 01/10] Change text field usages to number field --- app/components/form/fields/DiskSizeField.tsx | 1 - app/forms/firewall-rules-create.tsx | 4 ++-- app/forms/instance-create.tsx | 7 +++---- libs/ui/lib/number-input/NumberInput.tsx | 1 + 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/components/form/fields/DiskSizeField.tsx b/app/components/form/fields/DiskSizeField.tsx index 39afca5b36..2cb72254c8 100644 --- a/app/components/form/fields/DiskSizeField.tsx +++ b/app/components/form/fields/DiskSizeField.tsx @@ -26,7 +26,6 @@ export function DiskSizeField< return ( { - - - Date: Wed, 15 Nov 2023 12:18:43 -0500 Subject: [PATCH 02/10] Infer input type on number field --- app/components/form/fields/NumberField.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/form/fields/NumberField.tsx b/app/components/form/fields/NumberField.tsx index 238e88b119..36f725ad85 100644 --- a/app/components/form/fields/NumberField.tsx +++ b/app/components/form/fields/NumberField.tsx @@ -27,7 +27,7 @@ export function NumberField< helpText, required, ...props -}: Omit, 'id'>) { +}: Omit, 'id' | 'type'>) { // id is omitted from props because we generate it here const id = useId() return ( From 17c6722e28ce1d1195393f02e1776175dc22942d Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 15 Nov 2023 22:53:33 -0500 Subject: [PATCH 03/10] Ensure that TextField can't be of type number --- app/components/form/fields/TextField.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/components/form/fields/TextField.tsx b/app/components/form/fields/TextField.tsx index dc53ff6f3c..a818fb8f7a 100644 --- a/app/components/form/fields/TextField.tsx +++ b/app/components/form/fields/TextField.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ import cn from 'classnames' -import { useId } from 'react' +import { useId, type HTMLInputTypeAttribute } from 'react' import { Controller, type Control, @@ -30,10 +30,10 @@ import { ErrorMessage } from './ErrorMessage' export interface TextFieldProps< TFieldValues extends FieldValues, TName extends FieldPath, -> extends UITextFieldProps { +> extends Omit { name: TName /** HTML type attribute, defaults to text */ - type?: string + type?: Omit /** Will default to name if not provided */ label?: string /** @@ -134,7 +134,7 @@ export const TextFieldInner = < Date: Thu, 16 Nov 2023 12:09:42 -0500 Subject: [PATCH 04/10] Remove number validation logic from TextField --- app/components/form/fields/TextField.tsx | 36 ++---------------------- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/app/components/form/fields/TextField.tsx b/app/components/form/fields/TextField.tsx index a818fb8f7a..35a64e0e18 100644 --- a/app/components/form/fields/TextField.tsx +++ b/app/components/form/fields/TextField.tsx @@ -90,17 +90,9 @@ export function TextField< ) } -function numberToInputValue(value: number) { - // could add `|| value === 0`, but that means when the value is 0, we always - // show an empty string, which is weird, and doubly bad because then the - // browser apparently fails to validate it against minimum (if one is - // provided). I found it let me submit instance create with 0 CPUs. - return isNaN(value) ? '' : value.toString() -} - /** * Primarily exists for `TextField`, but we occasionally also need a plain field - * without a label on it. Note special handling of `type="number"`. + * without a label on it. * * Note that `id` is an allowed prop, unlike in `TextField`, where it is always * generated from `name`. This is because we need to pass the generated ID in @@ -128,7 +120,7 @@ export const TextFieldInner = < name={name} control={control} rules={{ required, validate }} - render={({ field: { onChange, value, ...fieldRest }, fieldState: { error } }) => { + render={({ field, fieldState: { error } }) => { return ( <> { - if (type === 'number') { - if (e.target.value.trim() === '') { - onChange(0) - } else if (!isNaN(e.target.valueAsNumber)) { - onChange(e.target.valueAsNumber) - } - // otherwise ignore the input. this means, for example, typing - // letters does nothing. If we instead said take anything - // that's NaN and fall back to 0, typing a letter would reset - // the field to 0, which is silly. Browsers are supposed to - // ignore non-numeric input for you anyway, but Firefox does - // not. - return - } - - onChange(e.target.value) - }} - value={type === 'number' ? numberToInputValue(value) : value} - {...fieldRest} + {...field} {...props} /> From 0fced775b7b0f28a1721c4bfe12e4861f8e168f7 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 30 Nov 2023 09:20:13 -0500 Subject: [PATCH 05/10] Add transform methods to text and number inputs --- app/components/form/fields/NumberField.tsx | 4 +++- app/components/form/fields/TextField.tsx | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/components/form/fields/NumberField.tsx b/app/components/form/fields/NumberField.tsx index 36f725ad85..08c7fffc33 100644 --- a/app/components/form/fields/NumberField.tsx +++ b/app/components/form/fields/NumberField.tsx @@ -68,6 +68,7 @@ export const NumberFieldInner = < description, required, id: idProp, + transform, }: TextFieldProps) => { const generatedId = useId() const id = idProp || generatedId @@ -77,7 +78,7 @@ export const NumberFieldInner = < name={name} control={control} rules={{ required, validate }} - render={({ field: { value, ...fieldRest }, fieldState: { error } }) => { + render={({ field: { value, onChange, ...fieldRest }, fieldState: { error } }) => { return ( <> onChange(transform ? transform(v) : v)} {...fieldRest} /> diff --git a/app/components/form/fields/TextField.tsx b/app/components/form/fields/TextField.tsx index 35a64e0e18..6bd2509b68 100644 --- a/app/components/form/fields/TextField.tsx +++ b/app/components/form/fields/TextField.tsx @@ -56,6 +56,7 @@ export interface TextFieldProps< units?: string validate?: Validate, TFieldValues> control: Control + transform?: (value: T) => T } export function TextField< @@ -111,6 +112,7 @@ export const TextFieldInner = < description, required, id: idProp, + transform, ...props }: TextFieldProps & UITextAreaProps) => { const generatedId = useId() @@ -120,7 +122,7 @@ export const TextFieldInner = < name={name} control={control} rules={{ required, validate }} - render={({ field, fieldState: { error } }) => { + render={({ field: { onChange, ...fieldRest }, fieldState: { error } }) => { return ( <> + onChange(transform ? transform(e.target.value) : e.target.value) + } + {...fieldRest} {...props} /> From 1d41ae349dc3895fe8699a4eddaf22d03d65a99d Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 30 Nov 2023 11:11:03 -0500 Subject: [PATCH 06/10] Update sdk validation --- libs/api/__generated__/validate.ts | 38 ++++++++++++++++++------------ 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/libs/api/__generated__/validate.ts b/libs/api/__generated__/validate.ts index 3ca5da16d6..cc47cfd499 100644 --- a/libs/api/__generated__/validate.ts +++ b/libs/api/__generated__/validate.ts @@ -117,7 +117,11 @@ export const AddressLot = z.preprocess( */ export const AddressLotBlock = z.preprocess( processResponseBody, - z.object({ firstAddress: z.string(), id: z.string().uuid(), lastAddress: z.string() }) + z.object({ + firstAddress: z.string().ip(), + id: z.string().uuid(), + lastAddress: z.string().ip(), + }) ) /** @@ -125,7 +129,7 @@ export const AddressLotBlock = z.preprocess( */ export const AddressLotBlockCreate = z.preprocess( processResponseBody, - z.object({ firstAddress: z.string(), lastAddress: z.string() }) + z.object({ firstAddress: z.string().ip(), lastAddress: z.string().ip() }) ) /** @@ -284,7 +288,7 @@ export const BgpImportedRouteIpv4 = z.preprocess( export const BgpPeerConfig = z.preprocess( processResponseBody, z.object({ - addr: z.string(), + addr: z.string().ip(), bgpAnnounceSet: NameOrId, bgpConfig: NameOrId, connectRetry: z.number().min(0).max(4294967295), @@ -318,7 +322,7 @@ export const BgpPeerState = z.preprocess( export const BgpPeerStatus = z.preprocess( processResponseBody, z.object({ - addr: z.string(), + addr: z.string().ip(), localAsn: z.number().min(0).max(4294967295), remoteAsn: z.number().min(0).max(4294967295), state: BgpPeerState, @@ -994,7 +998,7 @@ export const IpKind = z.preprocess(processResponseBody, z.enum(['ephemeral', 'fl export const ExternalIp = z.preprocess( processResponseBody, - z.object({ ip: z.string(), kind: IpKind }) + z.object({ ip: z.string().ip(), kind: IpKind }) ) /** @@ -1253,7 +1257,7 @@ export const InstanceNetworkInterfaceCreate = z.preprocess( processResponseBody, z.object({ description: z.string(), - ip: z.string().optional(), + ip: z.string().ip().optional(), name: Name, subnetName: Name, vpcName: Name, @@ -1324,7 +1328,7 @@ export const InstanceNetworkInterface = z.preprocess( description: z.string(), id: z.string().uuid(), instanceId: z.string().uuid(), - ip: z.string(), + ip: z.string().ip(), mac: MacAddr, name: Name, primary: SafeBoolean, @@ -1536,7 +1540,7 @@ export const LoopbackAddress = z.preprocess( export const LoopbackAddressCreate = z.preprocess( processResponseBody, z.object({ - address: z.string(), + address: z.string().ip(), addressLot: NameOrId, anycast: SafeBoolean, mask: z.number().min(0).max(255), @@ -1733,7 +1737,11 @@ export const RoleResultsPage = z.preprocess( */ export const Route = z.preprocess( processResponseBody, - z.object({ dst: IpNet, gw: z.string(), vid: z.number().min(0).max(65535).optional() }) + z.object({ + dst: IpNet, + gw: z.string().ip(), + vid: z.number().min(0).max(65535).optional(), + }) ) /** @@ -1752,7 +1760,7 @@ export const RouteConfig = z.preprocess( export const RouteDestination = z.preprocess( processResponseBody, z.union([ - z.object({ type: z.enum(['ip']), value: z.string() }), + z.object({ type: z.enum(['ip']), value: z.string().ip() }), z.object({ type: z.enum(['ip_net']), value: IpNet }), z.object({ type: z.enum(['vpc']), value: Name }), z.object({ type: z.enum(['subnet']), value: Name }), @@ -1765,7 +1773,7 @@ export const RouteDestination = z.preprocess( export const RouteTarget = z.preprocess( processResponseBody, z.union([ - z.object({ type: z.enum(['ip']), value: z.string() }), + z.object({ type: z.enum(['ip']), value: z.string().ip() }), z.object({ type: z.enum(['vpc']), value: Name }), z.object({ type: z.enum(['subnet']), value: Name }), z.object({ type: z.enum(['instance']), value: Name }), @@ -2155,7 +2163,7 @@ export const SwitchPortApplySettings = z.preprocess( export const SwitchPortBgpPeerConfig = z.preprocess( processResponseBody, z.object({ - addr: z.string(), + addr: z.string().ip(), bgpConfigId: z.string().uuid(), interfaceName: z.string(), portSettingsId: z.string().uuid(), @@ -2429,7 +2437,7 @@ export const VpcFirewallRuleHostFilter = z.preprocess( z.object({ type: z.enum(['vpc']), value: Name }), z.object({ type: z.enum(['subnet']), value: Name }), z.object({ type: z.enum(['instance']), value: Name }), - z.object({ type: z.enum(['ip']), value: z.string() }), + z.object({ type: z.enum(['ip']), value: z.string().ip() }), z.object({ type: z.enum(['ip_net']), value: IpNet }), ]) ) @@ -2468,7 +2476,7 @@ export const VpcFirewallRuleTarget = z.preprocess( z.object({ type: z.enum(['vpc']), value: Name }), z.object({ type: z.enum(['subnet']), value: Name }), z.object({ type: z.enum(['instance']), value: Name }), - z.object({ type: z.enum(['ip']), value: z.string() }), + z.object({ type: z.enum(['ip']), value: z.string().ip() }), z.object({ type: z.enum(['ip_net']), value: IpNet }), ]) ) @@ -3961,7 +3969,7 @@ export const NetworkingLoopbackAddressDeleteParams = z.preprocess( processResponseBody, z.object({ path: z.object({ - address: z.string(), + address: z.string().ip(), rackId: z.string().uuid(), subnetMask: z.number().min(0).max(255), switchLocation: Name, From d1d7c39c689c9a5e022d30bad0d9ef05515132f4 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 30 Nov 2023 11:16:54 -0500 Subject: [PATCH 07/10] Add transform methods to text and number inputs --- app/components/form/fields/DescriptionField.tsx | 2 +- app/components/form/fields/DiskSizeField.tsx | 2 +- app/components/form/fields/NameField.tsx | 2 +- app/components/form/fields/NumberField.tsx | 4 ++-- app/components/form/fields/TextField.tsx | 7 ++++--- app/forms/network-interface-create.tsx | 9 +++++++-- 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/app/components/form/fields/DescriptionField.tsx b/app/components/form/fields/DescriptionField.tsx index 51b2b4ed36..07f48e610e 100644 --- a/app/components/form/fields/DescriptionField.tsx +++ b/app/components/form/fields/DescriptionField.tsx @@ -15,7 +15,7 @@ const MAX_LEN = 512 export function DescriptionField< TFieldValues extends FieldValues, TName extends FieldPath, ->(props: Omit, 'validate'>) { +>(props: Omit, 'validate'>) { return } diff --git a/app/components/form/fields/DiskSizeField.tsx b/app/components/form/fields/DiskSizeField.tsx index 2cb72254c8..2ae7f603a5 100644 --- a/app/components/form/fields/DiskSizeField.tsx +++ b/app/components/form/fields/DiskSizeField.tsx @@ -15,7 +15,7 @@ import type { TextFieldProps } from './TextField' interface DiskSizeProps< TFieldValues extends FieldValues, TName extends FieldPath, -> extends TextFieldProps { +> extends TextFieldProps { minSize?: number } diff --git a/app/components/form/fields/NameField.tsx b/app/components/form/fields/NameField.tsx index fa2e71b0a9..16867058b2 100644 --- a/app/components/form/fields/NameField.tsx +++ b/app/components/form/fields/NameField.tsx @@ -19,7 +19,7 @@ export function NameField< name, label = capitalize(name), ...textFieldProps -}: Omit, 'validate'> & { label?: string }) { +}: Omit, 'validate'> & { label?: string }) { return ( validateName(name, label, required)} diff --git a/app/components/form/fields/NumberField.tsx b/app/components/form/fields/NumberField.tsx index 08c7fffc33..0167a5d21f 100644 --- a/app/components/form/fields/NumberField.tsx +++ b/app/components/form/fields/NumberField.tsx @@ -27,7 +27,7 @@ export function NumberField< helpText, required, ...props -}: Omit, 'id' | 'type'>) { +}: Omit, 'id' | 'type'>) { // id is omitted from props because we generate it here const id = useId() return ( @@ -69,7 +69,7 @@ export const NumberFieldInner = < required, id: idProp, transform, -}: TextFieldProps) => { +}: TextFieldProps) => { const generatedId = useId() const id = idProp || generatedId diff --git a/app/components/form/fields/TextField.tsx b/app/components/form/fields/TextField.tsx index 6bd2509b68..b8e7fca33b 100644 --- a/app/components/form/fields/TextField.tsx +++ b/app/components/form/fields/TextField.tsx @@ -28,6 +28,7 @@ import { capitalize } from '@oxide/util' import { ErrorMessage } from './ErrorMessage' export interface TextFieldProps< + Type, TFieldValues extends FieldValues, TName extends FieldPath, > extends Omit { @@ -56,7 +57,7 @@ export interface TextFieldProps< units?: string validate?: Validate, TFieldValues> control: Control - transform?: (value: T) => T + transform?: (value: Type) => Type | undefined } export function TextField< @@ -70,7 +71,7 @@ export function TextField< helpText, required, ...props -}: Omit, 'id'> & UITextAreaProps) { +}: Omit, 'id'> & UITextAreaProps) { // id is omitted from props because we generate it here const id = useId() return ( @@ -114,7 +115,7 @@ export const TextFieldInner = < id: idProp, transform, ...props -}: TextFieldProps & UITextAreaProps) => { +}: TextFieldProps & UITextAreaProps) => { const generatedId = useId() const id = idProp || generatedId return ( diff --git a/app/forms/network-interface-create.tsx b/app/forms/network-interface-create.tsx index 45e1f0003b..a459c9aa84 100644 --- a/app/forms/network-interface-create.tsx +++ b/app/forms/network-interface-create.tsx @@ -23,7 +23,7 @@ import { useForm, useProjectSelector } from 'app/hooks' const defaultValues: InstanceNetworkInterfaceCreate = { name: '', description: '', - ip: '', + ip: undefined, subnetName: '', vpcName: '', } @@ -80,7 +80,12 @@ export default function CreateNetworkInterfaceForm({ required control={form.control} /> - + (ip.trim() === '' ? undefined : ip)} + /> ) } From 41d47a9398eab81f2be904e05f728b4af806b66e Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 30 Nov 2023 11:28:44 -0500 Subject: [PATCH 08/10] document transform method --- app/components/form/fields/TextField.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/components/form/fields/TextField.tsx b/app/components/form/fields/TextField.tsx index b8e7fca33b..4b075a68c8 100644 --- a/app/components/form/fields/TextField.tsx +++ b/app/components/form/fields/TextField.tsx @@ -57,6 +57,10 @@ export interface TextFieldProps< units?: string validate?: Validate, TFieldValues> control: Control + /** + * This function can be provided to alter the value of the input + * as the input is changed + */ transform?: (value: Type) => Type | undefined } From 6950ca1644c4468000a78e8abb66d7153e02b404 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 30 Nov 2023 15:13:53 -0500 Subject: [PATCH 09/10] Ensure min/max are piped through number field --- app/components/form/fields/NumberField.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/components/form/fields/NumberField.tsx b/app/components/form/fields/NumberField.tsx index 0167a5d21f..7ca8da4b9c 100644 --- a/app/components/form/fields/NumberField.tsx +++ b/app/components/form/fields/NumberField.tsx @@ -69,6 +69,8 @@ export const NumberFieldInner = < required, id: idProp, transform, + min, + max, }: TextFieldProps) => { const generatedId = useId() const id = idProp || generatedId @@ -90,6 +92,8 @@ export const NumberFieldInner = < aria-describedby={description ? `${id}-label-tip` : undefined} defaultValue={value} onChange={(v) => onChange(transform ? transform(v) : v)} + minValue={typeof min !== 'undefined' ? Number(min) : undefined} + maxValue={typeof max !== 'undefined' ? Number(max) : undefined} {...fieldRest} /> From f7497732f1a58344575f1c02cde50d567955353b Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Sat, 2 Dec 2023 13:12:56 -0500 Subject: [PATCH 10/10] Pass name through to number input --- app/test/e2e/firewall-rules.e2e.ts | 2 +- libs/ui/lib/number-input/NumberInput.tsx | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/test/e2e/firewall-rules.e2e.ts b/app/test/e2e/firewall-rules.e2e.ts index 60b8f84f12..081831741d 100644 --- a/app/test/e2e/firewall-rules.e2e.ts +++ b/app/test/e2e/firewall-rules.e2e.ts @@ -118,7 +118,7 @@ test('can update firewall rule', async ({ page }) => { await expect(page.locator('input[name=name]')).toHaveValue('allow-icmp') // priority is populated - await expect(page.locator('input[name=priority]')).toHaveValue('65534') + await expect(page.locator('input[name=priority]')).toHaveValue('65,534') // protocol is populated await expect(page.locator('label >> text=ICMP')).toBeChecked() diff --git a/libs/ui/lib/number-input/NumberInput.tsx b/libs/ui/lib/number-input/NumberInput.tsx index 9114c503f8..690e2336ab 100644 --- a/libs/ui/lib/number-input/NumberInput.tsx +++ b/libs/ui/lib/number-input/NumberInput.tsx @@ -20,6 +20,7 @@ import { useNumberFieldState } from 'react-stately' export type NumberInputProps = { className?: string error?: boolean + name?: string } export const NumberInput = React.forwardRef< @@ -49,6 +50,7 @@ export const NumberInput = React.forwardRef< type="number" {...inputProps} ref={mergeRefs([forwardedRef, inputRef])} + name={props.name} className={cn( `w-full rounded border-none px-3 py-[0.6875rem] !outline-offset-1 text-sans-md