Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Number input #1582

Merged
merged 11 commits into from
Oct 5, 2023
8 changes: 4 additions & 4 deletions app/components/form/fields/DiskSizeField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
*
* Copyright Oxide Computer Company
*/
import type { FieldPath, FieldValues } from 'react-hook-form'
import type { FieldPath, FieldPathByValue, FieldValues } from 'react-hook-form'

import { MAX_DISK_SIZE_GiB } from '@oxide/api'

import { NumberField } from './NumberField'
import type { TextFieldProps } from './TextField'
import { TextField } from './TextField'

interface DiskSizeProps<
TFieldValues extends FieldValues,
Expand All @@ -21,10 +21,10 @@ interface DiskSizeProps<

export function DiskSizeField<
TFieldValues extends FieldValues,
TName extends FieldPath<TFieldValues>
TName extends FieldPathByValue<TFieldValues, number>
>({ required = true, name, minSize = 1, ...props }: DiskSizeProps<TFieldValues, TName>) {
return (
<TextField
<NumberField
units="GiB"
type="number"
required={required}
Expand Down
100 changes: 100 additions & 0 deletions app/components/form/fields/NumberField.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
*
* Copyright Oxide Computer Company
*/
import cn from 'classnames'
import { useId } from 'react'
import type { FieldPathByValue, FieldValues } from 'react-hook-form'
import { Controller } from 'react-hook-form'

import { FieldLabel, TextInputHint, NumberInput as UINumberField } from '@oxide/ui'
import { capitalize } from '@oxide/util'

import { ErrorMessage } from './ErrorMessage'
import type { TextFieldProps } from './TextField'

export function NumberField<
TFieldValues extends FieldValues,
// can only be used on fields with number values
TName extends FieldPathByValue<TFieldValues, number>
>({
name,
label = capitalize(name),
units,
description,
helpText,
required,
...props
}: Omit<TextFieldProps<TFieldValues, TName>, 'id'>) {
// id is omitted from props because we generate it here
const id = useId()
return (
<div className="max-w-lg">
<div className="mb-2">
<FieldLabel id={`${id}-label`} tip={description} optional={!required}>
{label} {units && <span className="ml-1 text-secondary">({units})</span>}
</FieldLabel>
{helpText && (
<TextInputHint id={`${id}-help-text`} className="mb-2">
{helpText}
</TextInputHint>
)}
</div>
{/* passing the generated id is very important for a11y */}
<NumberFieldInner name={name} {...props} id={id} />
</div>
)
}

/**
* Primarily exists for `NumberField`, but we occasionally also need a plain field
* 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
* from there to here. For the case where `TextFieldInner` is used
* independently, we also generate an ID for use only if none is passed in.
*/
export const NumberFieldInner = <
TFieldValues extends FieldValues,
TName extends FieldPathByValue<TFieldValues, number>
>({
name,
label = capitalize(name),
validate,
control,
description,
required,
id: idProp,
}: TextFieldProps<TFieldValues, TName>) => {
const generatedId = useId()
const id = idProp || generatedId

return (
<Controller
name={name}
control={control}
rules={{ required, validate }}
render={({ field: { value, ...fieldRest }, fieldState: { error } }) => {
return (
<>
<UINumberField
id={id}
error={!!error}
aria-labelledby={cn(`${id}-label`, {
[`${id}-help-text`]: !!description,
})}
aria-describedby={description ? `${id}-label-tip` : undefined}
defaultValue={value}
{...fieldRest}
/>
<ErrorMessage error={error} label={label} />
</>
)
}}
/>
)
}
2 changes: 1 addition & 1 deletion app/test/e2e/click-everything.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ test('Click through disks page', async ({ page }) => {
'role=textbox[name="Name"]',
'role=textbox[name="Description"]',
'role=radiogroup[name="Block size (Bytes)"]',
'role=spinbutton[name="Size (GiB)"]',
'role=textbox[name="Size (GiB)"]',
'role=button[name="Create Disk"]',
])
await page.goBack()
Expand Down
17 changes: 10 additions & 7 deletions app/test/e2e/instance-create.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ test('can create an instance', async ({ page }) => {
'role=textbox[name="Name"]',
'role=textbox[name="Description"]',
'role=textbox[name="Disk name"]',
'role=spinbutton[name="Disk size (GiB)"]',
'role=textbox[name="Disk size (GiB)"]',
'role=radiogroup[name="Network interface"]',
'role=textbox[name="Hostname"]',
'role=button[name="Create instance"]',
Expand All @@ -33,8 +33,9 @@ test('can create an instance', async ({ page }) => {
await page.fill('textarea[name=description]', 'An instance... from space!')
await page.locator('.ox-radio-card').nth(3).click()

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

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

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

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

// test disk size validation against image size
await page.getByRole('spinbutton', { name: 'Disk size (GiB)' }).fill('5')
await diskSizeInput.fill('5')
await diskSizeInput.blur() // need blur to trigger validation
await expectVisible(page, [
'main >> text=Must be as large as selected image (min. 6 GiB)',
])
await page.getByRole('spinbutton', { name: 'Disk size (GiB)' }).fill('10')
await diskSizeInput.fill('10')

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

Expand Down
2 changes: 1 addition & 1 deletion app/test/e2e/instance/attach-disk.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ test('Attach disk', async ({ page }) => {
'role=textbox[name="Name"]',
'role=textbox[name="Description"]',
'role=radiogroup[name="Block size (Bytes)"]',
'role=spinbutton[name="Size (GiB)"]',
'role=textbox[name="Size (GiB)"]',
'role=button[name="Create Disk"]',
])
await page.click('role=button[name="Cancel"]')
Expand Down
1 change: 1 addition & 0 deletions libs/ui/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export * from './lib/message/Message'
export * from './lib/modal/Modal'
export * from './lib/ModalLinks'
export * as MiniTable from './lib/mini-table/MiniTable'
export * from './lib/number-input/NumberInput'
export * from './lib/page-header/PageHeader'
export * from './lib/pagination/Pagination'
export * from './lib/progress/Progress'
Expand Down
6 changes: 3 additions & 3 deletions libs/ui/lib/date-picker/DatePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ export function DatePicker(props: DatePickerProps) {
className={cn(
state.isOpen && 'z-10 ring-2',
'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',
state.validationState === 'invalid'
state.isInvalid
? 'focus-error border-error ring-error-secondary'
: 'border-default ring-accent-secondary'
)}
>
<div className={cn('relative flex w-[10rem] items-center px-3 text-sans-md')}>
{label}
{state.validationState === 'invalid' && (
{state.isInvalid && (
<div className="absolute bottom-0 right-2 top-0 flex items-center text-error">
<Error12Icon className="h-3 w-3" />
</div>
Expand All @@ -84,7 +84,7 @@ export function DatePicker(props: DatePickerProps) {
</div>
</button>
</div>
{state.validationState === 'invalid' && (
{state.isInvalid && (
<p {...errorMessageProps} className="py-2 text-sans-md text-error">
Date is invalid
</p>
Expand Down
25 changes: 14 additions & 11 deletions libs/ui/lib/date-picker/DateRangePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,17 @@ export function DateRangePicker(props: DateRangePickerProps) {
hourCycle: 'h24',
})

const label = useMemo(
() =>
formatter.formatRange(
state.dateRange.start.toDate(getLocalTimeZone()),
state.dateRange.end.toDate(getLocalTimeZone())
),
[state, formatter]
)
const label = useMemo(() => {
// This is here to make TS happy. This should be impossible in practice
// because we always pass a value to this component and there is no way to
// unset the value through the UI.
if (!state.dateRange) return 'No range selected'

return formatter.formatRange(
state.dateRange.start.toDate(getLocalTimeZone()),
state.dateRange.end.toDate(getLocalTimeZone())
)
}, [state.dateRange, formatter])

return (
<div
Expand All @@ -62,14 +65,14 @@ export function DateRangePicker(props: DateRangePickerProps) {
className={cn(
state.isOpen && 'z-10 ring-2',
'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',
state.validationState === 'invalid'
state.isInvalid
? 'focus-error border-error ring-error-secondary'
: 'border-default ring-accent-secondary'
)}
>
<div className={cn('relative flex w-[16rem] items-center px-3 text-sans-md')}>
{label}
{state.validationState === 'invalid' && (
{state.isInvalid && (
<div className="absolute bottom-0 right-2 top-0 flex items-center text-error">
<Error12Icon className="h-3 w-3" />
</div>
Expand All @@ -80,7 +83,7 @@ export function DateRangePicker(props: DateRangePickerProps) {
</div>
</button>
</div>
{state.validationState === 'invalid' && (
{state.isInvalid && (
<p {...errorMessageProps} className="py-2 text-sans-md text-error">
Date range is invalid
</p>
Expand Down
44 changes: 44 additions & 0 deletions libs/ui/lib/number-input/NumberInput.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
*
* Copyright Oxide Computer Company
*/
import { NumberInput } from './NumberInput'
benjaminleonard marked this conversation as resolved.
Show resolved Hide resolved

export const Default = () => (
<div className="max-w-lg">
<NumberInput defaultValue={6} />
</div>
)

export const WithUnit = () => (
<div className="max-w-lg">
<NumberInput
defaultValue={6}
formatOptions={{
style: 'unit',
unit: 'inch',
unitDisplay: 'long',
}}
/>
</div>
)

export const StepValues = () => (
<div className="max-w-lg space-y-4 text-sans-md children:space-y-2">
<div>
<div>Step</div>
<NumberInput step={10} />
</div>
<div>
<div>Step + minValue</div>
<NumberInput minValue={2} step={2} />
</div>
<div>
<div>Step + minValue + maxValue</div>
<NumberInput minValue={2} maxValue={20} step={2} />
</div>
</div>
)
Loading
Loading