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.

Sign up for GitHub

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

Jump to bottom

instance create: focus image field on validation error #2364

Merged
merged 4 commits into from
Aug 13, 2024
Merged

instance create: focus image field on validation error #2364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6c97378
instance create: focus image field on validation error
david-crespo Aug 12, 2024
1b9feae
don't fix the type issue, but fix the diff
david-crespo Aug 12, 2024
bb0a85b
the ref doesn't need to be a ref ref. it can be a regular prop
david-crespo Aug 12, 2024
05d7740
Listbox -> ListboxField in floating IP attach modal
david-crespo Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/components/form/fields/ListboxField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export function ListboxField<
name={name}
hasError={fieldState.error !== undefined}
isLoading={isLoading}
buttonRef={field.ref}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I noticed while looking at this is that even after adding ref, we are not giving all the props from field to Listbox. We are still missing onBlur and disabled, both of which could become more UX-relevant in the future. For example, if we change a form from the RHF default of validating on submit to validating on blur, the lack of onBlur here will be a problem. Similarly, if we do #1895, I think the lack of disabled here will be a problem. We can fix it here, but I'm guessing there are a bunch more cases like this.

/>
<ErrorMessage error={fieldState.error} label={label} />
</div>
Expand Down
16 changes: 7 additions & 9 deletions app/pages/project/floating-ips/FloatingIpsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
import { IpGlobal16Icon, IpGlobal24Icon } from '@oxide/design-system/icons/react'

import { DocsPopover } from '~/components/DocsPopover'
import { ListboxField } from '~/components/form/fields/ListboxField'
import { HL } from '~/components/HL'
import { getProjectSelector, useProjectSelector } from '~/hooks'
import { confirmAction } from '~/stores/confirm-action'
Expand All @@ -34,7 +35,6 @@ import { Columns } from '~/table/columns/common'
import { PAGE_SIZE, useQueryTable } from '~/table/QueryTable'
import { CreateLink } from '~/ui/lib/CreateButton'
import { EmptyMessage } from '~/ui/lib/EmptyMessage'
import { Listbox } from '~/ui/lib/Listbox'
import { Message } from '~/ui/lib/Message'
import { Modal } from '~/ui/lib/Modal'
import { PageHeader, PageTitle } from '~/ui/lib/PageHeader'
Expand Down Expand Up @@ -268,6 +268,7 @@ const AttachFloatingIpModal = ({
},
})
const form = useForm({ defaultValues: { instanceId: '' } })
const instanceId = form.watch('instanceId')

return (
<Modal isOpen title="Attach floating IP" onDismiss={onDismiss}>
Expand All @@ -280,30 +281,27 @@ const AttachFloatingIpModal = ({
The selected instance will be reachable at <HL>{address}</HL>
</>
}
></Message>
/>
<form>
<Listbox
<ListboxField
control={form.control}
name="instanceId"
items={instances.map((i) => ({ value: i.id, label: i.name }))}
label="Instance"
onChange={(e) => {
form.setValue('instanceId', e)
}}
required
placeholder="Select an instance"
selected={form.watch('instanceId')}
/>
</form>
</Modal.Section>
</Modal.Body>
<Modal.Footer
actionText="Attach"
disabled={!form.getValues('instanceId')}
disabled={!instanceId}
onAction={() =>
floatingIpAttach.mutate({
path: { floatingIp },
query: { project },
body: { kind: 'instance', parent: form.getValues('instanceId') },
body: { kind: 'instance', parent: instanceId },
})
}
onDismiss={onDismiss}
Expand Down
6 changes: 5 additions & 1 deletion app/ui/lib/Listbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
ListboxOptions,
} from '@headlessui/react'
import cn from 'classnames'
import type { ReactNode } from 'react'
import { type ReactNode, type Ref } from 'react'

import { SelectArrows6Icon } from '@oxide/design-system/icons/react'

Expand Down Expand Up @@ -42,6 +42,8 @@ export interface ListboxProps<Value extends string = string> {
description?: React.ReactNode
required?: boolean
isLoading?: boolean
/** Necessary if you want RHF to be able to focus it on error */
buttonRef?: Ref<HTMLButtonElement>
}

export const Listbox = <Value extends string = string>({
Expand All @@ -59,6 +61,7 @@ export const Listbox = <Value extends string = string>({
required,
disabled,
isLoading = false,
buttonRef,
...props
}: ListboxProps<Value>) => {
const selectedItem = selected && items.find((i) => i.value === selected)
Expand Down Expand Up @@ -100,6 +103,7 @@ export const Listbox = <Value extends string = string>({
: 'bg-default',
isDisabled && hasError && '!border-error-secondary'
)}
ref={buttonRef}
{...props}
>
<div className="w-full overflow-hidden overflow-ellipsis whitespace-pre px-3 text-left">
Expand Down
Loading