Convert instance form #760
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/oxidecomputer/console-ui-storybook/FkAYrTSBXQJ9TKueGtosRqukREWH |
|
Preview will be deployed at https://console-git-update-instance-page.internal.oxide.computer |
zephraph
force-pushed
the
update-instance-page
branch
from
34f95f3 to
351f659
Compare
zephraph
force-pushed
the
update-instance-page
branch
from
ef6c020 to
914efff
Compare
* 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
| const attachDisk = useApiMutation('instanceDisksAttach', { | ||
| onSuccess(data) { | ||
| const { instanceName, ...others } = pathParams | ||
| invariant(instanceName, 'instanceName is required') |
There was a problem hiding this comment.
what does it mean to require instanceName at submit time but not at render time?
| PrebuiltFormProps<infer V, any> | ||
| > | ||
| ? V | ||
| : never |
There was a problem hiding this comment.
These types are complicated enough that it suggests to me we might be doing something we don't need to be doing, but I'm fine leaving it and mulling that over as we use it
There was a problem hiding this comment.
Let's go! Just a couple more comments here and there, no dealbreakers.
I could be wrong but I don't think this closes #730. If so, remove that from the description before merging so it doesn't get auto-closed. Also I'm not sure, but I think each issue needs its own "fixes" for the magic to work. Maybe not though.
| <TextField | ||
| id="hostname" | ||
| description="Will be generated if not provided" | ||
| /> |
There was a problem hiding this comment.
Since this is still not doing anything on the API side, it might make more sense to leave this field out for now and just pass the name value for hostname in the POST.
There was a problem hiding this comment.
Given it's already in the API (even if not wired up yet) I think we should keep it.
There was a problem hiding this comment.
K. I just put the issue for fixing this on the API side on our board. oxidecomputer/omicron#68
Fixes #734, #733, #732, #730, #331, #308
Converts the instance create form to the new form structure, implements a lot of missing UI, and wires up most of the form to the API. This is actually a rather large task and I won't be able to finish everything up in this one PR. I'll make issues for things that are deferred.