From ff7cdb28ae50e908473fb1292ed502f2671e0a6e Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 31 Jan 2024 12:03:30 -0800 Subject: [PATCH] Use NumberFields in more places; enhance NumberField component (#1926) * Use NumberFields in more places; enhance NumberField component * Apply default minimum of 0 on NumberFields * Refactor onChange logic in TextFields, permit 'text' and 'password' types --- app/components/form/fields/DiskSizeField.tsx | 1 - app/components/form/fields/NumberField.tsx | 8 +++- app/components/form/fields/TextField.tsx | 39 ++------------------ app/forms/instance-create.tsx | 7 ++-- app/forms/silo-create.tsx | 3 -- app/test/e2e/instance-create.e2e.ts | 4 +- 6 files changed, 15 insertions(+), 47 deletions(-) diff --git a/app/components/form/fields/DiskSizeField.tsx b/app/components/form/fields/DiskSizeField.tsx index 7d97e3ca54..cf0a0a021a 100644 --- a/app/components/form/fields/DiskSizeField.tsx +++ b/app/components/form/fields/DiskSizeField.tsx @@ -38,7 +38,6 @@ export function DiskSizeField< return ( ) => { const generatedId = useId() const id = idProp || generatedId @@ -89,6 +91,8 @@ export const NumberFieldInner = < })} aria-describedby={tooltipText ? `${id}-label-tip` : undefined} isDisabled={disabled} + maxValue={max ? Number(max) : undefined} + minValue={min !== undefined ? Number(min) : undefined} {...field} formatOptions={{ useGrouping: false, diff --git a/app/components/form/fields/TextField.tsx b/app/components/form/fields/TextField.tsx index e341be08aa..4d561c8585 100644 --- a/app/components/form/fields/TextField.tsx +++ b/app/components/form/fields/TextField.tsx @@ -33,7 +33,7 @@ export interface TextFieldProps< > extends UITextFieldProps { name: TName /** HTML type attribute, defaults to text */ - type?: string + type?: 'text' | 'password' /** Will default to name if not provided */ label?: string /** @@ -92,17 +92,9 @@ export function TextField< ) } -function numberToInputValue(value: number) { - // could add `|| value === 0`, but that means when the value is 0, we always - // show an empty string, which is weird, and doubly bad because then the - // browser apparently fails to validate it against minimum (if one is - // provided). I found it let me submit instance create with 0 CPUs. - return isNaN(value) ? '' : value.toString() -} - /** * Primarily exists for `TextField`, but we occasionally also need a plain field - * without a label on it. Note special handling of `type="number"`. + * without a label on it. * * Note that `id` is an allowed prop, unlike in `TextField`, where it is always * generated from `name`. This is because we need to pass the generated ID in @@ -131,7 +123,7 @@ export const TextFieldInner = < name={name} control={control} rules={{ required, validate }} - render={({ field: { onChange, value, ...fieldRest }, fieldState: { error } }) => { + render={({ field: { onChange, ...fieldRest }, fieldState: { error } }) => { return ( <> { - if (transform) { - onChange(transform(e.target.value)) - return - } - if (type === 'number') { - if (e.target.value.trim() === '') { - onChange(0) - } else if (!isNaN(e.target.valueAsNumber)) { - onChange(e.target.valueAsNumber) - } - // otherwise ignore the input. this means, for example, typing - // letters does nothing. If we instead said take anything - // that's NaN and fall back to 0, typing a letter would reset - // the field to 0, which is silly. Browsers are supposed to - // ignore non-numeric input for you anyway, but Firefox does - // not. - return - } - - onChange(e.target.value) + onChange(transform ? transform(e.target.value) : e.target.value) }} - value={type === 'number' ? numberToInputValue(value) : value} {...fieldRest} {...props} /> diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 6d12a2ae6a..cec3644edc 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -46,6 +46,7 @@ import { ImageSelectField, NameField, NetworkInterfaceField, + NumberField, RadioFieldDyn, SshKeysField, TextField, @@ -290,8 +291,7 @@ export function CreateInstanceForm() { - - @@ -117,7 +116,6 @@ export function CreateSiloSideModalForm() { label="Memory quota" name="quotas.memory" required - type="number" units="GiB" validate={validateQuota} /> @@ -126,7 +124,6 @@ export function CreateSiloSideModalForm() { label="Storage quota" name="quotas.storage" required - type="number" units="GiB" validate={validateQuota} /> diff --git a/app/test/e2e/instance-create.e2e.ts b/app/test/e2e/instance-create.e2e.ts index cb964cefaf..5cc626c2b8 100644 --- a/app/test/e2e/instance-create.e2e.ts +++ b/app/test/e2e/instance-create.e2e.ts @@ -109,8 +109,8 @@ test('can create an instance with custom hardware', async ({ page }) => { // Fill in custom specs await page.getByRole('tab', { name: 'Custom' }).click() - await page.fill('input[name=ncpus]', '29') - await page.fill('input[name=memory]', '53') + await page.getByRole('textbox', { name: 'CPUs' }).fill('29') + await page.getByRole('textbox', { name: 'Memory (GiB)' }).fill('53') await page.getByRole('textbox', { name: 'Disk name' }).fill('my-boot-disk') const diskSizeInput = page.getByRole('textbox', { name: 'Disk size (GiB)' })