From 351f65928b1c53d15f2c01931f9c3b5715675432 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 1 Apr 2022 02:45:27 -0400 Subject: [PATCH 01/32] WIP instance form work --- app/forms/index.ts | 7 + app/forms/instance-create.tsx | 378 ++++++++++++++++++ app/hooks/use-form.tsx | 5 +- app/pages/project/instances/create/index.ts | 1 - .../instances/create/instance-types.ts | 89 ----- app/routes.tsx | 3 +- libs/form/Form.tsx | 15 +- libs/form/fields/RadioField.tsx | 18 +- libs/form/fields/TableField.tsx | 71 ++++ libs/form/fields/index.ts | 1 + libs/form/form.css | 5 + libs/form/types.ts | 6 + libs/ui/lib/field-label/FieldLabel.tsx | 30 +- libs/ui/lib/mini-table/mini-table.css | 9 +- libs/ui/lib/radio/Radio.tsx | 12 +- libs/ui/lib/tabs/Tabs.tsx | 9 +- 16 files changed, 515 insertions(+), 144 deletions(-) create mode 100644 app/forms/instance-create.tsx delete mode 100644 app/pages/project/instances/create/instance-types.ts create mode 100644 libs/form/fields/TableField.tsx diff --git a/app/forms/index.ts b/app/forms/index.ts index 9e757e4f9a..68962c1de0 100644 --- a/app/forms/index.ts +++ b/app/forms/index.ts @@ -3,6 +3,8 @@ import type { EditSubnetForm } from './subnet-edit' import type { CreateOrgForm } from './org-create' import type { CreateDiskForm } from './disk-create' import type { CreateProjectForm } from './project-create' +import type CreateInstanceForm from './instance-create' +import type { ExtractFormValues } from '@oxide/form' /** * A map of all existing forms. When a new form is created in the forms directory, a @@ -12,7 +14,12 @@ import type { CreateProjectForm } from './project-create' export interface FormTypes { 'org-create': typeof CreateOrgForm 'project-create': typeof CreateProjectForm + 'instance-create': typeof CreateInstanceForm 'disk-create': typeof CreateDiskForm 'subnet-create': typeof CreateSubnetForm 'subnet-edit': typeof EditSubnetForm } + +export type FormValues = ExtractFormValues< + FormTypes[K] +> diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx new file mode 100644 index 0000000000..36022bc221 --- /dev/null +++ b/app/forms/instance-create.tsx @@ -0,0 +1,378 @@ +import { useApiMutation, useApiQueryClient } from '@oxide/api' +import type { PrebuiltFormProps } from '@oxide/form' +import { TableField } from '@oxide/form' +import { TextField } from '@oxide/form' +import { RadioField } from '@oxide/form' +import { TagsField } from '@oxide/form' +import { DescriptionField, NameField } from '@oxide/form' +import { Form } from '@oxide/form' +import { + CentOSResponsiveIcon, + DebianResponsiveIcon, + Divider, + FedoraResponsiveIcon, + FreeBSDResponsiveIcon, + RadioCard, + Success16Icon, + Tab, + Tabs, + TextFieldHint, + UbuntuResponsiveIcon, + WindowsResponsiveIcon, +} from '@oxide/ui' +import { useParams, useToast } from 'app/hooks' +import { useForm } from 'app/hooks/use-form' +import filesize from 'filesize' +import React from 'react' +import type { FormValues } from '.' + +const values = { + name: '', + description: '', + tags: {}, + type: '', + hostname: '', + newDisks: [], + attachedDisks: [], +} + +export default function CreateInstanceForm({ + id = 'create-instance-form', + title = 'Create instance', + initialValues = values, + onSubmit, + onSuccess, + onError, + ...props +}: PrebuiltFormProps) { + const queryClient = useApiQueryClient() + const addToast = useToast() + const [createDiskForm, showDiskForm] = useForm('disk-create') + + const { orgName, projectName } = useParams('orgName', 'projectName') + + const createInstance = useApiMutation('projectInstancesPost', { + onSuccess(instance) { + // refetch list of instances + queryClient.invalidateQueries('projectInstancesGet', { + orgName, + projectName, + }) + // avoid the instance fetch when the instance page loads since we have the data + queryClient.setQueryData( + 'projectInstancesGetInstance', + { orgName, projectName, instanceName: instance.name }, + instance + ) + addToast({ + icon: , + title: 'Success!', + content: 'Your instance has been created.', + timeout: 5000, + }) + onSuccess?.(instance) + }, + onError, + }) + + return ( +
{ + const instance = INSTANCE_SIZES.find( + (option) => option.id === values['type'] + ) || { memory: 0, ncpus: 0 } + createInstance.mutate({ + orgName, + projectName, + body: { + name: values['name'], + hostname: values.hostname, + description: `An instance in project: ${projectName}`, + memory: filesize(instance.memory, { output: 'object', base: 2 }) + .value, + ncpus: instance.ncpus, + }, + }) + }) + } + mutation={createInstance} + {...props} + > + + + + + + + Hardware + + General Purpose + + + General purpose instances provide a good balance of CPU, memory, and + high performance storage; well suited for a wide range of use cases. + + + {renderLargeRadioCards('general')} + + + + CPU Optimized + + + CPU optimized instances provide a good balance of... + + + {renderLargeRadioCards('cpuOptimized')} + + + + Memory optimized + + + CPU optimized instances provide a good balance of... + + + {renderLargeRadioCards('memoryOptimized')} + + + + Custom + + + Custom instances... + + + {renderLargeRadioCards('custom')} + + + + + + + Boot disk + + Distros + + + {renderDistroRadioCard({ + label: 'Ubuntu', + value: 'ubuntu', + Icon: UbuntuResponsiveIcon, + })} + {renderDistroRadioCard({ + label: 'FreeBSD', + value: 'freeBsd', + Icon: FreeBSDResponsiveIcon, + })} + {renderDistroRadioCard({ + label: 'Fedora', + value: 'fedora', + Icon: FedoraResponsiveIcon, + })} + {renderDistroRadioCard({ + label: 'Debian', + value: 'debian', + Icon: DebianResponsiveIcon, + })} + {renderDistroRadioCard({ + label: 'CentOS', + value: 'centos', + Icon: CentOSResponsiveIcon, + })} + {renderDistroRadioCard({ + label: 'Windows', + value: 'windows', + Icon: WindowsResponsiveIcon, + })} + + + + + Images + + Snapshots + + + + Additional disks + + > + id="new-disks" + name="New disks" + actionText="Create new disk" + onAddItem={(addDisk) => { + const hideForm = showDiskForm({ + onSubmit: (values) => { + addDisk(values) + hideForm() + }, + }) + }} + columns={[ + ['name', 'Name'], + ['size', 'Size', filesize], + ]} + /> + {createDiskForm} + + {/* */} + + + Networking + + + + ) +} + +interface DistroRadioCardProps { + label: string + value: string + Icon: React.ComponentType<{ className: string }> +} +const renderDistroRadioCard = ({ + label, + value, + Icon, +}: DistroRadioCardProps) => { + return ( + +
+ + {label} +
+
+ ) +} + +const renderLargeRadioCards = (category: string) => { + return INSTANCE_SIZES.filter((option) => option.category === category).map( + (option) => ( + +
+ {option.ncpus}{' '} + + CPU{option.ncpus === 1 ? '' : 's'} + +
+
+ {option.memory}{' '} + GB RAM +
+
+ ) + ) +} + +// This data structure is completely made up for the purposes of demonstration +// only. It is not meant to reflect any opinions on how the backend API endpoint +// should be structured. Thank you for reading and have a good day! +const INSTANCE_SIZES = [ + { + category: 'general', + id: 'general-xs', + memory: 2, + ncpus: 1, + }, + { + category: 'general', + id: 'general-sm', + memory: 4, + ncpus: 2, + }, + { + category: 'general', + id: 'general-med', + memory: 16, + ncpus: 4, + }, + { + category: 'general', + id: 'general-lg', + memory: 24, + ncpus: 6, + }, + { + category: 'general', + id: 'general-xl', + memory: 32, + ncpus: 8, + }, + { + category: 'cpuOptimized', + id: 'cpuOptimized-xs', + memory: 3, + ncpus: 1, + }, + { + category: 'cpuOptimized', + id: 'cpuOptimized-sm', + memory: 5, + ncpus: 3, + }, + { + category: 'cpuOptimized', + id: 'cpuOptimized-med', + memory: 7, + ncpus: 5, + }, + { + category: 'memoryOptimized', + id: 'memoryOptimized-xs', + memory: 3, + ncpus: 2, + }, + { + category: 'memoryOptimized', + id: 'memoryOptimized-sm', + memory: 5, + ncpus: 3, + }, + { + category: 'memoryOptimized', + id: 'memoryOptimized-med', + memory: 17, + ncpus: 5, + }, + { + category: 'custom', + id: 'custom-xs', + memory: 2, + ncpus: 1, + }, + { + category: 'custom', + id: 'custom-sm', + memory: 4, + ncpus: 2, + }, + { + category: 'custom', + id: 'custom-med', + memory: 16, + ncpus: 4, + }, +] diff --git a/app/hooks/use-form.tsx b/app/hooks/use-form.tsx index 041b498697..d6743a0b2f 100644 --- a/app/hooks/use-form.tsx +++ b/app/hooks/use-form.tsx @@ -22,11 +22,12 @@ export const useForm = ( const [isOpen, setShowForm] = useState(false) const [formProps, setFormProps] = useState | undefined>(props) - const invokeForm = (props?: FormProps) => { + const showForm = (props?: FormProps) => { if (props) { setFormProps(props) } setShowForm(true) + return () => setShowForm(false) } const onDismiss = useCallback(() => { @@ -53,6 +54,6 @@ export const useForm = ( , - invokeForm, + showForm, ] as const } diff --git a/app/pages/project/instances/create/index.ts b/app/pages/project/instances/create/index.ts index 41849f6a2a..e69de29bb2 100644 --- a/app/pages/project/instances/create/index.ts +++ b/app/pages/project/instances/create/index.ts @@ -1 +0,0 @@ -export * from './InstancesCreatePage' diff --git a/app/pages/project/instances/create/instance-types.ts b/app/pages/project/instances/create/instance-types.ts deleted file mode 100644 index 89807de227..0000000000 --- a/app/pages/project/instances/create/instance-types.ts +++ /dev/null @@ -1,89 +0,0 @@ -// This data structure is completely made up for the purposes of demonstration -// only. It is not meant to reflect any opinions on how the backend API endpoint -// should be structured. Thank you for reading and have a good day! -export const INSTANCE_SIZES = [ - { - category: 'general', - id: 'general-xs', - memory: 2, - ncpus: 1, - }, - { - category: 'general', - id: 'general-sm', - memory: 4, - ncpus: 2, - }, - { - category: 'general', - id: 'general-med', - memory: 16, - ncpus: 4, - }, - { - category: 'general', - id: 'general-lg', - memory: 24, - ncpus: 6, - }, - { - category: 'general', - id: 'general-xl', - memory: 32, - ncpus: 8, - }, - { - category: 'cpuOptimized', - id: 'cpuOptimized-xs', - memory: 3, - ncpus: 1, - }, - { - category: 'cpuOptimized', - id: 'cpuOptimized-sm', - memory: 5, - ncpus: 3, - }, - { - category: 'cpuOptimized', - id: 'cpuOptimized-med', - memory: 7, - ncpus: 5, - }, - { - category: 'memoryOptimized', - id: 'memoryOptimized-xs', - memory: 3, - ncpus: 2, - }, - { - category: 'memoryOptimized', - id: 'memoryOptimized-sm', - memory: 5, - ncpus: 3, - }, - { - category: 'memoryOptimized', - id: 'memoryOptimized-med', - memory: 17, - ncpus: 5, - }, - { - category: 'custom', - id: 'custom-xs', - memory: 2, - ncpus: 1, - }, - { - category: 'custom', - id: 'custom-sm', - memory: 4, - ncpus: 2, - }, - { - category: 'custom', - id: 'custom-med', - memory: 16, - ncpus: 4, - }, -] diff --git a/app/routes.tsx b/app/routes.tsx index 1cc2794057..e2e09c0525 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -4,7 +4,6 @@ import type { RouteMatch, RouteObject } from 'react-router-dom' import { Navigate, Route, Routes } from 'react-router-dom' import LoginPage from './pages/LoginPage' -import InstanceCreatePage from './pages/project/instances/create/InstancesCreatePage' import { AccessPage, DisksPage, @@ -111,7 +110,7 @@ export const routes = ( } /> } + element={} title="Create instance" icon={} /> diff --git a/libs/form/Form.tsx b/libs/form/Form.tsx index 46f98c29dd..f9ec09527b 100644 --- a/libs/form/Form.tsx +++ b/libs/form/Form.tsx @@ -2,14 +2,12 @@ import type { ButtonProps } from '@oxide/ui' import { Button } from '@oxide/ui' import { SideModal } from '@oxide/ui' import { useIsInSideModal } from '@oxide/ui' -import { Divider } from '@oxide/ui' import { addProps, classed, flattenChildren, invariant, isOneOf, - kebabCase, pluckFirstOfType, tunnel, Wrap, @@ -77,7 +75,7 @@ export function Form({ <>
( Form.PageActions = PageActionsTunnel.Out -const FormHeading = classed.h2`ox-form-heading text-content text-sans-2xl` +Form.Heading = classed.h2`ox-form-heading text-content text-sans-2xl` export interface FormSectionProps { id?: string children: React.ReactNode title: string } -Form.Section = ({ id, title, children }: FormSectionProps) => { - return ( - <> - - {title} - {children} - - ) -} diff --git a/libs/form/fields/RadioField.tsx b/libs/form/fields/RadioField.tsx index 7b798eba57..413e47806e 100644 --- a/libs/form/fields/RadioField.tsx +++ b/libs/form/fields/RadioField.tsx @@ -1,6 +1,7 @@ import type { RadioGroupProps } from '@oxide/ui' import { FieldLabel, RadioGroup, TextFieldHint } from '@oxide/ui' import { capitalize } from '@oxide/util' +import cn from 'classnames' import React from 'react' // TODO: Centralize these docstrings perhaps on the `FieldLabel` component? @@ -32,16 +33,23 @@ export interface RadioFieldProps extends Omit { export function RadioField({ id, name = id, - label = capitalize(name), + label, helpText, description, ...props }: RadioFieldProps) { return ( -
- - {label} - +
+ {label && ( + + {label} + + )} {/* TODO: Figure out where this hint field def should live */} {helpText && ( {helpText} diff --git a/libs/form/fields/TableField.tsx b/libs/form/fields/TableField.tsx new file mode 100644 index 0000000000..80672581f5 --- /dev/null +++ b/libs/form/fields/TableField.tsx @@ -0,0 +1,71 @@ +import { useField } from 'formik' +import { Error16Icon } from '@oxide/ui' +import { Button, FieldLabel, MiniTable } from '@oxide/ui' +import { capitalize } from '@oxide/util' +import React from 'react' + +// TODO: Simplify this type +type Column = K extends keyof (infer Item) + ? [key: K, name: string, format?: (value: Item[K]) => string] + : never + +export interface TableFieldProps> { + id: string + name?: string + label?: string + actionText: string + onAddItem: (addItem: (item: Item) => void) => void + onRemoveItem?: (item: Item) => void + columns: Column[] +} + +export function TableField>({ + id, + name = id, + label = capitalize(name), + actionText, + onAddItem, + onRemoveItem, + columns, +}: TableFieldProps) { + const [, { value = [] }, { setValue }] = useField({ name }) + + return ( +
+ {label} + {!!value.length && ( + + + {columns.map(([key, display]) => ( + {display} + ))} + {/* For remove button */} + + + + {value.map((item, index) => ( + + {columns.map(([key]) => ( + + {item[key]} + + ))} + + + + + ))} + + + )} + +
+ ) +} diff --git a/libs/form/fields/index.ts b/libs/form/fields/index.ts index 8777a1d04c..e86498ca07 100644 --- a/libs/form/fields/index.ts +++ b/libs/form/fields/index.ts @@ -3,3 +3,4 @@ export * from './TextField' export * from './NameField' export * from './TagsField' export * from './RadioField' +export * from './TableField' diff --git a/libs/form/form.css b/libs/form/form.css index 869d8dcd75..e2d24ee8b5 100644 --- a/libs/form/form.css +++ b/libs/form/form.css @@ -16,3 +16,8 @@ width: calc(100% + var(--content-gutter) * 2) !important; margin-left: calc(var(--content-gutter) * -1) !important; } + +.ox-form, +.ox-form .ox-tab-panel { + @apply space-y-7; +} diff --git a/libs/form/types.ts b/libs/form/types.ts index a4c942b81d..2fa2a81fbb 100644 --- a/libs/form/types.ts +++ b/libs/form/types.ts @@ -27,3 +27,9 @@ export type ExtendedPrebuiltFormProps = C extends ComponentType ? B : never : never + +export type ExtractFormValues = C extends ComponentType< + PrebuiltFormProps +> + ? V + : never diff --git a/libs/ui/lib/field-label/FieldLabel.tsx b/libs/ui/lib/field-label/FieldLabel.tsx index 5b3c22bf69..86a9a82542 100644 --- a/libs/ui/lib/field-label/FieldLabel.tsx +++ b/libs/ui/lib/field-label/FieldLabel.tsx @@ -5,40 +5,22 @@ import { Info8Icon, Tooltip } from '@oxide/ui' /** * Ensures that label always has an `htmlFor` prop associated with it */ -type FieldLabelProps = ( - | { - id: string - htmlFor?: string - as?: never - } - | { - as: 'label' - htmlFor: string - id?: never - } - | { - as?: never - htmlFor: string - id?: never - } - | { - as: Exclude - htmlFor?: string - id?: never - } -) & { +interface FieldLabelProps { + id: string + as?: ElementType + htmlFor?: string tip?: string optional?: boolean } -export const FieldLabel = ({ +export const FieldLabel = ({ id, children, htmlFor, tip, optional, as, -}: PropsWithChildren>) => { +}: PropsWithChildren) => { const Component = as || 'label' return (
diff --git a/libs/ui/lib/mini-table/mini-table.css b/libs/ui/lib/mini-table/mini-table.css index 5e1ee4d352..16ba8c1f74 100644 --- a/libs/ui/lib/mini-table/mini-table.css +++ b/libs/ui/lib/mini-table/mini-table.css @@ -24,7 +24,14 @@ } .ox-mini-table td > div { - @apply border-y py-3 pl-3 text-default bg-default border-accent; + @apply flex h-11 items-center border-y py-3 pl-3 text-default bg-default border-accent; +} + +.ox-mini-table td:last-child > div { + @apply w-12 justify-center pl-0 pr-0; +} +.ox-mini-table td:last-child > div > button { + @apply pl-4; } .ox-mini-table tr:hover td > div { diff --git a/libs/ui/lib/radio/Radio.tsx b/libs/ui/lib/radio/Radio.tsx index adc171d2da..a197f59188 100644 --- a/libs/ui/lib/radio/Radio.tsx +++ b/libs/ui/lib/radio/Radio.tsx @@ -6,7 +6,7 @@ * difference is that label content is handled through children. */ -import type { PropsWithChildren } from 'react' +import type { ComponentProps, PropsWithChildren } from 'react' import React from 'react' import cn from 'classnames' import { Field } from 'formik' @@ -72,6 +72,12 @@ export function RadioCard({ children, className, ...inputProps }: RadioProps) { } // TODO: Remove importants after tailwind variantOrder bug fixed -RadioCard.Unit = ({ children }: PropsWithChildren) => ( - {children} +RadioCard.Unit = ({ + children, + className, + ...props +}: ComponentProps<'span'>) => ( + + {children} + ) diff --git a/libs/ui/lib/tabs/Tabs.tsx b/libs/ui/lib/tabs/Tabs.tsx index 463dcf697d..1642e996e9 100644 --- a/libs/ui/lib/tabs/Tabs.tsx +++ b/libs/ui/lib/tabs/Tabs.tsx @@ -45,12 +45,13 @@ export function Tabs({ addKey((i) => `${id}-tab-${i}`) ) const panels = pluckAllOfType(childArray, Tab.Panel).map( - addProps((i) => ({ + addProps((i, panelProps) => ({ key: `${id}-panel-${i}`, index: i, className: cn( fullWidth && - 'children:mx-[var(--content-gutter)] children:w-[calc(100%-var(--content-gutter)*2)]' + 'children:mx-[var(--content-gutter)] children:w-[calc(100%-var(--content-gutter)*2)]', + panelProps.className ), })) ) @@ -98,7 +99,7 @@ export function Tab({ className, ...props }: TabProps) { export interface TabPanelProps extends RTabPanelProps { className?: string } -Tab.Panel = function Panel({ children, ...props }: TabPanelProps) { +Tab.Panel = function Panel({ children, className, ...props }: TabPanelProps) { const { selectedIndex } = useTabsContext() // `index` is a secret prop that's automatically generated by the parents tab // component. We use it here to determine if the panel's contents should @@ -108,7 +109,7 @@ Tab.Panel = function Panel({ children, ...props }: TabPanelProps) { // eslint-disable-next-line @typescript-eslint/no-explicit-any const isSelected = selectedIndex === (props as any).index return ( - + {(isSelected && children) || } ) From 914effffb7bc919000becdabadd164eadb5a067e Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 1 Apr 2022 10:57:31 -0400 Subject: [PATCH 02/32] Fix button size --- libs/form/fields/TableField.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/form/fields/TableField.tsx b/libs/form/fields/TableField.tsx index 80672581f5..e3b7050c36 100644 --- a/libs/form/fields/TableField.tsx +++ b/libs/form/fields/TableField.tsx @@ -62,7 +62,9 @@ export function TableField>({ )} From e8d164736d88916a03844b848694f8b4344db40b Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 1 Apr 2022 11:55:16 -0400 Subject: [PATCH 03/32] Minor accessibilty updates to the table --- libs/form/fields/TableField.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libs/form/fields/TableField.tsx b/libs/form/fields/TableField.tsx index e3b7050c36..d1e71c3aa2 100644 --- a/libs/form/fields/TableField.tsx +++ b/libs/form/fields/TableField.tsx @@ -31,7 +31,7 @@ export function TableField>({ const [, { value = [] }, { setValue }] = useField({ name }) return ( -
+
{label} {!!value.length && ( @@ -44,7 +44,14 @@ export function TableField>({ {value.map((item, index) => ( - + `${name}: ${item[key]}`) + .join(' ')} + key={`item-${index}`} + > {columns.map(([key]) => ( {item[key]} @@ -52,7 +59,7 @@ export function TableField>({ ))} From fa4b299d841736294ff25cd4882e79e9949cbfcc Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 1 Apr 2022 13:45:09 -0400 Subject: [PATCH 04/32] Wire up dismiss states --- libs/form/fields/TableField.tsx | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/libs/form/fields/TableField.tsx b/libs/form/fields/TableField.tsx index d1e71c3aa2..5eac2a0f59 100644 --- a/libs/form/fields/TableField.tsx +++ b/libs/form/fields/TableField.tsx @@ -4,12 +4,14 @@ import { Button, FieldLabel, MiniTable } from '@oxide/ui' import { capitalize } from '@oxide/util' import React from 'react' +type NamedItem = { name: string; [key: string]: any } + // TODO: Simplify this type type Column = K extends keyof (infer Item) ? [key: K, name: string, format?: (value: Item[K]) => string] : never -export interface TableFieldProps> { +export interface TableFieldProps { id: string name?: string label?: string @@ -19,7 +21,7 @@ export interface TableFieldProps> { columns: Column[] } -export function TableField>({ +export function TableField({ id, name = id, label = capitalize(name), @@ -34,7 +36,7 @@ export function TableField>({
{label} {!!value.length && ( - + {columns.map(([key, display]) => ( {display} @@ -58,7 +60,13 @@ export function TableField>({ ))} - @@ -71,7 +79,6 @@ export function TableField>({ variant="secondary" size="sm" onClick={() => onAddItem((item) => setValue(value.concat(item)))} - className="mt-4" > {actionText} From 6399c8d6e3644e93105f73f08f3346fb369c0218 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 1 Apr 2022 16:32:45 -0400 Subject: [PATCH 05/32] Improve tests, name validation --- app/forms/index.spec.ts | 1 + app/forms/index.ts | 2 +- app/forms/instance-create.tsx | 1 + libs/form/fields/NameField.spec.tsx | 21 +++++++++++---------- libs/form/fields/NameField.tsx | 28 +++++++++++++++++----------- libs/util/str.spec.ts | 5 ----- libs/util/str.ts | 3 +-- 7 files changed, 32 insertions(+), 29 deletions(-) diff --git a/app/forms/index.spec.ts b/app/forms/index.spec.ts index 56c931b566..c6dc616226 100644 --- a/app/forms/index.spec.ts +++ b/app/forms/index.spec.ts @@ -2,6 +2,7 @@ import babel from '@babel/core' import { traverse } from '@babel/core' import fs from 'fs/promises' import path from 'path' +import './index' test('FormTypes must contain references to all forms', async () => { let formIds: string[] = [] diff --git a/app/forms/index.ts b/app/forms/index.ts index 68962c1de0..884d317342 100644 --- a/app/forms/index.ts +++ b/app/forms/index.ts @@ -12,9 +12,9 @@ import type { ExtractFormValues } from '@oxide/form' * and a value of the form's type. There's a test to validate that this happens. */ export interface FormTypes { + 'instance-create': typeof CreateInstanceForm 'org-create': typeof CreateOrgForm 'project-create': typeof CreateProjectForm - 'instance-create': typeof CreateInstanceForm 'disk-create': typeof CreateDiskForm 'subnet-create': typeof CreateSubnetForm 'subnet-edit': typeof EditSubnetForm diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 36022bc221..5cf6e0afa8 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -194,6 +194,7 @@ export default function CreateInstanceForm({ { + const validate = validateName('Name', true) it('returns undefined for valid names', () => { - expect(validateName('abc')).toBeUndefined() - expect(validateName('abc-def')).toBeUndefined() - expect(validateName('abc9-d0ef-6')).toBeUndefined() + expect(validate('abc')).toBeUndefined() + expect(validate('abc-def')).toBeUndefined() + expect(validate('abc9-d0ef-6')).toBeUndefined() }) it('detects names starting with something other than lower-case letter', () => { - expect(validateName('Abc')).toEqual('Must start with a lower-case letter') - expect(validateName('9bc')).toEqual('Must start with a lower-case letter') - expect(validateName('Abc-')).toEqual('Must start with a lower-case letter') + expect(validate('Abc')).toEqual('Must start with a lower-case letter') + expect(validate('9bc')).toEqual('Must start with a lower-case letter') + expect(validate('Abc-')).toEqual('Must start with a lower-case letter') }) it('requires names to end with letter or number', () => { - expect(validateName('abc-')).toEqual('Must end with a letter or number') - expect(validateName('abc---')).toEqual('Must end with a letter or number') + expect(validate('abc-')).toEqual('Must end with a letter or number') + expect(validate('abc---')).toEqual('Must end with a letter or number') }) it('rejects invalid characters', () => { - expect(validateName('aBc')).toEqual( + expect(validate('aBc')).toEqual( 'Can only contain lower-case letters, numbers, and dashes' ) - expect(validateName('asldk:c')).toEqual( + expect(validate('asldk:c')).toEqual( 'Can only contain lower-case letters, numbers, and dashes' ) }) diff --git a/libs/form/fields/NameField.tsx b/libs/form/fields/NameField.tsx index 62fecea4f3..75648c7e11 100644 --- a/libs/form/fields/NameField.tsx +++ b/libs/form/fields/NameField.tsx @@ -1,6 +1,7 @@ import type { TextFieldProps } from './TextField' import { TextField } from './TextField' import React from 'react' +import { capitalize } from '@oxide/util' export interface NameFieldProps extends Omit { @@ -10,12 +11,14 @@ export interface NameFieldProps export function NameField({ required = true, name = 'name', + label = capitalize(name), ...textFieldProps }: NameFieldProps) { return ( @@ -23,14 +26,17 @@ export function NameField({ } // TODO Update JSON schema to match this, add fuzz testing between this and name pattern -export function validateName(name: string) { - if (name.length === 0) { - return 'A name is required' - } else if (!/^[a-z]/.test(name)) { - return 'Must start with a lower-case letter' - } else if (!/[a-z0-9]$/.test(name)) { - return 'Must end with a letter or number' - } else if (!/^[a-z0-9-]+$/.test(name)) { - return 'Can only contain lower-case letters, numbers, and dashes' +export const validateName = + (label: string, required: boolean) => (name: string) => { + if (!required && name.length === 0) return + + if (name.length === 0) { + return `${label} is required` + } else if (!/^[a-z]/.test(name)) { + return 'Must start with a lower-case letter' + } else if (!/[a-z0-9]$/.test(name)) { + return 'Must end with a letter or number' + } else if (!/^[a-z0-9-]+$/.test(name)) { + return 'Can only contain lower-case letters, numbers, and dashes' + } } -} diff --git a/libs/util/str.spec.ts b/libs/util/str.spec.ts index 0aed11ad1c..ac3333f9e6 100644 --- a/libs/util/str.spec.ts +++ b/libs/util/str.spec.ts @@ -4,11 +4,6 @@ describe('capitalize', () => { it('capitalizes the first letter', () => { expect(capitalize('this is a sentence')).toEqual('This is a sentence') }) - - it('passes through falsy values', () => { - expect(capitalize('')).toEqual('') - expect(capitalize(undefined)).toEqual(undefined) - }) }) describe('camelCase', () => { diff --git a/libs/util/str.ts b/libs/util/str.ts index c2040ca059..5573cc5891 100644 --- a/libs/util/str.ts +++ b/libs/util/str.ts @@ -1,5 +1,4 @@ -// TODO: should this even accept undefined? kind of weird -export const capitalize = (s: string | undefined) => +export const capitalize = (s: string) => s && s.charAt(0).toUpperCase() + s.slice(1) export const pluralize = (s: string, n: number) => From 0c5c1d3f956e6a6298256214e880d12c1c601716 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Sun, 3 Apr 2022 21:26:56 -0400 Subject: [PATCH 06/32] Update extract types --- libs/form/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/form/types.ts b/libs/form/types.ts index 09e372d416..27af5b99ff 100644 --- a/libs/form/types.ts +++ b/libs/form/types.ts @@ -31,7 +31,7 @@ export type ExtendedPrebuiltFormProps = C extends ComponentType< : never export type ExtractFormValues = C extends ComponentType< - PrebuiltFormProps + PrebuiltFormProps > ? V : never From 65f0eaecaf10b1369251a46e42c01c226047e221 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Sun, 3 Apr 2022 22:49:13 -0400 Subject: [PATCH 07/32] Add disk attach --- app/forms/disk-attach.tsx | 55 +++++++++++++++++++++++++++++++++ app/forms/disk-create.tsx | 2 -- app/forms/index.ts | 4 +++ app/forms/instance-create.tsx | 52 +++++++++++++++++++------------ libs/form/fields/TableField.tsx | 7 ++++- 5 files changed, 97 insertions(+), 23 deletions(-) create mode 100644 app/forms/disk-attach.tsx diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx new file mode 100644 index 0000000000..c92533fee3 --- /dev/null +++ b/app/forms/disk-attach.tsx @@ -0,0 +1,55 @@ +import { Form, NameField } from '@oxide/form' +import React from 'react' +import type { PrebuiltFormProps } from '@oxide/form' +import { useParams } from 'app/hooks' +import type { Disk } from '@oxide/api' +import { useApiMutation, useApiQueryClient } from '@oxide/api' + +const values = { + name: '', +} + +export function AttachDiskForm({ + id = 'create-disk-form', + title = 'Create Disk', + initialValues = values, + onSubmit, + onSuccess, + onError, + ...props +}: PrebuiltFormProps) { + const parentNames = useParams('orgName', 'projectName', 'instanceName') + const queryClient = useApiQueryClient() + + const createDisk = useApiMutation('instanceDisksAttach', { + onSuccess(data) { + queryClient.invalidateQueries('instanceDisksGet', parentNames) + onSuccess?.(data) + }, + onError, + }) + + return ( + { + createDisk.mutate({ ...parentNames, body }) + }) + } + mutation={createDisk} + {...props} + > + + + {title} + + + + ) +} + +export default AttachDiskForm diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 6c7db1dd10..8d39315910 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -17,7 +17,6 @@ const values = { name: '', description: '', size: 0, - type: '', sourceType: '', deletionRule: '', } @@ -59,7 +58,6 @@ export function CreateDiskForm({ - Blank disk Image diff --git a/app/forms/index.ts b/app/forms/index.ts index 884d317342..1ce212c396 100644 --- a/app/forms/index.ts +++ b/app/forms/index.ts @@ -1,3 +1,5 @@ +// TODO: Make these just be default exports + import type { CreateSubnetForm } from './subnet-create' import type { EditSubnetForm } from './subnet-edit' import type { CreateOrgForm } from './org-create' @@ -5,6 +7,7 @@ import type { CreateDiskForm } from './disk-create' import type { CreateProjectForm } from './project-create' import type CreateInstanceForm from './instance-create' import type { ExtractFormValues } from '@oxide/form' +import type AttachDiskForm from './disk-attach' /** * A map of all existing forms. When a new form is created in the forms directory, a @@ -15,6 +18,7 @@ export interface FormTypes { 'instance-create': typeof CreateInstanceForm 'org-create': typeof CreateOrgForm 'project-create': typeof CreateProjectForm + 'disk-attach': typeof AttachDiskForm 'disk-create': typeof CreateDiskForm 'subnet-create': typeof CreateSubnetForm 'subnet-edit': typeof EditSubnetForm diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 5cf6e0afa8..e4107cc27c 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -47,7 +47,8 @@ export default function CreateInstanceForm({ }: PrebuiltFormProps) { const queryClient = useApiQueryClient() const addToast = useToast() - const [createDiskForm, showDiskForm] = useForm('disk-create') + const [createDiskForm, showCreateDiskForm] = useForm('disk-create') + const [attachDiskForm, showAttachDiskForm] = useForm('disk-attach') const { orgName, projectName } = useParams('orgName', 'projectName') @@ -208,35 +209,46 @@ export default function CreateInstanceForm({ Additional disks - > + & { type: 'create' }) + | (FormValues<'disk-attach'> & { type: 'attach'; size: number }) + > id="new-disks" name="New disks" actionText="Create new disk" - onAddItem={(addDisk) => { - const hideForm = showDiskForm({ - onSubmit: (values) => { - addDisk(values) - hideForm() + actions={[ + [ + 'Create new disk', + (addDisk) => { + const hideForm = showCreateDiskForm({ + onSubmit: (values) => { + addDisk({ type: 'create', ...values }) + hideForm() + }, + }) }, - }) - }} + ], + [ + 'Attach existing disk', + (addDisk) => { + const hideForm = showAttachDiskForm({ + onSubmit: (values) => { + // TODO fetch disk size + addDisk({ type: 'attach', ...values, size: 0 }) + hideForm() + }, + }) + }, + ], + ]} columns={[ ['name', 'Name'], + ['type', 'Type'], ['size', 'Size', filesize], ]} /> {createDiskForm} - - {/* */} + {attachDiskForm} Networking diff --git a/libs/form/fields/TableField.tsx b/libs/form/fields/TableField.tsx index 5eac2a0f59..e19e412a7d 100644 --- a/libs/form/fields/TableField.tsx +++ b/libs/form/fields/TableField.tsx @@ -11,14 +11,19 @@ type Column = K extends keyof (infer Item) ? [key: K, name: string, format?: (value: Item[K]) => string] : never +type Action = [ + name: string, + action: (addItem: (item: Item) => void) => void +] + export interface TableFieldProps { id: string name?: string label?: string actionText: string - onAddItem: (addItem: (item: Item) => void) => void onRemoveItem?: (item: Item) => void columns: Column[] + actions: Action[] } export function TableField({ From 5f20f328c0ae0585d03327c662617f4d28d491a1 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 4 Apr 2022 14:15:21 -0400 Subject: [PATCH 08/32] Fix instance-create page not rendering --- app/routes.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/routes.tsx b/app/routes.tsx index 56531947ff..89961fe240 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -110,7 +110,7 @@ export const routes = ( } /> } + element={} title="Create instance" icon={} /> From 879dffdbd7ed38a3e68f658a72c13a539305d423 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 5 Apr 2022 10:31:07 -0400 Subject: [PATCH 09/32] Add ability to get form context from user --- app/components/FormParamFields.tsx | 55 ++++++++++++++++++++++++++++++ app/forms/disk-create.tsx | 24 ++++++++----- app/forms/org-create.tsx | 4 +-- app/forms/project-create.tsx | 16 ++++----- app/forms/subnet-create.tsx | 23 ++++++++----- app/forms/subnet-edit.tsx | 4 +-- app/hooks/use-form.tsx | 4 +-- app/routes.spec.tsx | 39 ++++++++++++++++++++- app/routes.tsx | 21 ++++++++++++ libs/form/Form.tsx | 7 ++-- libs/form/types.ts | 16 +++++---- 11 files changed, 173 insertions(+), 40 deletions(-) create mode 100644 app/components/FormParamFields.tsx diff --git a/app/components/FormParamFields.tsx b/app/components/FormParamFields.tsx new file mode 100644 index 0000000000..b6235a2fd5 --- /dev/null +++ b/app/components/FormParamFields.tsx @@ -0,0 +1,55 @@ +import { TextField } from '@oxide/form' +import type { PathParam } from 'app/routes' +import { PARAM_DISPLAY, VALID_PARAMS } from 'app/routes' +import { useFormikContext } from 'formik' +import { useParams } from 'react-router-dom' +import React, { useEffect } from 'react' + +interface FormParamFieldsProps { + id: string + params: PathParam[] +} + +/** + * Renders a set of inputs to capture route params required by a form if + * said form isn't being rendered in the context where their available. + */ +export function FormParamFields({ id, params }: FormParamFieldsProps) { + return ( + <> + {VALID_PARAMS.filter((param) => param in params).map((param) => ( + + ))} + + ) +} + +interface FormParamProps { + id: string + param: PathParam +} +const FormParam = ({ id, param }: FormParamProps) => { + const { initialValues, setFieldValue } = + useFormikContext>() + const params = useParams() + + useEffect(() => { + if (!initialValues[param] && params[param]) { + setFieldValue(param, params[param]) + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [param, params]) + + /** + * If the param is in initialValues and non-empty that means it was + * explicitly handed down to the Form which implies it shouldn't be + * configurable to the user. If it's retrieved from the URL it also + * means it shouldn't be configurable to the user. This may change in + * the future. + */ + if (!initialValues[param] && params[param]) return null + + return ( + + ) +} diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 8d39315910..a6f4b519be 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -9,9 +9,10 @@ import { import { Divider } from '@oxide/ui' import React from 'react' import type { PrebuiltFormProps } from '@oxide/form' -import { useParams } from 'app/hooks' import type { Disk } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' +import { invariant } from '@oxide/util' +import { FormParamFields } from 'app/components/FormParamFields' const values = { name: '', @@ -29,14 +30,13 @@ export function CreateDiskForm({ onSuccess, onError, ...props -}: PrebuiltFormProps) { - const parentNames = useParams('orgName', 'projectName') +}: PrebuiltFormProps) { const queryClient = useApiQueryClient() const createDisk = useApiMutation('projectDisksPost', { - onSuccess(data) { - queryClient.invalidateQueries('projectDisksGet', parentNames) - onSuccess?.(data) + onSuccess(data, { body: _, ...pathParams }) { + queryClient.invalidateQueries('projectDisksGet', pathParams) + onSuccess?.(data, pathParams) }, onError, }) @@ -48,13 +48,21 @@ export function CreateDiskForm({ initialValues={initialValues} onSubmit={ onSubmit || - ((body) => { - createDisk.mutate({ ...parentNames, body }) + (({ orgName, projectName, ...body }) => { + invariant( + orgName && projectName, + `disk-create form is missing a path param` + ) + createDisk.mutate({ orgName, projectName, body }) }) } mutation={createDisk} {...props} > + diff --git a/app/forms/org-create.tsx b/app/forms/org-create.tsx index d442a721f9..b1bf2d63f6 100644 --- a/app/forms/org-create.tsx +++ b/app/forms/org-create.tsx @@ -24,7 +24,7 @@ export function CreateOrgForm({ const addToast = useToast() const createOrg = useApiMutation('organizationsPost', { - onSuccess(org) { + onSuccess(org, params) { queryClient.invalidateQueries('organizationsGet', {}) // avoid the org fetch when the org page loads since we have the data queryClient.setQueryData( @@ -38,7 +38,7 @@ export function CreateOrgForm({ content: 'Your organization has been created.', timeout: 5000, }) - onSuccess?.(org) + onSuccess?.(org, params) }, onError, }) diff --git a/app/forms/project-create.tsx b/app/forms/project-create.tsx index 2077a6a67f..d94b1480ce 100644 --- a/app/forms/project-create.tsx +++ b/app/forms/project-create.tsx @@ -3,9 +3,10 @@ import React from 'react' import { Success16Icon } from '@oxide/ui' import type { Project } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { useParams, useToast } from '../hooks' +import { useToast } from '../hooks' import { Form, NameField, DescriptionField } from '@oxide/form' import type { PrebuiltFormProps } from '@oxide/form' +import { invariant } from '@oxide/util' const values = { name: '', @@ -20,14 +21,12 @@ export function CreateProjectForm({ onSuccess, onError, ...props -}: PrebuiltFormProps) { +}: PrebuiltFormProps) { const queryClient = useApiQueryClient() const addToast = useToast() - const { orgName } = useParams('orgName') - const createProject = useApiMutation('organizationProjectsPost', { - onSuccess(project) { + onSuccess(project, { orgName }) { // refetch list of projects in sidebar queryClient.invalidateQueries('organizationProjectsGet', { orgName }) // avoid the project fetch when the project page loads since we have the data @@ -42,7 +41,7 @@ export function CreateProjectForm({ content: 'Your project has been created.', timeout: 5000, }) - onSuccess?.(project) + onSuccess?.(project, { orgName }) }, onError, }) @@ -54,10 +53,11 @@ export function CreateProjectForm({ title={title} onSubmit={ onSubmit || - (({ name, description }) => { + (({ orgName, ...body }) => { + invariant(orgName, `instance-create form is missing a path param`) createProject.mutate({ orgName, - body: { name, description }, + body, }) }) } diff --git a/app/forms/subnet-create.tsx b/app/forms/subnet-create.tsx index 5c29e890e8..fa20912fdf 100644 --- a/app/forms/subnet-create.tsx +++ b/app/forms/subnet-create.tsx @@ -4,7 +4,7 @@ import React from 'react' import type { PrebuiltFormProps } from '@oxide/form' import type { VpcSubnet } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { useParams } from 'app/hooks' +import { invariant } from '@oxide/util' const values = { name: '', @@ -21,14 +21,17 @@ export function CreateSubnetForm({ onSuccess, onError, ...props -}: PrebuiltFormProps) { - const parentNames = useParams('orgName', 'projectName', 'vpcName') +}: PrebuiltFormProps< + typeof values, + VpcSubnet, + 'orgName' | 'projectName' | 'vpcName' +>) { const queryClient = useApiQueryClient() const createSubnet = useApiMutation('vpcSubnetsPost', { - onSuccess(data) { - queryClient.invalidateQueries('vpcSubnetsGet', parentNames) - onSuccess?.(data) + onSuccess(data, { body: _, ...params }) { + queryClient.invalidateQueries('vpcSubnetsGet', params) + onSuccess?.(data, params) }, onError, }) @@ -39,8 +42,12 @@ export function CreateSubnetForm({ initialValues={initialValues} onSubmit={ onSubmit || - ((body) => { - createSubnet.mutate({ ...parentNames, body }) + (({ orgName, projectName, vpcName, ...body }) => { + invariant( + orgName && projectName && vpcName, + 'subnet-create form is missing a path param' + ) + createSubnet.mutate({ orgName, projectName, vpcName, body }) }) } mutation={createSubnet} diff --git a/app/forms/subnet-edit.tsx b/app/forms/subnet-edit.tsx index 96b2c8bf96..d46c30bea9 100644 --- a/app/forms/subnet-edit.tsx +++ b/app/forms/subnet-edit.tsx @@ -18,9 +18,9 @@ export function EditSubnetForm({ const queryClient = useApiQueryClient() const updateSubnet = useApiMutation('vpcSubnetsPutSubnet', { - onSuccess(data) { + onSuccess(data, params) { queryClient.invalidateQueries('vpcSubnetsGet', parentNames) - onSuccess?.(data) + onSuccess?.(data, params) }, onError, }) diff --git a/app/hooks/use-form.tsx b/app/hooks/use-form.tsx index 0897504577..f9a0d507cc 100644 --- a/app/hooks/use-form.tsx +++ b/app/hooks/use-form.tsx @@ -31,9 +31,9 @@ export const useForm = ( }, [formProps, setShowForm]) const onSuccess = useCallback( - (data) => { + (data, params) => { setShowForm(false) - formProps?.onSuccess?.(data) + formProps?.onSuccess?.(data, params) }, [formProps, setShowForm] ) diff --git a/app/routes.spec.tsx b/app/routes.spec.tsx index cd58866c6a..22f60237e1 100644 --- a/app/routes.spec.tsx +++ b/app/routes.spec.tsx @@ -1,9 +1,15 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ +import babel from '@babel/core' +import { traverse } from '@babel/core' +import type { JSXAttribute } from '@babel/types' +import fs from 'fs/promises' +import path from 'path' + import { matchRoutes } from 'react-router-dom' import { renderAppAt } from './test/utils' -import { getRouteConfig } from './routes' +import { getRouteConfig, VALID_PARAMS } from './routes' describe('routes', () => { it('should render successfully', async () => { @@ -35,3 +41,34 @@ describe('routeConfig', () => { `) }) }) + +test('VALID_PARAMS should contain all dynamic path params', async () => { + const params: Set = new Set() + const routesFile = await fs.readFile( + path.join(__dirname, './routes.tsx'), + 'utf-8' + ) + const ast = await parse(routesFile) + + traverse(ast, { + JSXOpeningElement(path) { + if ((path.node.name as babel.types.JSXIdentifier).name !== 'Route') return + + const routePath = path.node.attributes.find( + (attr) => (attr as JSXAttribute).name.name === 'path' + ) as JSXAttribute | undefined + if (!routePath) return + const { value } = routePath.value as babel.types.StringLiteral + if (!value.startsWith(':')) return + params.add(value.slice(1)) + }, + }) + + expect(Array.from(params)).toEqual(VALID_PARAMS) +}) + +const parse = (src: string) => + babel.parseAsync(src, { + plugins: ['@babel/plugin-syntax-typescript'], + filename: __filename, + }) diff --git a/app/routes.tsx b/app/routes.tsx index 89961fe240..dfb9a0f4ad 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -34,6 +34,27 @@ import { } from '@oxide/ui' import { FormPage } from './components/FormPage' +/** + * All valid dynamic route params. This contents of this + * array are validated to be in sync with the route config + * by a test in `routes.spec.tsx`. + */ +export const VALID_PARAMS = [ + 'orgName', + 'projectName', + 'instanceName', + 'vpcName', +] as const + +export type PathParam = typeof VALID_PARAMS[number] + +export const PARAM_DISPLAY: Record = { + orgName: 'Organization Name', + projectName: 'Project Name', + instanceName: 'Instance Name', + vpcName: 'Vpc Name', +} + /* * We are doing something a little unorthodox with the route config here. We * realized that tagging nodes in the route tree with arbitrary data is very diff --git a/libs/form/Form.tsx b/libs/form/Form.tsx index f9ec09527b..3e369f4333 100644 --- a/libs/form/Form.tsx +++ b/libs/form/Form.tsx @@ -44,7 +44,8 @@ type Mutation = error: ErrorResponse } -export interface FormProps extends FormikConfig { +export interface FormProps + extends FormikConfig> { id: string title?: ReactNode children: ReactNode @@ -52,14 +53,14 @@ export interface FormProps extends FormikConfig { mutation: Mutation } -export function Form({ +export function Form({ id, title, children, mutation, onDismiss, ...formikProps -}: FormProps) { +}: FormProps) { const isSideModal = useIsInSideModal() const childArray = flattenChildren(children) const actions = pluckFirstOfType(childArray, Form.Actions) diff --git a/libs/form/types.ts b/libs/form/types.ts index 27af5b99ff..e6f1072295 100644 --- a/libs/form/types.ts +++ b/libs/form/types.ts @@ -6,15 +6,19 @@ import type { FormProps } from './Form' * A form that's built out ahead of time and intended to be re-used dynamically. Fields * that are expected to be provided by default are set to optional. */ -export type PrebuiltFormProps = Omit< +export type PrebuiltFormProps< + Values, + Data, + RouteParams extends string = never +> = Omit< Optional< - FormProps, + FormProps>, 'id' | 'title' | 'initialValues' | 'onSubmit' | 'mutation' >, 'children' > & { children?: never - onSuccess?: (data: Data) => void + onSuccess?: (data: Data, params: Record) => void onError?: (err: ErrorResponse) => void } @@ -25,13 +29,13 @@ export type ExtendedPrebuiltFormProps = C extends ComponentType< infer B > ? // eslint-disable-next-line @typescript-eslint/no-explicit-any - B extends PrebuiltFormProps - ? PrebuiltFormProps + B extends PrebuiltFormProps + ? PrebuiltFormProps : never : never export type ExtractFormValues = C extends ComponentType< - PrebuiltFormProps + PrebuiltFormProps > ? V : never From 2cc26a6af8319a13bfaf46248388fb6915cac9db Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 5 Apr 2022 10:31:21 -0400 Subject: [PATCH 10/32] WIP instance create --- app/forms/disk-attach.tsx | 40 ++++++++++++++++++++++++--------- app/forms/instance-create.tsx | 33 ++++++++++++++++++++------- app/hooks/use-params.ts | 13 +++++++++-- libs/form/fields/TableField.tsx | 23 ++++++++++--------- 4 files changed, 78 insertions(+), 31 deletions(-) diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index c92533fee3..fd0d8ac864 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -1,30 +1,34 @@ -import { Form, NameField } from '@oxide/form' +import { Form, NameField, TextField } from '@oxide/form' import React from 'react' import type { PrebuiltFormProps } from '@oxide/form' -import { useParams } from 'app/hooks' import type { Disk } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' +import { invariant } from '@oxide/util' +import { FormParamFields } from 'app/components/FormParamFields' const values = { name: '', } export function AttachDiskForm({ - id = 'create-disk-form', + id = 'form-disk-attach', title = 'Create Disk', initialValues = values, onSubmit, onSuccess, onError, ...props -}: PrebuiltFormProps) { - const parentNames = useParams('orgName', 'projectName', 'instanceName') +}: PrebuiltFormProps< + typeof values, + Disk, + 'orgName' | 'projectName' | 'instanceName' +>) { const queryClient = useApiQueryClient() const createDisk = useApiMutation('instanceDisksAttach', { - onSuccess(data) { - queryClient.invalidateQueries('instanceDisksGet', parentNames) - onSuccess?.(data) + onSuccess(data, { body: _, ...pathParams }) { + queryClient.invalidateQueries('instanceDisksGet', pathParams) + onSuccess?.(data, pathParams) }, onError, }) @@ -36,14 +40,28 @@ export function AttachDiskForm({ initialValues={initialValues} onSubmit={ onSubmit || - ((body) => { - createDisk.mutate({ ...parentNames, body }) + (({ orgName, projectName, instanceName, name }) => { + invariant( + orgName && projectName && instanceName, + `disk-attach form is missing a path param` + ) + createDisk.mutate({ + orgName, + projectName, + instanceName, + body: { disk: name }, + }) }) } mutation={createDisk} {...props} > - + + + {title} diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index e4107cc27c..c0d1bc48d0 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -1,3 +1,4 @@ +import type { Instance } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import type { PrebuiltFormProps } from '@oxide/form' import { TableField } from '@oxide/form' @@ -20,7 +21,9 @@ import { UbuntuResponsiveIcon, WindowsResponsiveIcon, } from '@oxide/ui' -import { useParams, useToast } from 'app/hooks' +import { invariant } from '@oxide/util' +import { FormParamFields } from 'app/components/FormParamFields' +import { useToast } from 'app/hooks' import { useForm } from 'app/hooks/use-form' import filesize from 'filesize' import React from 'react' @@ -44,16 +47,14 @@ export default function CreateInstanceForm({ onSuccess, onError, ...props -}: PrebuiltFormProps) { +}: PrebuiltFormProps) { const queryClient = useApiQueryClient() const addToast = useToast() const [createDiskForm, showCreateDiskForm] = useForm('disk-create') const [attachDiskForm, showAttachDiskForm] = useForm('disk-attach') - const { orgName, projectName } = useParams('orgName', 'projectName') - const createInstance = useApiMutation('projectInstancesPost', { - onSuccess(instance) { + onSuccess(instance, { orgName, projectName }) { // refetch list of instances queryClient.invalidateQueries('projectInstancesGet', { orgName, @@ -71,7 +72,10 @@ export default function CreateInstanceForm({ content: 'Your instance has been created.', timeout: 5000, }) - onSuccess?.(instance) + onSuccess?.(instance, { + orgName, + projectName, + }) }, onError, }) @@ -83,7 +87,11 @@ export default function CreateInstanceForm({ title={title} onSubmit={ onSubmit || - ((values) => { + (({ orgName, projectName, ...values }) => { + invariant( + orgName && projectName, + `instance-create form is missing a path param` + ) const instance = INSTANCE_SIZES.find( (option) => option.id === values['type'] ) || { memory: 0, ncpus: 0 } @@ -104,6 +112,10 @@ export default function CreateInstanceForm({ mutation={createInstance} {...props} > + @@ -214,8 +226,8 @@ export default function CreateInstanceForm({ | (FormValues<'disk-attach'> & { type: 'attach'; size: number }) > id="new-disks" + label="" name="New disks" - actionText="Create new disk" actions={[ [ 'Create new disk', @@ -257,6 +269,11 @@ export default function CreateInstanceForm({ id="hostname" description="Will be generated if not provided" /> + + + {title} + + ) } diff --git a/app/hooks/use-params.ts b/app/hooks/use-params.ts index 4e17ceeeef..7de336b30f 100644 --- a/app/hooks/use-params.ts +++ b/app/hooks/use-params.ts @@ -1,6 +1,15 @@ import { useParams as _useParams } from 'react-router-dom' import { invariant } from '@oxide/util' +type OptionalKey = K extends `${infer U}?` ? U : never +type RequiredKey = K extends OptionalKey ? never : K + +type ParamsWithOptionalKeys = { + [key in RequiredKey]: string +} & { + [key in OptionalKey]?: string +} + /** * Wrapper for React Router's `useParams` that throws (in dev) if any of the * specified params is missing. @@ -9,7 +18,7 @@ import { invariant } from '@oxide/util' */ export function useParams( ...paramNames: K[] -): Record { +): ParamsWithOptionalKeys { const params = _useParams() if (process.env.NODE_ENV !== 'production') { for (const k of paramNames) { @@ -19,5 +28,5 @@ export function useParams( ) } } - return params as Record + return params as ParamsWithOptionalKeys } diff --git a/libs/form/fields/TableField.tsx b/libs/form/fields/TableField.tsx index e19e412a7d..f52ef9d7a9 100644 --- a/libs/form/fields/TableField.tsx +++ b/libs/form/fields/TableField.tsx @@ -20,7 +20,6 @@ export interface TableFieldProps { id: string name?: string label?: string - actionText: string onRemoveItem?: (item: Item) => void columns: Column[] actions: Action[] @@ -30,10 +29,9 @@ export function TableField({ id, name = id, label = capitalize(name), - actionText, - onAddItem, onRemoveItem, columns, + actions, }: TableFieldProps) { const [, { value = [] }, { setValue }] = useField({ name }) @@ -80,13 +78,18 @@ export function TableField({ )} - +
+ {actions.map(([name, action]) => ( + + ))} +
) } From 9a51f08051774d9f12e756733ca8c75218514339 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 5 Apr 2022 14:42:10 -0400 Subject: [PATCH 11/32] Fix issue where form params weren't being set --- app/components/FormParamFields.tsx | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/app/components/FormParamFields.tsx b/app/components/FormParamFields.tsx index b6235a2fd5..eb0ced6fd7 100644 --- a/app/components/FormParamFields.tsx +++ b/app/components/FormParamFields.tsx @@ -14,7 +14,22 @@ interface FormParamFieldsProps { * Renders a set of inputs to capture route params required by a form if * said form isn't being rendered in the context where their available. */ -export function FormParamFields({ id, params }: FormParamFieldsProps) { +export function FormParamFields({ + id, + params: paramKeys, +}: FormParamFieldsProps) { + const { initialValues, setFieldValue } = + useFormikContext>() + const params = useParams() + + useEffect(() => { + for (const param of paramKeys) { + if (!initialValues[param] && params[param]) { + setFieldValue(param, params[param]) + } + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [params]) return ( <> {VALID_PARAMS.filter((param) => param in params).map((param) => ( @@ -29,17 +44,10 @@ interface FormParamProps { param: PathParam } const FormParam = ({ id, param }: FormParamProps) => { - const { initialValues, setFieldValue } = + const { initialValues } = useFormikContext>() const params = useParams() - useEffect(() => { - if (!initialValues[param] && params[param]) { - setFieldValue(param, params[param]) - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [param, params]) - /** * If the param is in initialValues and non-empty that means it was * explicitly handed down to the Form which implies it shouldn't be From 914bb3d404207d35956221cb3647a1fe98fbe5e4 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 5 Apr 2022 14:42:42 -0400 Subject: [PATCH 12/32] fixup disk attach form --- app/forms/disk-attach.tsx | 5 ++--- libs/form/fields/NameField.tsx | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index fd0d8ac864..a23b66a6a0 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -12,7 +12,7 @@ const values = { export function AttachDiskForm({ id = 'form-disk-attach', - title = 'Create Disk', + title = 'Attach Disk', initialValues = values, onSubmit, onSuccess, @@ -60,8 +60,7 @@ export function AttachDiskForm({ id="form-disk-attach-params" params={['orgName', 'projectName', 'instanceName']} /> - - + {title} diff --git a/libs/form/fields/NameField.tsx b/libs/form/fields/NameField.tsx index 75648c7e11..f01da77576 100644 --- a/libs/form/fields/NameField.tsx +++ b/libs/form/fields/NameField.tsx @@ -28,7 +28,7 @@ export function NameField({ // TODO Update JSON schema to match this, add fuzz testing between this and name pattern export const validateName = (label: string, required: boolean) => (name: string) => { - if (!required && name.length === 0) return + if (!required && !name) return if (name.length === 0) { return `${label} is required` From a68dfb0a60f0f1e7a6bf61da1a56e6e0291e6d27 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 5 Apr 2022 20:08:48 +0000 Subject: [PATCH 13/32] automatically update packer-id --- tools/create_gcp_instance.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/create_gcp_instance.sh b/tools/create_gcp_instance.sh index 2f0f82c20e..a77d463d69 100755 --- a/tools/create_gcp_instance.sh +++ b/tools/create_gcp_instance.sh @@ -41,7 +41,7 @@ retry 2 gcloud compute instances create "$INSTANCE_NAME" \ --description="Machine automatically generated from branch ${BRANCH_NAME} of the oxidecomputer/console git repo." \ --hostname="${INSTANCE_NAME}.internal.oxide.computer" \ --zone=$ZONE \ - --image=packer-1649181062 \ + --image=packer-1649182740 \ --maintenance-policy=TERMINATE \ --restart-on-failure \ --machine-type=$INSTANCE_TYPE \ From 24b0daa93a07208f16f9fae2fdde243888237de5 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 6 Apr 2022 17:25:51 -0400 Subject: [PATCH 14/32] Move pre-built form types out of form lib --- app/forms/disk-create.tsx | 5 +++-- app/forms/index.ts | 43 +++++++++++++++++++++++++++++++++++- app/forms/org-create.tsx | 5 +++-- app/forms/project-create.tsx | 5 +++-- app/forms/subnet-create.tsx | 3 ++- app/forms/subnet-edit.tsx | 7 +++--- libs/form/index.ts | 1 - libs/form/types.ts | 41 ---------------------------------- 8 files changed, 57 insertions(+), 53 deletions(-) delete mode 100644 libs/form/types.ts diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index a6f4b519be..71e30eca44 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -1,3 +1,4 @@ +import React from 'react' import { DescriptionField, Form, @@ -7,11 +8,11 @@ import { Radio, } from '@oxide/form' import { Divider } from '@oxide/ui' -import React from 'react' -import type { PrebuiltFormProps } from '@oxide/form' import type { Disk } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import { invariant } from '@oxide/util' + +import type { PrebuiltFormProps } from 'app/forms' import { FormParamFields } from 'app/components/FormParamFields' const values = { diff --git a/app/forms/index.ts b/app/forms/index.ts index 1ce212c396..f3782076b5 100644 --- a/app/forms/index.ts +++ b/app/forms/index.ts @@ -6,9 +6,12 @@ import type { CreateOrgForm } from './org-create' import type { CreateDiskForm } from './disk-create' import type { CreateProjectForm } from './project-create' import type CreateInstanceForm from './instance-create' -import type { ExtractFormValues } from '@oxide/form' import type AttachDiskForm from './disk-attach' +import type { FormProps } from '@oxide/form' +import type { ErrorResponse } from '@oxide/api' +import type { ComponentType } from 'react' + /** * A map of all existing forms. When a new form is created in the forms directory, a * new entry should be added here with the key of the string name of the form's filename @@ -27,3 +30,41 @@ export interface FormTypes { export type FormValues = ExtractFormValues< FormTypes[K] > + +/** + * A form that's built out ahead of time and intended to be re-used dynamically. Fields + * that are expected to be provided by default are set to optional. + */ +export type PrebuiltFormProps< + Values, + Data, + RouteParams extends string = never +> = Omit< + Optional< + FormProps>, + 'id' | 'title' | 'initialValues' | 'onSubmit' | 'mutation' + >, + 'children' +> & { + children?: never + onSuccess?: (data: Data, params: Record) => void + onError?: (err: ErrorResponse) => void +} + +/** + * A utility type for a prebuilt form that extends another form + */ +export type ExtendedPrebuiltFormProps = C extends ComponentType< + infer B +> + ? // eslint-disable-next-line @typescript-eslint/no-explicit-any + B extends PrebuiltFormProps + ? PrebuiltFormProps + : never + : never + +export type ExtractFormValues = C extends ComponentType< + PrebuiltFormProps +> + ? V + : never diff --git a/app/forms/org-create.tsx b/app/forms/org-create.tsx index b1bf2d63f6..8a2cbc7c34 100644 --- a/app/forms/org-create.tsx +++ b/app/forms/org-create.tsx @@ -2,9 +2,10 @@ import React from 'react' import { Form, NameField, DescriptionField } from '@oxide/form' import type { Organization } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { useToast } from 'app/hooks' import { Success16Icon } from '@oxide/ui' -import type { PrebuiltFormProps } from '@oxide/form' + +import { useToast } from 'app/hooks' +import type { PrebuiltFormProps } from 'app/forms' const values = { name: '', diff --git a/app/forms/project-create.tsx b/app/forms/project-create.tsx index d94b1480ce..01baf7b7b1 100644 --- a/app/forms/project-create.tsx +++ b/app/forms/project-create.tsx @@ -3,11 +3,12 @@ import React from 'react' import { Success16Icon } from '@oxide/ui' import type { Project } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { useToast } from '../hooks' import { Form, NameField, DescriptionField } from '@oxide/form' -import type { PrebuiltFormProps } from '@oxide/form' import { invariant } from '@oxide/util' +import { useToast } from 'app/hooks' +import type { PrebuiltFormProps } from 'app/forms' + const values = { name: '', description: '', diff --git a/app/forms/subnet-create.tsx b/app/forms/subnet-create.tsx index fa20912fdf..8c6e7023c5 100644 --- a/app/forms/subnet-create.tsx +++ b/app/forms/subnet-create.tsx @@ -1,11 +1,12 @@ import { DescriptionField, Form, NameField, TextField } from '@oxide/form' import { Divider } from '@oxide/ui' import React from 'react' -import type { PrebuiltFormProps } from '@oxide/form' import type { VpcSubnet } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import { invariant } from '@oxide/util' +import type { PrebuiltFormProps } from 'app/forms' + const values = { name: '', description: '', diff --git a/app/forms/subnet-edit.tsx b/app/forms/subnet-edit.tsx index d46c30bea9..a8e80d160b 100644 --- a/app/forms/subnet-edit.tsx +++ b/app/forms/subnet-edit.tsx @@ -1,11 +1,12 @@ import React from 'react' -import { CreateSubnetForm } from './subnet-create' -import type { ExtendedPrebuiltFormProps } from '@oxide/form' -import { useParams } from 'app/hooks' import type { VpcSubnet } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import { invariant } from '@oxide/util' +import { CreateSubnetForm } from './subnet-create' +import { useParams } from 'app/hooks' +import type { ExtendedPrebuiltFormProps } from 'app/forms' + export function EditSubnetForm({ id = 'edit-subnet-form', title = 'Edit subnet', diff --git a/libs/form/index.ts b/libs/form/index.ts index cc9c74b943..dcf2030d15 100644 --- a/libs/form/index.ts +++ b/libs/form/index.ts @@ -1,3 +1,2 @@ export * from './Form' export * from './fields' -export * from './types' diff --git a/libs/form/types.ts b/libs/form/types.ts deleted file mode 100644 index e6f1072295..0000000000 --- a/libs/form/types.ts +++ /dev/null @@ -1,41 +0,0 @@ -import type { ErrorResponse } from '@oxide/api' -import type { ComponentType } from 'react' -import type { FormProps } from './Form' - -/** - * A form that's built out ahead of time and intended to be re-used dynamically. Fields - * that are expected to be provided by default are set to optional. - */ -export type PrebuiltFormProps< - Values, - Data, - RouteParams extends string = never -> = Omit< - Optional< - FormProps>, - 'id' | 'title' | 'initialValues' | 'onSubmit' | 'mutation' - >, - 'children' -> & { - children?: never - onSuccess?: (data: Data, params: Record) => void - onError?: (err: ErrorResponse) => void -} - -/** - * A utility type for a prebuilt form that extends another form - */ -export type ExtendedPrebuiltFormProps = C extends ComponentType< - infer B -> - ? // eslint-disable-next-line @typescript-eslint/no-explicit-any - B extends PrebuiltFormProps - ? PrebuiltFormProps - : never - : never - -export type ExtractFormValues = C extends ComponentType< - PrebuiltFormProps -> - ? V - : never From e91aa2d114f412335603e7624b29ce516cecc18d Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 16:41:36 -0400 Subject: [PATCH 15/32] Delete old instance create page --- .../instances/create/InstancesCreatePage.tsx | 327 ------------------ app/pages/project/instances/create/index.ts | 0 .../create/modals/existing-disk-modal.tsx | 101 ------ .../instances/create/modals/network-modal.tsx | 59 ---- .../create/modals/new-disk-modal.tsx | 99 ------ app/pages/project/instances/index.tsx | 1 - 6 files changed, 587 deletions(-) delete mode 100644 app/pages/project/instances/create/InstancesCreatePage.tsx delete mode 100644 app/pages/project/instances/create/index.ts delete mode 100644 app/pages/project/instances/create/modals/existing-disk-modal.tsx delete mode 100644 app/pages/project/instances/create/modals/network-modal.tsx delete mode 100644 app/pages/project/instances/create/modals/new-disk-modal.tsx diff --git a/app/pages/project/instances/create/InstancesCreatePage.tsx b/app/pages/project/instances/create/InstancesCreatePage.tsx deleted file mode 100644 index bcd58f01e6..0000000000 --- a/app/pages/project/instances/create/InstancesCreatePage.tsx +++ /dev/null @@ -1,327 +0,0 @@ -import React, { useState } from 'react' -import { useNavigate } from 'react-router-dom' -import cn from 'classnames' -import { Formik, Form } from 'formik' - -import { - Button, - RadioGroupHint, - RadioGroup, - RadioCard, - Tabs, - Tab, - TextField, - TextFieldHint, - FieldLabel, - Badge, -} from '@oxide/ui' -import { classed } from '@oxide/util' -import { useApiMutation } from '@oxide/api' -import { getServerError } from 'app/util/errors' -import { INSTANCE_SIZES } from './instance-types' -import { NewDiskModal } from './modals/new-disk-modal' -import { ExistingDiskModal } from './modals/existing-disk-modal' -import { NetworkModal } from './modals/network-modal' -import { useParams } from 'app/hooks' - -// TODO: these probably should not both exist -const headingStyle = 'text-white text-sans-xl' -const Heading = classed.h2`text-white text-sans-xl mt-16 mb-8` - -// TODO: need to fix page container if we want these to go all the way across -const Divider = () =>
- -const GB = 1024 * 1024 * 1024 - -const ERROR_CODES = { - ObjectAlreadyExists: - 'An instance with that name already exists in this project', -} - -export default function InstanceCreatePage() { - const navigate = useNavigate() - const { orgName, projectName } = useParams('orgName', 'projectName') - - const [showNewDiskModal, setShowNewDiskModal] = useState(false) - const [showExistingDiskModal, setShowExistingDiskModal] = useState(false) - const [showNetworkModal, setShowNetworkModal] = useState(false) - - const createInstance = useApiMutation('projectInstancesPost', { - onSuccess() { - navigate('..') // project page - }, - }) - - const renderLargeRadioCards = (category: string) => { - return INSTANCE_SIZES.filter((option) => option.category === category).map( - (option) => ( - -
- {option.ncpus} CPUs -
-
- {option.memory} GB RAM -
-
- ) - ) - } - - return ( - <> - { - if (!createInstance.isLoading) { - const instance = INSTANCE_SIZES.find( - (option) => option.id === values['instance-type'] - ) || { memory: 0, ncpus: 0 } - - createInstance.mutate({ - orgName, - projectName, - body: { - name: values['instance-name'], - hostname: values.hostname, - description: `An instance in project: ${projectName}`, - memory: instance.memory * GB, - ncpus: instance.ncpus, - }, - }) - } - }} - > -
- Choose an image - - Distributions - -
- Choose a pre-built image - - CentOS - Debian - Fedora - FreeBSD - Ubuntu - Windows - -
-
- - Custom Images - -
- Choose a custom image - - Custom CentOS - Custom Debian - Custom Fedora - -
-
-
- - Choose CPUs and RAM - - General purpose - -
- - Choose a general purpose instance - - - General purpose instances provide a good balance of CPU, - memory, and high performance storage; well suited for a wide - range of use cases. - - {/* TODO: find the logic behind this ad hoc spacing */} - - {renderLargeRadioCards('general')} - -
-
- - CPU-optimized - -
- - Choose a CPU-optimized instance - - - CPU optimized instances provide a good balance of... - - - {renderLargeRadioCards('cpuOptimized')} - -
-
- - Memory-optimized - -
- - Choose a memory-optimized instance - - - Memory optimized instances provide a good balance of... - - - {renderLargeRadioCards('memoryOptimized')} - -
-
- - - Custom New - - -
- Choose a custom instance - - Custom instances... - - - {renderLargeRadioCards('custom')} - -
-
-
- -
-
- - Boot disk storage - - - - 100 GB - - - 200 GB - - - 500 GB - - - 1,000 GB - - - 2,000 GB - - Custom - -
- -
-

Additional volumes

- - setShowNewDiskModal(false)} - /> - - setShowExistingDiskModal(false)} - orgName={orgName} - projectName={projectName} - /> -
-
- - Networking - - setShowNetworkModal(false)} - orgName={orgName} - projectName={projectName} - /> - - - - Finalize and create -
- Choose a name - - Choose an identifying name you will remember. Names may contain - alphanumeric characters, dashes, and periods. - - -
-
- Choose a hostname - - Optional. If left blank, we will use the instance name. - - -
- - {/* this is going to be a tag multiselect, not a text input */} -
- Add tags - - Use tags to organize and relate resources. Tags may contain - letters, numbers, colons, dashes, and underscores. - - -
- - -
- {getServerError(createInstance.error, ERROR_CODES)} -
- -
- - ) -} diff --git a/app/pages/project/instances/create/index.ts b/app/pages/project/instances/create/index.ts deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/app/pages/project/instances/create/modals/existing-disk-modal.tsx b/app/pages/project/instances/create/modals/existing-disk-modal.tsx deleted file mode 100644 index 99fdf7fa16..0000000000 --- a/app/pages/project/instances/create/modals/existing-disk-modal.tsx +++ /dev/null @@ -1,101 +0,0 @@ -import React from 'react' -import { Formik, Form } from 'formik' - -import type { Disk } from '@oxide/api' -import { useApiQuery } from '@oxide/api' -import { - Button, - Dropdown, - FieldLabel, - Radio, - RadioGroup, - SideModal_old as SideModal, -} from '@oxide/ui' - -type Props = { - isOpen: boolean - onDismiss: () => void - orgName: string - projectName: string -} - -const isUnattached = ({ state }: Disk) => { - const stateStr = state.state - return ( - stateStr !== 'attached' && - stateStr !== 'attaching' && - stateStr !== 'detaching' - ) -} - -export function ExistingDiskModal({ - isOpen, - onDismiss, - orgName, - projectName, -}: Props) { - // TODO: maybe wait to fetch until you open the modal - const { data } = useApiQuery('projectDisksGet', { orgName, projectName }) - - const disks = data?.items - .filter(isUnattached) - .map((d) => ({ value: d.id, label: d.name })) - - return ( - - - {}} - > -
- {/*

Disk

*/} - -
- Mode - - Read/Write - Read only - -
-
- Deletion Rule - - Keep disk - Delete disk - -
- -
-
- - Deletion rules - Disk naming - - - {/* TODO: not supposed to be a ghost button */} - - - -
- ) -} diff --git a/app/pages/project/instances/create/modals/network-modal.tsx b/app/pages/project/instances/create/modals/network-modal.tsx deleted file mode 100644 index 0160fee191..0000000000 --- a/app/pages/project/instances/create/modals/network-modal.tsx +++ /dev/null @@ -1,59 +0,0 @@ -import React from 'react' - -import { Button, Dropdown, SideModal_old as SideModal } from '@oxide/ui' -import { useApiQuery } from '@oxide/api' - -type Props = { - isOpen: boolean - onDismiss: () => void - orgName: string - projectName: string -} - -export function NetworkModal({ - isOpen, - onDismiss, - orgName, - projectName, -}: Props) { - const { data: vpcs } = useApiQuery('projectVpcsGet', { orgName, projectName }) - const vpcItems = vpcs?.items.map((v) => ({ value: v.id, label: v.name })) - return ( - - - {/* TODO: tearing up Dropdown into bits will let us fix button alignment */} -
- - -
-
- - -
- - - -
- - Subnetworks - External IPs - - - {/* TODO: not supposed to be a ghost button */} - - - -
- ) -} diff --git a/app/pages/project/instances/create/modals/new-disk-modal.tsx b/app/pages/project/instances/create/modals/new-disk-modal.tsx deleted file mode 100644 index 72b3d8bc95..0000000000 --- a/app/pages/project/instances/create/modals/new-disk-modal.tsx +++ /dev/null @@ -1,99 +0,0 @@ -import React from 'react' -import { Formik, Form } from 'formik' - -import { - Button, - FieldLabel, - Radio, - RadioGroup, - SideModal_old as SideModal, - TextField, -} from '@oxide/ui' - -type Props = { - isOpen: boolean - onDismiss: () => void -} - -export function NewDiskModal({ isOpen, onDismiss }: Props) { - return ( - - - {}} - > -
-
- - Name - - -
-
- - Description - - -
-
- Type - -
-
- - Source type - - -
-
- Deletion rule - - Keep disk - Delete disk - -
-
- Size (GiB) - -
-
- Configuration options - - Manually format and mount - Automatically format and mount - -
-
-
-
- - Formatting a persistent disk - Deletion Rules - Disk naming - - - {/* TODO: not supposed to be a ghost button */} - - - -
- ) -} diff --git a/app/pages/project/instances/index.tsx b/app/pages/project/instances/index.tsx index 2b56c14ad1..96dc1aabc7 100644 --- a/app/pages/project/instances/index.tsx +++ b/app/pages/project/instances/index.tsx @@ -1,3 +1,2 @@ -export * from './create' export * from './instance/InstancePage' export * from './InstancesPage' From 1c7660c6957cdc8be2e1b6cd789cf183c9cd7b78 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 17:59:31 -0400 Subject: [PATCH 16/32] Revert page param changes --- app/components/FormParamFields.tsx | 63 ------------------------------ app/forms/disk-attach.tsx | 23 ++++++----- app/forms/disk-create.tsx | 17 ++++---- app/forms/index.ts | 16 +++----- app/forms/instance-create.tsx | 32 ++++++--------- app/forms/org-create.tsx | 7 ++-- app/forms/project-create.tsx | 17 ++++---- app/forms/subnet-create.tsx | 24 ++++-------- app/forms/subnet-edit.tsx | 4 +- app/routes.spec.tsx | 39 +----------------- app/routes.tsx | 21 ---------- libs/form/Form.tsx | 7 ++-- 12 files changed, 62 insertions(+), 208 deletions(-) delete mode 100644 app/components/FormParamFields.tsx diff --git a/app/components/FormParamFields.tsx b/app/components/FormParamFields.tsx deleted file mode 100644 index eb0ced6fd7..0000000000 --- a/app/components/FormParamFields.tsx +++ /dev/null @@ -1,63 +0,0 @@ -import { TextField } from '@oxide/form' -import type { PathParam } from 'app/routes' -import { PARAM_DISPLAY, VALID_PARAMS } from 'app/routes' -import { useFormikContext } from 'formik' -import { useParams } from 'react-router-dom' -import React, { useEffect } from 'react' - -interface FormParamFieldsProps { - id: string - params: PathParam[] -} - -/** - * Renders a set of inputs to capture route params required by a form if - * said form isn't being rendered in the context where their available. - */ -export function FormParamFields({ - id, - params: paramKeys, -}: FormParamFieldsProps) { - const { initialValues, setFieldValue } = - useFormikContext>() - const params = useParams() - - useEffect(() => { - for (const param of paramKeys) { - if (!initialValues[param] && params[param]) { - setFieldValue(param, params[param]) - } - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [params]) - return ( - <> - {VALID_PARAMS.filter((param) => param in params).map((param) => ( - - ))} - - ) -} - -interface FormParamProps { - id: string - param: PathParam -} -const FormParam = ({ id, param }: FormParamProps) => { - const { initialValues } = - useFormikContext>() - const params = useParams() - - /** - * If the param is in initialValues and non-empty that means it was - * explicitly handed down to the Form which implies it shouldn't be - * configurable to the user. If it's retrieved from the URL it also - * means it shouldn't be configurable to the user. This may change in - * the future. - */ - if (!initialValues[param] && params[param]) return null - - return ( - - ) -} diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index a23b66a6a0..64aaa6d965 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -1,10 +1,11 @@ -import { Form, NameField, TextField } from '@oxide/form' +import { Form, NameField } from '@oxide/form' import React from 'react' -import type { PrebuiltFormProps } from '@oxide/form' import type { Disk } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import { invariant } from '@oxide/util' import { FormParamFields } from 'app/components/FormParamFields' +import { useParams } from 'app/hooks' +import type { PrebuiltFormProps } from 'app/forms' const values = { name: '', @@ -18,17 +19,19 @@ export function AttachDiskForm({ onSuccess, onError, ...props -}: PrebuiltFormProps< - typeof values, - Disk, - 'orgName' | 'projectName' | 'instanceName' ->) { +}: PrebuiltFormProps) { const queryClient = useApiQueryClient() + const pathParams = useParams('orgName', 'projectName', 'instanceName?') const createDisk = useApiMutation('instanceDisksAttach', { - onSuccess(data, { body: _, ...pathParams }) { - queryClient.invalidateQueries('instanceDisksGet', pathParams) - onSuccess?.(data, pathParams) + onSuccess(data) { + const { instanceName, ...others } = pathParams + invariant(instanceName, 'instanceName is required') + queryClient.invalidateQueries('instanceDisksGet', { + instanceName, + ...others, + }) + onSuccess?.(data) }, onError, }) diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 71e30eca44..996bcf4651 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -10,10 +10,10 @@ import { import { Divider } from '@oxide/ui' import type { Disk } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { invariant } from '@oxide/util' import type { PrebuiltFormProps } from 'app/forms' import { FormParamFields } from 'app/components/FormParamFields' +import { useParams } from 'app/hooks' const values = { name: '', @@ -31,13 +31,14 @@ export function CreateDiskForm({ onSuccess, onError, ...props -}: PrebuiltFormProps) { +}: PrebuiltFormProps) { const queryClient = useApiQueryClient() + const pathParams = useParams('orgName', 'projectName') const createDisk = useApiMutation('projectDisksPost', { - onSuccess(data, { body: _, ...pathParams }) { + onSuccess(data) { queryClient.invalidateQueries('projectDisksGet', pathParams) - onSuccess?.(data, pathParams) + onSuccess?.(data) }, onError, }) @@ -49,12 +50,8 @@ export function CreateDiskForm({ initialValues={initialValues} onSubmit={ onSubmit || - (({ orgName, projectName, ...body }) => { - invariant( - orgName && projectName, - `disk-create form is missing a path param` - ) - createDisk.mutate({ orgName, projectName, body }) + ((body) => { + createDisk.mutate({ ...pathParams, body }) }) } mutation={createDisk} diff --git a/app/forms/index.ts b/app/forms/index.ts index f3782076b5..a0851d80af 100644 --- a/app/forms/index.ts +++ b/app/forms/index.ts @@ -35,19 +35,15 @@ export type FormValues = ExtractFormValues< * A form that's built out ahead of time and intended to be re-used dynamically. Fields * that are expected to be provided by default are set to optional. */ -export type PrebuiltFormProps< - Values, - Data, - RouteParams extends string = never -> = Omit< +export type PrebuiltFormProps = Omit< Optional< - FormProps>, + FormProps, 'id' | 'title' | 'initialValues' | 'onSubmit' | 'mutation' >, 'children' > & { children?: never - onSuccess?: (data: Data, params: Record) => void + onSuccess?: (data: Data) => void onError?: (err: ErrorResponse) => void } @@ -58,13 +54,13 @@ export type ExtendedPrebuiltFormProps = C extends ComponentType< infer B > ? // eslint-disable-next-line @typescript-eslint/no-explicit-any - B extends PrebuiltFormProps - ? PrebuiltFormProps + B extends PrebuiltFormProps + ? PrebuiltFormProps : never : never export type ExtractFormValues = C extends ComponentType< - PrebuiltFormProps + PrebuiltFormProps > ? V : never diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index c0d1bc48d0..de12cab116 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -1,6 +1,6 @@ import type { Instance } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import type { PrebuiltFormProps } from '@oxide/form' +import type { PrebuiltFormProps } from 'app/forms' import { TableField } from '@oxide/form' import { TextField } from '@oxide/form' import { RadioField } from '@oxide/form' @@ -23,7 +23,7 @@ import { } from '@oxide/ui' import { invariant } from '@oxide/util' import { FormParamFields } from 'app/components/FormParamFields' -import { useToast } from 'app/hooks' +import { useParams, useToast } from 'app/hooks' import { useForm } from 'app/hooks/use-form' import filesize from 'filesize' import React from 'react' @@ -47,23 +47,21 @@ export default function CreateInstanceForm({ onSuccess, onError, ...props -}: PrebuiltFormProps) { +}: PrebuiltFormProps) { const queryClient = useApiQueryClient() const addToast = useToast() const [createDiskForm, showCreateDiskForm] = useForm('disk-create') const [attachDiskForm, showAttachDiskForm] = useForm('disk-attach') + const pageParams = useParams('orgName', 'projectName') const createInstance = useApiMutation('projectInstancesPost', { - onSuccess(instance, { orgName, projectName }) { + onSuccess(instance) { // refetch list of instances - queryClient.invalidateQueries('projectInstancesGet', { - orgName, - projectName, - }) + queryClient.invalidateQueries('projectInstancesGet', pageParams) // avoid the instance fetch when the instance page loads since we have the data queryClient.setQueryData( 'projectInstancesGetInstance', - { orgName, projectName, instanceName: instance.name }, + { ...pageParams, instanceName: instance.name }, instance ) addToast({ @@ -72,10 +70,7 @@ export default function CreateInstanceForm({ content: 'Your instance has been created.', timeout: 5000, }) - onSuccess?.(instance, { - orgName, - projectName, - }) + onSuccess?.(instance) }, onError, }) @@ -87,21 +82,16 @@ export default function CreateInstanceForm({ title={title} onSubmit={ onSubmit || - (({ orgName, projectName, ...values }) => { - invariant( - orgName && projectName, - `instance-create form is missing a path param` - ) + ((values) => { const instance = INSTANCE_SIZES.find( (option) => option.id === values['type'] ) || { memory: 0, ncpus: 0 } createInstance.mutate({ - orgName, - projectName, + ...pageParams, body: { name: values['name'], hostname: values.hostname, - description: `An instance in project: ${projectName}`, + description: `An instance in project: ${pageParams.projectName}`, memory: filesize(instance.memory, { output: 'object', base: 2 }) .value, ncpus: instance.ncpus, diff --git a/app/forms/org-create.tsx b/app/forms/org-create.tsx index 8a2cbc7c34..705b68855d 100644 --- a/app/forms/org-create.tsx +++ b/app/forms/org-create.tsx @@ -2,9 +2,8 @@ import React from 'react' import { Form, NameField, DescriptionField } from '@oxide/form' import type { Organization } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { Success16Icon } from '@oxide/ui' - import { useToast } from 'app/hooks' +import { Success16Icon } from '@oxide/ui' import type { PrebuiltFormProps } from 'app/forms' const values = { @@ -25,7 +24,7 @@ export function CreateOrgForm({ const addToast = useToast() const createOrg = useApiMutation('organizationsPost', { - onSuccess(org, params) { + onSuccess(org) { queryClient.invalidateQueries('organizationsGet', {}) // avoid the org fetch when the org page loads since we have the data queryClient.setQueryData( @@ -39,7 +38,7 @@ export function CreateOrgForm({ content: 'Your organization has been created.', timeout: 5000, }) - onSuccess?.(org, params) + onSuccess?.(org) }, onError, }) diff --git a/app/forms/project-create.tsx b/app/forms/project-create.tsx index 01baf7b7b1..a4f6cc77a2 100644 --- a/app/forms/project-create.tsx +++ b/app/forms/project-create.tsx @@ -3,10 +3,8 @@ import React from 'react' import { Success16Icon } from '@oxide/ui' import type { Project } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' +import { useParams, useToast } from '../hooks' import { Form, NameField, DescriptionField } from '@oxide/form' -import { invariant } from '@oxide/util' - -import { useToast } from 'app/hooks' import type { PrebuiltFormProps } from 'app/forms' const values = { @@ -22,12 +20,14 @@ export function CreateProjectForm({ onSuccess, onError, ...props -}: PrebuiltFormProps) { +}: PrebuiltFormProps) { const queryClient = useApiQueryClient() const addToast = useToast() + const { orgName } = useParams('orgName') + const createProject = useApiMutation('organizationProjectsPost', { - onSuccess(project, { orgName }) { + onSuccess(project) { // refetch list of projects in sidebar queryClient.invalidateQueries('organizationProjectsGet', { orgName }) // avoid the project fetch when the project page loads since we have the data @@ -42,7 +42,7 @@ export function CreateProjectForm({ content: 'Your project has been created.', timeout: 5000, }) - onSuccess?.(project, { orgName }) + onSuccess?.(project) }, onError, }) @@ -54,11 +54,10 @@ export function CreateProjectForm({ title={title} onSubmit={ onSubmit || - (({ orgName, ...body }) => { - invariant(orgName, `instance-create form is missing a path param`) + (({ name, description }) => { createProject.mutate({ orgName, - body, + body: { name, description }, }) }) } diff --git a/app/forms/subnet-create.tsx b/app/forms/subnet-create.tsx index 8c6e7023c5..61989deef0 100644 --- a/app/forms/subnet-create.tsx +++ b/app/forms/subnet-create.tsx @@ -3,8 +3,7 @@ import { Divider } from '@oxide/ui' import React from 'react' import type { VpcSubnet } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { invariant } from '@oxide/util' - +import { useParams } from 'app/hooks' import type { PrebuiltFormProps } from 'app/forms' const values = { @@ -22,17 +21,14 @@ export function CreateSubnetForm({ onSuccess, onError, ...props -}: PrebuiltFormProps< - typeof values, - VpcSubnet, - 'orgName' | 'projectName' | 'vpcName' ->) { +}: PrebuiltFormProps) { + const parentNames = useParams('orgName', 'projectName', 'vpcName') const queryClient = useApiQueryClient() const createSubnet = useApiMutation('vpcSubnetsPost', { - onSuccess(data, { body: _, ...params }) { - queryClient.invalidateQueries('vpcSubnetsGet', params) - onSuccess?.(data, params) + onSuccess(data) { + queryClient.invalidateQueries('vpcSubnetsGet', parentNames) + onSuccess?.(data) }, onError, }) @@ -43,12 +39,8 @@ export function CreateSubnetForm({ initialValues={initialValues} onSubmit={ onSubmit || - (({ orgName, projectName, vpcName, ...body }) => { - invariant( - orgName && projectName && vpcName, - 'subnet-create form is missing a path param' - ) - createSubnet.mutate({ orgName, projectName, vpcName, body }) + ((body) => { + createSubnet.mutate({ ...parentNames, body }) }) } mutation={createSubnet} diff --git a/app/forms/subnet-edit.tsx b/app/forms/subnet-edit.tsx index a8e80d160b..795a1e6395 100644 --- a/app/forms/subnet-edit.tsx +++ b/app/forms/subnet-edit.tsx @@ -19,9 +19,9 @@ export function EditSubnetForm({ const queryClient = useApiQueryClient() const updateSubnet = useApiMutation('vpcSubnetsPutSubnet', { - onSuccess(data, params) { + onSuccess(data) { queryClient.invalidateQueries('vpcSubnetsGet', parentNames) - onSuccess?.(data, params) + onSuccess?.(data) }, onError, }) diff --git a/app/routes.spec.tsx b/app/routes.spec.tsx index 22f60237e1..cd58866c6a 100644 --- a/app/routes.spec.tsx +++ b/app/routes.spec.tsx @@ -1,15 +1,9 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ -import babel from '@babel/core' -import { traverse } from '@babel/core' -import type { JSXAttribute } from '@babel/types' -import fs from 'fs/promises' -import path from 'path' - import { matchRoutes } from 'react-router-dom' import { renderAppAt } from './test/utils' -import { getRouteConfig, VALID_PARAMS } from './routes' +import { getRouteConfig } from './routes' describe('routes', () => { it('should render successfully', async () => { @@ -41,34 +35,3 @@ describe('routeConfig', () => { `) }) }) - -test('VALID_PARAMS should contain all dynamic path params', async () => { - const params: Set = new Set() - const routesFile = await fs.readFile( - path.join(__dirname, './routes.tsx'), - 'utf-8' - ) - const ast = await parse(routesFile) - - traverse(ast, { - JSXOpeningElement(path) { - if ((path.node.name as babel.types.JSXIdentifier).name !== 'Route') return - - const routePath = path.node.attributes.find( - (attr) => (attr as JSXAttribute).name.name === 'path' - ) as JSXAttribute | undefined - if (!routePath) return - const { value } = routePath.value as babel.types.StringLiteral - if (!value.startsWith(':')) return - params.add(value.slice(1)) - }, - }) - - expect(Array.from(params)).toEqual(VALID_PARAMS) -}) - -const parse = (src: string) => - babel.parseAsync(src, { - plugins: ['@babel/plugin-syntax-typescript'], - filename: __filename, - }) diff --git a/app/routes.tsx b/app/routes.tsx index dfb9a0f4ad..89961fe240 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -34,27 +34,6 @@ import { } from '@oxide/ui' import { FormPage } from './components/FormPage' -/** - * All valid dynamic route params. This contents of this - * array are validated to be in sync with the route config - * by a test in `routes.spec.tsx`. - */ -export const VALID_PARAMS = [ - 'orgName', - 'projectName', - 'instanceName', - 'vpcName', -] as const - -export type PathParam = typeof VALID_PARAMS[number] - -export const PARAM_DISPLAY: Record = { - orgName: 'Organization Name', - projectName: 'Project Name', - instanceName: 'Instance Name', - vpcName: 'Vpc Name', -} - /* * We are doing something a little unorthodox with the route config here. We * realized that tagging nodes in the route tree with arbitrary data is very diff --git a/libs/form/Form.tsx b/libs/form/Form.tsx index 3e369f4333..f9ec09527b 100644 --- a/libs/form/Form.tsx +++ b/libs/form/Form.tsx @@ -44,8 +44,7 @@ type Mutation = error: ErrorResponse } -export interface FormProps - extends FormikConfig> { +export interface FormProps extends FormikConfig { id: string title?: ReactNode children: ReactNode @@ -53,14 +52,14 @@ export interface FormProps mutation: Mutation } -export function Form({ +export function Form({ id, title, children, mutation, onDismiss, ...formikProps -}: FormProps) { +}: FormProps) { const isSideModal = useIsInSideModal() const childArray = flattenChildren(children) const actions = pluckFirstOfType(childArray, Form.Actions) From 24788996153994e197702a43b7f4b51241f6d9cf Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 18:04:03 -0400 Subject: [PATCH 17/32] Remove old comment --- libs/ui/lib/field-label/FieldLabel.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/libs/ui/lib/field-label/FieldLabel.tsx b/libs/ui/lib/field-label/FieldLabel.tsx index 86a9a82542..d816682326 100644 --- a/libs/ui/lib/field-label/FieldLabel.tsx +++ b/libs/ui/lib/field-label/FieldLabel.tsx @@ -2,9 +2,6 @@ import React from 'react' import type { ElementType, PropsWithChildren } from 'react' import { Info8Icon, Tooltip } from '@oxide/ui' -/** - * Ensures that label always has an `htmlFor` prop associated with it - */ interface FieldLabelProps { id: string as?: ElementType From fffc858416f6a643819c17a1a2b3a93c6182c155 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 18:07:22 -0400 Subject: [PATCH 18/32] Remove some left over form param references --- app/forms/disk-attach.tsx | 4 ---- app/forms/disk-create.tsx | 4 ---- app/forms/instance-create.tsx | 4 ---- 3 files changed, 12 deletions(-) diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index 64aaa6d965..b3c76fb460 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -59,10 +59,6 @@ export function AttachDiskForm({ mutation={createDisk} {...props} > - {title} diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 996bcf4651..9e5e2e1d41 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -57,10 +57,6 @@ export function CreateDiskForm({ mutation={createDisk} {...props} > - diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index de12cab116..6419d85a2b 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -102,10 +102,6 @@ export default function CreateInstanceForm({ mutation={createInstance} {...props} > - From 51f1d87ac3c90c567baa51de7751327308a5fc5a Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 18:19:46 -0400 Subject: [PATCH 19/32] Remove more form param references, cleanup type failures --- app/forms/disk-attach.tsx | 18 +- app/forms/disk-create.tsx | 1 - app/forms/instance-create.tsx | 2 - app/hooks/use-form.tsx | 4 +- .../VpcPage/modals/firewall-rules.tsx | 22 +- .../networking/VpcPage/modals/vpc-routers.tsx | 7 +- .../networking/VpcPage/modals/vpc-subnets.tsx | 199 ------------------ libs/util/classed.ts | 2 +- 8 files changed, 32 insertions(+), 223 deletions(-) delete mode 100644 app/pages/project/networking/VpcPage/modals/vpc-subnets.tsx diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index b3c76fb460..223fda81ca 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -3,7 +3,6 @@ import React from 'react' import type { Disk } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import { invariant } from '@oxide/util' -import { FormParamFields } from 'app/components/FormParamFields' import { useParams } from 'app/hooks' import type { PrebuiltFormProps } from 'app/forms' @@ -23,7 +22,7 @@ export function AttachDiskForm({ const queryClient = useApiQueryClient() const pathParams = useParams('orgName', 'projectName', 'instanceName?') - const createDisk = useApiMutation('instanceDisksAttach', { + const attachDisk = useApiMutation('instanceDisksAttach', { onSuccess(data) { const { instanceName, ...others } = pathParams invariant(instanceName, 'instanceName is required') @@ -43,20 +42,17 @@ export function AttachDiskForm({ initialValues={initialValues} onSubmit={ onSubmit || - (({ orgName, projectName, instanceName, name }) => { - invariant( - orgName && projectName && instanceName, - `disk-attach form is missing a path param` - ) - createDisk.mutate({ - orgName, - projectName, + (({ name }) => { + const { instanceName, ...others } = pathParams + invariant(instanceName, 'instanceName is required') + attachDisk.mutate({ instanceName, + ...others, body: { disk: name }, }) }) } - mutation={createDisk} + mutation={attachDisk} {...props} > diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 9e5e2e1d41..840f62944a 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -12,7 +12,6 @@ import type { Disk } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import type { PrebuiltFormProps } from 'app/forms' -import { FormParamFields } from 'app/components/FormParamFields' import { useParams } from 'app/hooks' const values = { diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 6419d85a2b..78c5de7d6c 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -21,8 +21,6 @@ import { UbuntuResponsiveIcon, WindowsResponsiveIcon, } from '@oxide/ui' -import { invariant } from '@oxide/util' -import { FormParamFields } from 'app/components/FormParamFields' import { useParams, useToast } from 'app/hooks' import { useForm } from 'app/hooks/use-form' import filesize from 'filesize' diff --git a/app/hooks/use-form.tsx b/app/hooks/use-form.tsx index f9a0d507cc..0897504577 100644 --- a/app/hooks/use-form.tsx +++ b/app/hooks/use-form.tsx @@ -31,9 +31,9 @@ export const useForm = ( }, [formProps, setShowForm]) const onSuccess = useCallback( - (data, params) => { + (data) => { setShowForm(false) - formProps?.onSuccess?.(data, params) + formProps?.onSuccess?.(data) }, [formProps, setShowForm] ) diff --git a/app/pages/project/networking/VpcPage/modals/firewall-rules.tsx b/app/pages/project/networking/VpcPage/modals/firewall-rules.tsx index 46bf6660e0..263c9d6ca1 100644 --- a/app/pages/project/networking/VpcPage/modals/firewall-rules.tsx +++ b/app/pages/project/networking/VpcPage/modals/firewall-rules.tsx @@ -73,11 +73,13 @@ const CommonForm = ({ id, error }: FormProps) => { {/* TODO: better text or heading or tip or something on this checkbox */} Enabled
- Name + + Name +
- + Description {/* TODO: indicate optional */} @@ -85,7 +87,9 @@ const CommonForm = ({ id, error }: FormProps) => {
- Priority + + Priority + Must be 0–65535 @@ -125,7 +129,9 @@ const CommonForm = ({ id, error }: FormProps) => { }} />
- Target name + + Target name +
@@ -212,7 +218,9 @@ const CommonForm = ({ id, error }: FormProps) => { So we should probably have the label on this field change when the host type changes. Also need to confirm that it's just an IP and not a block. */} - Value + + Value + For IP, an address. For the rest, a name. [TODO: copy] @@ -286,7 +294,9 @@ const CommonForm = ({ id, error }: FormProps) => {
- Port filter + + Port filter + A single port (1234) or a range (1234-2345) diff --git a/app/pages/project/networking/VpcPage/modals/vpc-routers.tsx b/app/pages/project/networking/VpcPage/modals/vpc-routers.tsx index dfd2acac02..7eb01e01fd 100644 --- a/app/pages/project/networking/VpcPage/modals/vpc-routers.tsx +++ b/app/pages/project/networking/VpcPage/modals/vpc-routers.tsx @@ -25,13 +25,18 @@ const CommonForm = ({ error, id }: FormProps) => (
- + Name
diff --git a/app/pages/project/networking/VpcPage/modals/vpc-subnets.tsx b/app/pages/project/networking/VpcPage/modals/vpc-subnets.tsx deleted file mode 100644 index af8067d1fa..0000000000 --- a/app/pages/project/networking/VpcPage/modals/vpc-subnets.tsx +++ /dev/null @@ -1,199 +0,0 @@ -import React from 'react' -import { Formik, Form } from 'formik' - -import { - Button, - FieldLabel, - SideModal_old as SideModal, - TextField, -} from '@oxide/ui' -import type { VpcSubnet, ErrorResponse } from '@oxide/api' -import { useApiMutation, useApiQueryClient } from '@oxide/api' -import { getServerError } from 'app/util/errors' - -type FormProps = { - error: ErrorResponse | null - id: string -} - -// the moment the two forms diverge, inline them rather than introducing BS -// props here -const CommonForm = ({ id, error }: FormProps) => ( - - -
- - IPv4 block - - -
-
- - IPv6 block - - -
-
- -
- - Name - - -
-
- - Description {/* TODO: indicate optional */} - - -
-
- -
{getServerError(error)}
-
- -) - -type CreateProps = { - isOpen: boolean - onDismiss: () => void - orgName: string - projectName: string - vpcName: string -} - -export function CreateVpcSubnetModal({ - isOpen, - onDismiss, - orgName, - projectName, - vpcName, -}: CreateProps) { - const parentNames = { orgName, projectName, vpcName } - const queryClient = useApiQueryClient() - - function dismiss() { - createSubnet.reset() - onDismiss() - } - - const createSubnet = useApiMutation('vpcSubnetsPost', { - onSuccess() { - queryClient.invalidateQueries('vpcSubnetsGet', parentNames) - dismiss() - }, - }) - - const formId = 'create-vpc-subnet-form' - - return ( - - { - // XXX body is optional. useApiMutation should be smarter and require body when it's required - // TODO: validate IP blocks client-side using the patterns. sadly non-trivial - createSubnet.mutate({ ...parentNames, body }) - }} - > - - - - - - - - ) -} - -type EditProps = { - onDismiss: () => void - orgName: string - projectName: string - vpcName: string - originalSubnet: VpcSubnet | null -} - -export function EditVpcSubnetModal({ - onDismiss, - orgName, - projectName, - vpcName, - originalSubnet, -}: EditProps) { - const parentNames = { orgName, projectName, vpcName } - const queryClient = useApiQueryClient() - - function dismiss() { - updateSubnet.reset() - onDismiss() - } - - const updateSubnet = useApiMutation('vpcSubnetsPutSubnet', { - onSuccess() { - queryClient.invalidateQueries('vpcSubnetsGet', parentNames) - dismiss() - }, - }) - - if (!originalSubnet) return null - - const formId = 'edit-vpc-subnet-form' - return ( - - { - updateSubnet.mutate({ - ...parentNames, - subnetName: originalSubnet.name, - body: { - name, - description, - // TODO: validate these client-side using the patterns. sadly non-trivial - ipv4Block: ipv4Block || null, - ipv6Block: ipv6Block || null, - }, - }) - }} - > - - - - - - - - ) -} diff --git a/libs/util/classed.ts b/libs/util/classed.ts index c4e500a198..94712cc30b 100644 --- a/libs/util/classed.ts +++ b/libs/util/classed.ts @@ -15,7 +15,7 @@ const make = ) // allow arbitrary components to hang off this one, e.g., Table.Body Comp.displayName = `classed.${tag}` - return Comp as typeof Comp & Record + return Comp as typeof Comp & Record> } // JSX.IntrinsicElements[T] ensures same props as the real DOM element. For example, From a5a03d0f33589f5985de525ebb1cb7cde3c25ae6 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 18:21:41 -0400 Subject: [PATCH 20/32] Fix lint issues --- app/forms/index.ts | 1 + libs/form/fields/RadioField.tsx | 1 - libs/form/fields/TableField.tsx | 1 + libs/ui/lib/radio/Radio.tsx | 2 +- libs/util/classed.ts | 2 ++ 5 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/forms/index.ts b/app/forms/index.ts index a0851d80af..62b4fa8939 100644 --- a/app/forms/index.ts +++ b/app/forms/index.ts @@ -60,6 +60,7 @@ export type ExtendedPrebuiltFormProps = C extends ComponentType< : never export type ExtractFormValues = C extends ComponentType< + // eslint-disable-next-line @typescript-eslint/no-explicit-any PrebuiltFormProps > ? V diff --git a/libs/form/fields/RadioField.tsx b/libs/form/fields/RadioField.tsx index 413e47806e..160edad05d 100644 --- a/libs/form/fields/RadioField.tsx +++ b/libs/form/fields/RadioField.tsx @@ -1,6 +1,5 @@ import type { RadioGroupProps } from '@oxide/ui' import { FieldLabel, RadioGroup, TextFieldHint } from '@oxide/ui' -import { capitalize } from '@oxide/util' import cn from 'classnames' import React from 'react' diff --git a/libs/form/fields/TableField.tsx b/libs/form/fields/TableField.tsx index f52ef9d7a9..7cfeeb31fe 100644 --- a/libs/form/fields/TableField.tsx +++ b/libs/form/fields/TableField.tsx @@ -4,6 +4,7 @@ import { Button, FieldLabel, MiniTable } from '@oxide/ui' import { capitalize } from '@oxide/util' import React from 'react' +// eslint-disable-next-line @typescript-eslint/no-explicit-any type NamedItem = { name: string; [key: string]: any } // TODO: Simplify this type diff --git a/libs/ui/lib/radio/Radio.tsx b/libs/ui/lib/radio/Radio.tsx index a197f59188..395542f181 100644 --- a/libs/ui/lib/radio/Radio.tsx +++ b/libs/ui/lib/radio/Radio.tsx @@ -6,7 +6,7 @@ * difference is that label content is handled through children. */ -import type { ComponentProps, PropsWithChildren } from 'react' +import type { ComponentProps } from 'react' import React from 'react' import cn from 'classnames' import { Field } from 'formik' diff --git a/libs/util/classed.ts b/libs/util/classed.ts index 94712cc30b..8b0331c387 100644 --- a/libs/util/classed.ts +++ b/libs/util/classed.ts @@ -15,6 +15,8 @@ const make = ) // allow arbitrary components to hang off this one, e.g., Table.Body Comp.displayName = `classed.${tag}` + + // eslint-disable-next-line @typescript-eslint/no-explicit-any return Comp as typeof Comp & Record> } From bbba80f3b9e0ac8e98d5d14ebef2c3ccf884382a Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 18:33:26 -0400 Subject: [PATCH 21/32] Fix e2e tests --- app/pages/__tests__/InstanceCreatePage.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/pages/__tests__/InstanceCreatePage.spec.tsx b/app/pages/__tests__/InstanceCreatePage.spec.tsx index ef42f76494..4da2ad2b9f 100644 --- a/app/pages/__tests__/InstanceCreatePage.spec.tsx +++ b/app/pages/__tests__/InstanceCreatePage.spec.tsx @@ -14,7 +14,7 @@ const formUrl = `/orgs/${org.name}/projects/${project.name}/instances/new` describe('InstanceCreatePage', () => { it('shows specific message for known server error code', async () => { renderAppAt(formUrl) - typeByRole('textbox', 'Choose a name', instance.name) // already exists in db + typeByRole('textbox', 'Name', instance.name) // already exists in db clickByRole('button', 'Create instance') @@ -43,7 +43,7 @@ describe('InstanceCreatePage', () => { const instancesPage = `/orgs/${org.name}/projects/${project.name}/instances` expect(window.location.pathname).not.toEqual(instancesPage) - typeByRole('textbox', 'Choose a name', 'new-instance') + typeByRole('textbox', 'Name', 'new-instance') fireEvent.click(screen.getByLabelText(/6 CPUs/)) clickByRole('button', 'Create instance') From 459548666b18aa78b14c2597826fdabe25139dcc Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 23:40:00 -0400 Subject: [PATCH 22/32] Add e2e test for instance create --- app/forms/__tests__/instance-create.e2e.ts | 26 ++++++++ .../__tests__/InstanceCreatePage.spec.tsx | 60 ------------------- 2 files changed, 26 insertions(+), 60 deletions(-) create mode 100644 app/forms/__tests__/instance-create.e2e.ts delete mode 100644 app/pages/__tests__/InstanceCreatePage.spec.tsx diff --git a/app/forms/__tests__/instance-create.e2e.ts b/app/forms/__tests__/instance-create.e2e.ts new file mode 100644 index 0000000000..0425e3d20a --- /dev/null +++ b/app/forms/__tests__/instance-create.e2e.ts @@ -0,0 +1,26 @@ +import { test, expect } from '@playwright/test' + +test.describe('Instance Create Form', () => { + test('can invoke instance create form from instances page', async ({ + page, + }) => { + await page.goto('/orgs/maze-war/projects/mock-project/instances') + await page.locator('text="New Instance"').click() + await expect(page.locator('h1:has-text("Create instance")')).toBeVisible() + + await page.fill('input[name=name]', 'mock-instance') + await page.locator('.ox-radio-card').nth(0).click() + + await page.locator('input[value=ubuntu] ~ .ox-radio-card').click() + + await page.locator('button:has-text("Create instance")').click() + + await page.waitForNavigation() + + expect(page.url()).toContain( + '/orgs/maze-war/projects/mock-project/instances/mock-instance' + ) + + await expect(page.locator('h1:has-text("mock-instance")')).toBeVisible() + }) +}) diff --git a/app/pages/__tests__/InstanceCreatePage.spec.tsx b/app/pages/__tests__/InstanceCreatePage.spec.tsx deleted file mode 100644 index 4da2ad2b9f..0000000000 --- a/app/pages/__tests__/InstanceCreatePage.spec.tsx +++ /dev/null @@ -1,60 +0,0 @@ -import { - clickByRole, - fireEvent, - override, - renderAppAt, - screen, - typeByRole, - waitFor, -} from 'app/test/utils' -import { org, project, instance } from '@oxide/api-mocks' - -const formUrl = `/orgs/${org.name}/projects/${project.name}/instances/new` - -describe('InstanceCreatePage', () => { - it('shows specific message for known server error code', async () => { - renderAppAt(formUrl) - typeByRole('textbox', 'Name', instance.name) // already exists in db - - clickByRole('button', 'Create instance') - - await screen.findByText( - 'An instance with that name already exists in this project' - ) - // don't nav away - expect(window.location.pathname).toEqual(formUrl) - }) - - it('shows generic message for unknown server error', async () => { - const createUrl = `/api/organizations/${org.name}/projects/${project.name}/instances` - override('post', createUrl, 400, { error_code: 'UnknownCode' }) - renderAppAt(formUrl) - - clickByRole('button', 'Create instance') - - await screen.findByText('Unknown error from server') - // don't nav away - expect(window.location.pathname).toEqual(formUrl) - }) - - it('navigates to project instances page on success', async () => { - renderAppAt(formUrl) - - const instancesPage = `/orgs/${org.name}/projects/${project.name}/instances` - expect(window.location.pathname).not.toEqual(instancesPage) - - typeByRole('textbox', 'Name', 'new-instance') - fireEvent.click(screen.getByLabelText(/6 CPUs/)) - - clickByRole('button', 'Create instance') - - const submit = screen.getByRole('button', { name: 'Create instance' }) - await waitFor(() => expect(submit).toBeDisabled()) - - // nav to instances list - await waitFor(() => expect(window.location.pathname).toEqual(instancesPage)) - - // new instance shows up in the list - await screen.findByText('new-instance') - }) -}) From f2d805776b6eb20973e728ec7b81376376244772 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 23:56:15 -0400 Subject: [PATCH 23/32] Update app/forms/instance-create.tsx Co-authored-by: David Crespo --- app/forms/instance-create.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 78c5de7d6c..9d4b8ad3cf 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -1,12 +1,15 @@ import type { Instance } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import type { PrebuiltFormProps } from 'app/forms' -import { TableField } from '@oxide/form' -import { TextField } from '@oxide/form' -import { RadioField } from '@oxide/form' -import { TagsField } from '@oxide/form' -import { DescriptionField, NameField } from '@oxide/form' -import { Form } from '@oxide/form' +import { + DescriptionField, + Form, + NameField, + RadioField, + TableField, + TagsField, + TextField, +} from '@oxide/form' import { CentOSResponsiveIcon, DebianResponsiveIcon, From 2a11866caa0caa43fccc804a26e9858ca01e5d66 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 7 Apr 2022 23:59:08 -0400 Subject: [PATCH 24/32] export use-forms from app/forms/index --- app/forms/instance-create.tsx | 3 +-- app/hooks/index.ts | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 78c5de7d6c..af3d3ef4aa 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -21,8 +21,7 @@ import { UbuntuResponsiveIcon, WindowsResponsiveIcon, } from '@oxide/ui' -import { useParams, useToast } from 'app/hooks' -import { useForm } from 'app/hooks/use-form' +import { useParams, useToast, useForm } from 'app/hooks' import filesize from 'filesize' import React from 'react' import type { FormValues } from '.' diff --git a/app/hooks/index.ts b/app/hooks/index.ts index 55f7155680..fc1bedf280 100644 --- a/app/hooks/index.ts +++ b/app/hooks/index.ts @@ -2,3 +2,4 @@ export * from './use-quick-actions' export * from './use-key' export * from './use-params' export * from './use-toast' +export * from './use-form' From e4c0c75b8161165c386153570882a1e2a8dfb249 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 8 Apr 2022 10:52:27 -0400 Subject: [PATCH 25/32] Wire up disk creates --- app/forms/instance-create.tsx | 5 +++-- app/hooks/use-params.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 0aebc6e569..00faa42824 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -35,7 +35,7 @@ const values = { tags: {}, type: '', hostname: '', - newDisks: [], + disks: [], attachedDisks: [], } @@ -95,6 +95,7 @@ export default function CreateInstanceForm({ memory: filesize(instance.memory, { output: 'object', base: 2 }) .value, ncpus: instance.ncpus, + disks: values.disks, }, }) }) @@ -213,7 +214,7 @@ export default function CreateInstanceForm({ > id="new-disks" label="" - name="New disks" + name="disks" actions={[ [ 'Create new disk', diff --git a/app/hooks/use-params.ts b/app/hooks/use-params.ts index 7de336b30f..e3dcd98774 100644 --- a/app/hooks/use-params.ts +++ b/app/hooks/use-params.ts @@ -23,7 +23,7 @@ export function useParams( if (process.env.NODE_ENV !== 'production') { for (const k of paramNames) { invariant( - k in params, + k.endsWith('?') || k in params, `Param '${k}' not found in route. You might be rendering a component under the wrong route.` ) } From 880ba8823d6e4a9f91db03c2e550457d36b3a846 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 8 Apr 2022 10:55:59 -0400 Subject: [PATCH 26/32] Remove disk size, capitalize source type --- app/forms/instance-create.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 00faa42824..92b30354f7 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -28,6 +28,7 @@ import { useParams, useToast, useForm } from 'app/hooks' import filesize from 'filesize' import React from 'react' import type { FormValues } from '.' +import { capitalize } from '@oxide/util' const values = { name: '', @@ -210,7 +211,7 @@ export default function CreateInstanceForm({ & { type: 'create' }) - | (FormValues<'disk-attach'> & { type: 'attach'; size: number }) + | (FormValues<'disk-attach'> & { type: 'attach' }) > id="new-disks" label="" @@ -233,7 +234,7 @@ export default function CreateInstanceForm({ const hideForm = showAttachDiskForm({ onSubmit: (values) => { // TODO fetch disk size - addDisk({ type: 'attach', ...values, size: 0 }) + addDisk({ type: 'attach', ...values }) hideForm() }, }) @@ -242,8 +243,7 @@ export default function CreateInstanceForm({ ]} columns={[ ['name', 'Name'], - ['type', 'Type'], - ['size', 'Size', filesize], + ['type', 'Type', capitalize], ]} /> {createDiskForm} From 1e0ee980b2bd230a611b4063f1dc704d96048fb1 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 8 Apr 2022 12:41:33 -0400 Subject: [PATCH 27/32] Fix disk attach api --- app/forms/instance-create.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 92b30354f7..71b2871d35 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -234,7 +234,8 @@ export default function CreateInstanceForm({ const hideForm = showAttachDiskForm({ onSubmit: (values) => { // TODO fetch disk size - addDisk({ type: 'attach', ...values }) + // @ts-expect-error The API here needs to be updated to be name instead of disk + addDisk({ type: 'attach', disk: values.name }) hideForm() }, }) From 92a59b6e49b6998e8b5d74d59bc5a52a9ad9f0c8 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 8 Apr 2022 13:39:21 -0400 Subject: [PATCH 28/32] revert disk attach api change --- app/forms/instance-create.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 71b2871d35..92b30354f7 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -234,8 +234,7 @@ export default function CreateInstanceForm({ const hideForm = showAttachDiskForm({ onSubmit: (values) => { // TODO fetch disk size - // @ts-expect-error The API here needs to be updated to be name instead of disk - addDisk({ type: 'attach', disk: values.name }) + addDisk({ type: 'attach', ...values }) hideForm() }, }) From 89a86d84ee3bdd5cd3f14590dad277d07a957445 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 8 Apr 2022 16:05:36 -0500 Subject: [PATCH 29/32] update disk name thing --- app/forms/disk-attach.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index 223fda81ca..a652d38467 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -48,7 +48,7 @@ export function AttachDiskForm({ attachDisk.mutate({ instanceName, ...others, - body: { disk: name }, + body: { name }, }) }) } From a21e9b5455d5d74bc8a0dee62e5877aca44e3e65 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 8 Apr 2022 23:38:51 -0500 Subject: [PATCH 30/32] Invert disks table logic flow (#777) * it works! * fully inline disks MiniTable * DisksTable -> DisksTableField * move DisksTableField to its own file to reduce noise in instant create * convert VPC subnets and delete useForm --- app/components/DisksTableField.tsx | 108 ++++++++++++++++++ app/forms/instance-create.tsx | 50 +------- app/hooks/index.ts | 1 - app/hooks/use-form.tsx | 55 --------- .../networking/VpcPage/tabs/VpcSubnetsTab.tsx | 36 ++++-- libs/form/fields/TableField.tsx | 96 ---------------- libs/form/fields/index.ts | 1 - 7 files changed, 139 insertions(+), 208 deletions(-) create mode 100644 app/components/DisksTableField.tsx delete mode 100644 app/hooks/use-form.tsx delete mode 100644 libs/form/fields/TableField.tsx diff --git a/app/components/DisksTableField.tsx b/app/components/DisksTableField.tsx new file mode 100644 index 0000000000..87ce56e718 --- /dev/null +++ b/app/components/DisksTableField.tsx @@ -0,0 +1,108 @@ +import React, { useState } from 'react' +import { useField } from 'formik' +import { CreateDiskForm } from 'app/forms/disk-create' +import { AttachDiskForm } from 'app/forms/disk-attach' +import { + Button, + Error16Icon, + FieldLabel, + MiniTable, + SideModal, +} from '@oxide/ui' +import type { FormValues } from '../forms' + +type DiskTableItem = + | (FormValues<'disk-create'> & { type: 'create' }) + | (FormValues<'disk-attach'> & { type: 'attach' }) + +export function DisksTableField() { + const [showDiskCreate, setShowDiskCreate] = useState(false) + const [showDiskAttach, setShowDiskAttach] = useState(false) + + const [, { value: items = [] }, { setValue: setItems }] = useField< + DiskTableItem[] + >({ name: 'disks' }) + + return ( + <> +
+ {/* this was empty */} + {!!items.length && ( + + + Name + Type + {/* For remove button */} + + + + {items.map((item, index) => ( + + {item.name} + {item.type} + + + + + ))} + + + )} + +
+ + +
+
+ + setShowDiskCreate(false)} + > + { + setItems([...items, { type: 'create', ...values }]) + setShowDiskCreate(false) + }} + /> + + setShowDiskAttach(false)} + > + { + setItems([...items, { type: 'attach', ...values }]) + setShowDiskAttach(false) + }} + /> + + + ) +} diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 92b30354f7..8652ecc98b 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -1,3 +1,4 @@ +import React from 'react' import type { Instance } from '@oxide/api' import { useApiMutation, useApiQueryClient } from '@oxide/api' import type { PrebuiltFormProps } from 'app/forms' @@ -6,7 +7,6 @@ import { Form, NameField, RadioField, - TableField, TagsField, TextField, } from '@oxide/form' @@ -24,11 +24,9 @@ import { UbuntuResponsiveIcon, WindowsResponsiveIcon, } from '@oxide/ui' -import { useParams, useToast, useForm } from 'app/hooks' +import { useParams, useToast } from 'app/hooks' +import { DisksTableField } from 'app/components/DisksTableField' import filesize from 'filesize' -import React from 'react' -import type { FormValues } from '.' -import { capitalize } from '@oxide/util' const values = { name: '', @@ -51,8 +49,6 @@ export default function CreateInstanceForm({ }: PrebuiltFormProps) { const queryClient = useApiQueryClient() const addToast = useToast() - const [createDiskForm, showCreateDiskForm] = useForm('disk-create') - const [attachDiskForm, showAttachDiskForm] = useForm('disk-attach') const pageParams = useParams('orgName', 'projectName') const createInstance = useApiMutation('projectInstancesPost', { @@ -209,45 +205,7 @@ export default function CreateInstanceForm({ Additional disks - & { type: 'create' }) - | (FormValues<'disk-attach'> & { type: 'attach' }) - > - id="new-disks" - label="" - name="disks" - actions={[ - [ - 'Create new disk', - (addDisk) => { - const hideForm = showCreateDiskForm({ - onSubmit: (values) => { - addDisk({ type: 'create', ...values }) - hideForm() - }, - }) - }, - ], - [ - 'Attach existing disk', - (addDisk) => { - const hideForm = showAttachDiskForm({ - onSubmit: (values) => { - // TODO fetch disk size - addDisk({ type: 'attach', ...values }) - hideForm() - }, - }) - }, - ], - ]} - columns={[ - ['name', 'Name'], - ['type', 'Type', capitalize], - ]} - /> - {createDiskForm} - {attachDiskForm} + Networking diff --git a/app/hooks/index.ts b/app/hooks/index.ts index fc1bedf280..55f7155680 100644 --- a/app/hooks/index.ts +++ b/app/hooks/index.ts @@ -2,4 +2,3 @@ export * from './use-quick-actions' export * from './use-key' export * from './use-params' export * from './use-toast' -export * from './use-form' diff --git a/app/hooks/use-form.tsx b/app/hooks/use-form.tsx deleted file mode 100644 index 0897504577..0000000000 --- a/app/hooks/use-form.tsx +++ /dev/null @@ -1,55 +0,0 @@ -import type { ComponentProps } from 'react' -import { SideModal } from '@oxide/ui' -import { useCallback } from 'react' -import { useState, Suspense, useMemo } from 'react' -import React from 'react' -import type { FormTypes } from 'app/forms' - -/** - * Dynamically load a form from the `forms` directory where id is the name of the form. It - * returns an element that can be used to render the form and an invocation function to display - * the form. The invocation can take the form's props to alter the form's behavior. - */ -export const useForm = ( - type: K, - props?: ComponentProps -) => { - const [isOpen, setShowForm] = useState(false) - const [formProps, setFormProps] = useState(props) - - const showForm = (innerProps?: typeof props) => { - if (innerProps) { - setFormProps(innerProps) - } - setShowForm(true) - return () => setShowForm(false) - } - - const onDismiss = useCallback(() => { - setShowForm(false) - formProps?.onDismiss?.() - }, [formProps, setShowForm]) - - const onSuccess = useCallback( - (data) => { - setShowForm(false) - formProps?.onSuccess?.(data) - }, - [formProps, setShowForm] - ) - - const DynForm = useMemo( - () => React.lazy(() => import(`../forms/${type}.tsx`)), - [type] - ) - - return [ - - - {/* @ts-expect-error TODO: Figure out why this is erroring */} - - - , - showForm, - ] as const -} diff --git a/app/pages/project/networking/VpcPage/tabs/VpcSubnetsTab.tsx b/app/pages/project/networking/VpcPage/tabs/VpcSubnetsTab.tsx index 40c4d36f04..1ac19bac1e 100644 --- a/app/pages/project/networking/VpcPage/tabs/VpcSubnetsTab.tsx +++ b/app/pages/project/networking/VpcPage/tabs/VpcSubnetsTab.tsx @@ -1,22 +1,23 @@ -import React from 'react' +import React, { useState } from 'react' import { useParams } from 'app/hooks' import type { MenuAction } from '@oxide/table' import { useQueryTable, TwoLineCell, DateCell } from '@oxide/table' -import { Button } from '@oxide/ui' +import { Button, SideModal } from '@oxide/ui' import type { VpcSubnet } from '@oxide/api' -import { useForm } from 'app/hooks/use-form' +import { CreateSubnetForm } from 'app/forms/subnet-create' +import { EditSubnetForm } from 'app/forms/subnet-edit' export const VpcSubnetsTab = () => { const vpcParams = useParams('orgName', 'projectName', 'vpcName') const { Table, Column } = useQueryTable('vpcSubnetsGet', vpcParams) - const [createSubnetForm, showCreateSubnet] = useForm('subnet-create') - const [editSubnetForm, showEditSubnet] = useForm('subnet-edit') + const [showCreate, setShowCreate] = useState(false) + const [editing, setEditing] = useState(null) const makeActions = (subnet: VpcSubnet): MenuAction[] => [ { label: 'Edit', - onActivate: () => showEditSubnet({ initialValues: subnet }), + onActivate: () => setEditing(subnet), }, ] @@ -26,12 +27,29 @@ export const VpcSubnetsTab = () => { - {createSubnetForm} - {editSubnetForm} + setShowCreate(false)} + > + setShowCreate(false)} /> + + setEditing(null)} + > + {editing && ( + setEditing(null)} + /> + )} +
diff --git a/libs/form/fields/TableField.tsx b/libs/form/fields/TableField.tsx deleted file mode 100644 index 7cfeeb31fe..0000000000 --- a/libs/form/fields/TableField.tsx +++ /dev/null @@ -1,96 +0,0 @@ -import { useField } from 'formik' -import { Error16Icon } from '@oxide/ui' -import { Button, FieldLabel, MiniTable } from '@oxide/ui' -import { capitalize } from '@oxide/util' -import React from 'react' - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type NamedItem = { name: string; [key: string]: any } - -// TODO: Simplify this type -type Column = K extends keyof (infer Item) - ? [key: K, name: string, format?: (value: Item[K]) => string] - : never - -type Action = [ - name: string, - action: (addItem: (item: Item) => void) => void -] - -export interface TableFieldProps { - id: string - name?: string - label?: string - onRemoveItem?: (item: Item) => void - columns: Column[] - actions: Action[] -} - -export function TableField({ - id, - name = id, - label = capitalize(name), - onRemoveItem, - columns, - actions, -}: TableFieldProps) { - const [, { value = [] }, { setValue }] = useField({ name }) - - return ( -
- {label} - {!!value.length && ( - - - {columns.map(([key, display]) => ( - {display} - ))} - {/* For remove button */} - - - - {value.map((item, index) => ( - `${name}: ${item[key]}`) - .join(' ')} - key={`item-${index}`} - > - {columns.map(([key]) => ( - - {item[key]} - - ))} - - - - - ))} - - - )} -
- {actions.map(([name, action]) => ( - - ))} -
-
- ) -} diff --git a/libs/form/fields/index.ts b/libs/form/fields/index.ts index e86498ca07..8777a1d04c 100644 --- a/libs/form/fields/index.ts +++ b/libs/form/fields/index.ts @@ -3,4 +3,3 @@ export * from './TextField' export * from './NameField' export * from './TagsField' export * from './RadioField' -export * from './TableField' From f64f84fe296c41c5e267338f4f0317122be5fce6 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Sat, 9 Apr 2022 08:41:03 -0500 Subject: [PATCH 31/32] allow any param, but it might be undefined (#779) --- app/forms/disk-attach.tsx | 2 +- app/hooks/use-params.ts | 27 +++++++++++---------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index a652d38467..6d2751b8c0 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -20,7 +20,7 @@ export function AttachDiskForm({ ...props }: PrebuiltFormProps) { const queryClient = useApiQueryClient() - const pathParams = useParams('orgName', 'projectName', 'instanceName?') + const pathParams = useParams('orgName', 'projectName') const attachDisk = useApiMutation('instanceDisksAttach', { onSuccess(data) { diff --git a/app/hooks/use-params.ts b/app/hooks/use-params.ts index e3dcd98774..e272978c35 100644 --- a/app/hooks/use-params.ts +++ b/app/hooks/use-params.ts @@ -1,32 +1,27 @@ +import type { Params } from 'react-router-dom' import { useParams as _useParams } from 'react-router-dom' import { invariant } from '@oxide/util' -type OptionalKey = K extends `${infer U}?` ? U : never -type RequiredKey = K extends OptionalKey ? never : K - -type ParamsWithOptionalKeys = { - [key in RequiredKey]: string -} & { - [key in OptionalKey]?: string -} - /** * Wrapper for React Router's `useParams` that throws (in dev) if any of the - * specified params is missing. + * specified params is missing. The specified params are guaranteed by TS to be + * present on the resulting params object. Any other param is allowed to be + * pulled off the object, but TS will require you to check if it's undefined. * - * @returns an object where the params are guaranteed (in dev) to be present + * @returns an object where the specified params are guaranteed (in dev) to be + * present */ -export function useParams( - ...paramNames: K[] -): ParamsWithOptionalKeys { +// default of never is required to prevent the highly undesirable property that if +// you don't pass any arguments, the result object thinks every property is defined +export function useParams(...paramNames: K[]) { const params = _useParams() if (process.env.NODE_ENV !== 'production') { for (const k of paramNames) { invariant( - k.endsWith('?') || k in params, + k in params, `Param '${k}' not found in route. You might be rendering a component under the wrong route.` ) } } - return params as ParamsWithOptionalKeys + return params as { readonly [k in K]: string } & Params } From 18e222f976e2e98cb6ca129676f885fd8e4c5909 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 11 Apr 2022 12:38:25 -0400 Subject: [PATCH 32/32] Address PR feedback, validateName -> getNameValidator --- libs/form/fields/NameField.spec.tsx | 4 ++-- libs/form/fields/NameField.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/form/fields/NameField.spec.tsx b/libs/form/fields/NameField.spec.tsx index d967246f67..1daf3be52e 100644 --- a/libs/form/fields/NameField.spec.tsx +++ b/libs/form/fields/NameField.spec.tsx @@ -1,7 +1,7 @@ -import { validateName } from './NameField' +import { getNameValidator } from './NameField' describe('validateName', () => { - const validate = validateName('Name', true) + const validate = getNameValidator('Name', true) it('returns undefined for valid names', () => { expect(validate('abc')).toBeUndefined() expect(validate('abc-def')).toBeUndefined() diff --git a/libs/form/fields/NameField.tsx b/libs/form/fields/NameField.tsx index f01da77576..1beafe3562 100644 --- a/libs/form/fields/NameField.tsx +++ b/libs/form/fields/NameField.tsx @@ -16,7 +16,7 @@ export function NameField({ }: NameFieldProps) { return ( (name: string) => { if (!required && !name) return