Skip to content

Commit 1ccff37

Browse files
benjaminleonardgithub-actions[bot]david-crespo
authored
Number input (#1582)
* Number input first pass * Add number field * Update libs/ui/lib/number-input/NumberInput.stories.tsx Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update libs/ui/lib/number-input/NumberInput.tsx Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Bot commit: format with prettier * fix defaultValue and onChange (mostly), only use number input on number fields * actually fix the type error by getting rid of the spread * props cleanup * update e2es * fix date picker state type error, use isInvalid (validationState deprecated) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
1 parent 0cc1e03 commit 1ccff37

File tree

12 files changed

+3339
-1818
lines changed

12 files changed

+3339
-1818
lines changed

app/components/form/fields/DiskSizeField.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@
55
*
66
* Copyright Oxide Computer Company
77
*/
8-
import type { FieldPath, FieldValues } from 'react-hook-form'
8+
import type { FieldPath, FieldPathByValue, FieldValues } from 'react-hook-form'
99

1010
import { MAX_DISK_SIZE_GiB } from '@oxide/api'
1111

12+
import { NumberField } from './NumberField'
1213
import type { TextFieldProps } from './TextField'
13-
import { TextField } from './TextField'
1414

1515
interface DiskSizeProps<
1616
TFieldValues extends FieldValues,
@@ -21,10 +21,10 @@ interface DiskSizeProps<
2121

2222
export function DiskSizeField<
2323
TFieldValues extends FieldValues,
24-
TName extends FieldPath<TFieldValues>
24+
TName extends FieldPathByValue<TFieldValues, number>
2525
>({ required = true, name, minSize = 1, ...props }: DiskSizeProps<TFieldValues, TName>) {
2626
return (
27-
<TextField
27+
<NumberField
2828
units="GiB"
2929
type="number"
3030
required={required}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* This Source Code Form is subject to the terms of the Mozilla Public
3+
* License, v. 2.0. If a copy of the MPL was not distributed with this
4+
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
5+
*
6+
* Copyright Oxide Computer Company
7+
*/
8+
import cn from 'classnames'
9+
import { useId } from 'react'
10+
import type { FieldPathByValue, FieldValues } from 'react-hook-form'
11+
import { Controller } from 'react-hook-form'
12+
13+
import { FieldLabel, TextInputHint, NumberInput as UINumberField } from '@oxide/ui'
14+
import { capitalize } from '@oxide/util'
15+
16+
import { ErrorMessage } from './ErrorMessage'
17+
import type { TextFieldProps } from './TextField'
18+
19+
export function NumberField<
20+
TFieldValues extends FieldValues,
21+
// can only be used on fields with number values
22+
TName extends FieldPathByValue<TFieldValues, number>
23+
>({
24+
name,
25+
label = capitalize(name),
26+
units,
27+
description,
28+
helpText,
29+
required,
30+
...props
31+
}: Omit<TextFieldProps<TFieldValues, TName>, 'id'>) {
32+
// id is omitted from props because we generate it here
33+
const id = useId()
34+
return (
35+
<div className="max-w-lg">
36+
<div className="mb-2">
37+
<FieldLabel id={`${id}-label`} tip={description} optional={!required}>
38+
{label} {units && <span className="ml-1 text-secondary">({units})</span>}
39+
</FieldLabel>
40+
{helpText && (
41+
<TextInputHint id={`${id}-help-text`} className="mb-2">
42+
{helpText}
43+
</TextInputHint>
44+
)}
45+
</div>
46+
{/* passing the generated id is very important for a11y */}
47+
<NumberFieldInner name={name} {...props} id={id} />
48+
</div>
49+
)
50+
}
51+
52+
/**
53+
* Primarily exists for `NumberField`, but we occasionally also need a plain field
54+
* without a label on it.
55+
*
56+
* Note that `id` is an allowed prop, unlike in `TextField`, where it is always
57+
* generated from `name`. This is because we need to pass the generated ID in
58+
* from there to here. For the case where `TextFieldInner` is used
59+
* independently, we also generate an ID for use only if none is passed in.
60+
*/
61+
export const NumberFieldInner = <
62+
TFieldValues extends FieldValues,
63+
TName extends FieldPathByValue<TFieldValues, number>
64+
>({
65+
name,
66+
label = capitalize(name),
67+
validate,
68+
control,
69+
description,
70+
required,
71+
id: idProp,
72+
}: TextFieldProps<TFieldValues, TName>) => {
73+
const generatedId = useId()
74+
const id = idProp || generatedId
75+
76+
return (
77+
<Controller
78+
name={name}
79+
control={control}
80+
rules={{ required, validate }}
81+
render={({ field: { value, ...fieldRest }, fieldState: { error } }) => {
82+
return (
83+
<>
84+
<UINumberField
85+
id={id}
86+
error={!!error}
87+
aria-labelledby={cn(`${id}-label`, {
88+
[`${id}-help-text`]: !!description,
89+
})}
90+
aria-describedby={description ? `${id}-label-tip` : undefined}
91+
defaultValue={value}
92+
{...fieldRest}
93+
/>
94+
<ErrorMessage error={error} label={label} />
95+
</>
96+
)
97+
}}
98+
/>
99+
)
100+
}

app/test/e2e/click-everything.e2e.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ test('Click through disks page', async ({ page }) => {
5252
'role=textbox[name="Name"]',
5353
'role=textbox[name="Description"]',
5454
'role=radiogroup[name="Block size (Bytes)"]',
55-
'role=spinbutton[name="Size (GiB)"]',
55+
'role=textbox[name="Size (GiB)"]',
5656
'role=button[name="Create Disk"]',
5757
])
5858
await page.goBack()

app/test/e2e/instance-create.e2e.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ test('can create an instance', async ({ page }) => {
2222
'role=textbox[name="Name"]',
2323
'role=textbox[name="Description"]',
2424
'role=textbox[name="Disk name"]',
25-
'role=spinbutton[name="Disk size (GiB)"]',
25+
'role=textbox[name="Disk size (GiB)"]',
2626
'role=radiogroup[name="Network interface"]',
2727
'role=textbox[name="Hostname"]',
2828
'role=button[name="Create instance"]',
@@ -33,8 +33,9 @@ test('can create an instance', async ({ page }) => {
3333
await page.fill('textarea[name=description]', 'An instance... from space!')
3434
await page.locator('.ox-radio-card').nth(3).click()
3535

36-
await page.fill('input[name=bootDiskName]', 'my-boot-disk')
37-
await page.fill('input[name=bootDiskSize]', '20')
36+
await page.getByRole('textbox', { name: 'Disk name' }).fill('my-boot-disk')
37+
const diskSizeInput = page.getByRole('textbox', { name: 'Disk size (GiB)' })
38+
await diskSizeInput.fill('20')
3839

3940
// pick a project image just to show we can
4041
await page.getByRole('tab', { name: 'Project images' }).click()
@@ -95,20 +96,22 @@ test('can create an instance with custom hardware', async ({ page }) => {
9596
await page.fill('input[name=ncpus]', '29')
9697
await page.fill('input[name=memory]', '53')
9798

98-
await page.fill('input[name=bootDiskName]', 'my-boot-disk')
99-
await page.fill('input[name=bootDiskSize]', '20')
99+
await page.getByRole('textbox', { name: 'Disk name' }).fill('my-boot-disk')
100+
const diskSizeInput = page.getByRole('textbox', { name: 'Disk size (GiB)' })
101+
await diskSizeInput.fill('20')
100102

101103
// pick a project image just to show we can
102104
await page.getByRole('tab', { name: 'Project images' }).click()
103105
await page.getByRole('button', { name: 'Image' }).click()
104106
await page.getByRole('option', { name: images[2].name }).click()
105107

106108
// test disk size validation against image size
107-
await page.getByRole('spinbutton', { name: 'Disk size (GiB)' }).fill('5')
109+
await diskSizeInput.fill('5')
110+
await diskSizeInput.blur() // need blur to trigger validation
108111
await expectVisible(page, [
109112
'main >> text=Must be as large as selected image (min. 6 GiB)',
110113
])
111-
await page.getByRole('spinbutton', { name: 'Disk size (GiB)' }).fill('10')
114+
await diskSizeInput.fill('10')
112115

113116
await page.getByRole('button', { name: 'Create instance' }).click()
114117

app/test/e2e/instance/attach-disk.e2e.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ test('Attach disk', async ({ page }) => {
3737
'role=textbox[name="Name"]',
3838
'role=textbox[name="Description"]',
3939
'role=radiogroup[name="Block size (Bytes)"]',
40-
'role=spinbutton[name="Size (GiB)"]',
40+
'role=textbox[name="Size (GiB)"]',
4141
'role=button[name="Create Disk"]',
4242
])
4343
await page.click('role=button[name="Cancel"]')

libs/ui/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export * from './lib/message/Message'
3131
export * from './lib/modal/Modal'
3232
export * from './lib/ModalLinks'
3333
export * as MiniTable from './lib/mini-table/MiniTable'
34+
export * from './lib/number-input/NumberInput'
3435
export * from './lib/page-header/PageHeader'
3536
export * from './lib/pagination/Pagination'
3637
export * from './lib/progress/Progress'

libs/ui/lib/date-picker/DatePicker.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,14 @@ export function DatePicker(props: DatePickerProps) {
6666
className={cn(
6767
state.isOpen && 'z-10 ring-2',
6868
'relative flex h-10 items-center rounded-l rounded-r border text-sans-md border-default focus-within:ring-2 hover:border-raise focus:z-10',
69-
state.validationState === 'invalid'
69+
state.isInvalid
7070
? 'focus-error border-error ring-error-secondary'
7171
: 'border-default ring-accent-secondary'
7272
)}
7373
>
7474
<div className={cn('relative flex w-[10rem] items-center px-3 text-sans-md')}>
7575
{label}
76-
{state.validationState === 'invalid' && (
76+
{state.isInvalid && (
7777
<div className="absolute bottom-0 right-2 top-0 flex items-center text-error">
7878
<Error12Icon className="h-3 w-3" />
7979
</div>
@@ -84,7 +84,7 @@ export function DatePicker(props: DatePickerProps) {
8484
</div>
8585
</button>
8686
</div>
87-
{state.validationState === 'invalid' && (
87+
{state.isInvalid && (
8888
<p {...errorMessageProps} className="py-2 text-sans-md text-error">
8989
Date is invalid
9090
</p>

libs/ui/lib/date-picker/DateRangePicker.tsx

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,17 @@ export function DateRangePicker(props: DateRangePickerProps) {
4141
hourCycle: 'h24',
4242
})
4343

44-
const label = useMemo(
45-
() =>
46-
formatter.formatRange(
47-
state.dateRange.start.toDate(getLocalTimeZone()),
48-
state.dateRange.end.toDate(getLocalTimeZone())
49-
),
50-
[state, formatter]
51-
)
44+
const label = useMemo(() => {
45+
// This is here to make TS happy. This should be impossible in practice
46+
// because we always pass a value to this component and there is no way to
47+
// unset the value through the UI.
48+
if (!state.dateRange) return 'No range selected'
49+
50+
return formatter.formatRange(
51+
state.dateRange.start.toDate(getLocalTimeZone()),
52+
state.dateRange.end.toDate(getLocalTimeZone())
53+
)
54+
}, [state.dateRange, formatter])
5255

5356
return (
5457
<div
@@ -62,14 +65,14 @@ export function DateRangePicker(props: DateRangePickerProps) {
6265
className={cn(
6366
state.isOpen && 'z-10 ring-2',
6467
'relative flex h-10 items-center rounded-l rounded-r border text-sans-md border-default focus-within:ring-2 hover:border-raise focus:z-10',
65-
state.validationState === 'invalid'
68+
state.isInvalid
6669
? 'focus-error border-error ring-error-secondary'
6770
: 'border-default ring-accent-secondary'
6871
)}
6972
>
7073
<div className={cn('relative flex w-[16rem] items-center px-3 text-sans-md')}>
7174
{label}
72-
{state.validationState === 'invalid' && (
75+
{state.isInvalid && (
7376
<div className="absolute bottom-0 right-2 top-0 flex items-center text-error">
7477
<Error12Icon className="h-3 w-3" />
7578
</div>
@@ -80,7 +83,7 @@ export function DateRangePicker(props: DateRangePickerProps) {
8083
</div>
8184
</button>
8285
</div>
83-
{state.validationState === 'invalid' && (
86+
{state.isInvalid && (
8487
<p {...errorMessageProps} className="py-2 text-sans-md text-error">
8588
Date range is invalid
8689
</p>
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* This Source Code Form is subject to the terms of the Mozilla Public
3+
* License, v. 2.0. If a copy of the MPL was not distributed with this
4+
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
5+
*
6+
* Copyright Oxide Computer Company
7+
*/
8+
import { NumberInput } from './NumberInput'
9+
10+
export const Default = () => (
11+
<div className="max-w-lg">
12+
<NumberInput defaultValue={6} />
13+
</div>
14+
)
15+
16+
export const WithUnit = () => (
17+
<div className="max-w-lg">
18+
<NumberInput
19+
defaultValue={6}
20+
formatOptions={{
21+
style: 'unit',
22+
unit: 'inch',
23+
unitDisplay: 'long',
24+
}}
25+
/>
26+
</div>
27+
)
28+
29+
export const StepValues = () => (
30+
<div className="max-w-lg space-y-4 text-sans-md children:space-y-2">
31+
<div>
32+
<div>Step</div>
33+
<NumberInput step={10} />
34+
</div>
35+
<div>
36+
<div>Step + minValue</div>
37+
<NumberInput minValue={2} step={2} />
38+
</div>
39+
<div>
40+
<div>Step + minValue + maxValue</div>
41+
<NumberInput minValue={2} maxValue={20} step={2} />
42+
</div>
43+
</div>
44+
)

0 commit comments

Comments
 (0)