Prototype: Stop instance disk modal #2747
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
app/forms/disk-attach.tsx
Outdated
| @@ -25,6 +26,7 @@ type AttachDiskProps = { | |||
| diskNamesToExclude?: string[] | |||
| loading?: boolean | |||
| submitError?: ApiError | null | |||
| instance: Instance | |||
There was a problem hiding this comment.
Requiring the instance isn't compatible with the use of this on the instance create form, but we can figure out how to deal with that.
|
I like it as an interaction. I simplified things a bit, which makes it less of a question of whether it's worth the complexity. |
david-crespo
force-pushed
the
stop-instance-disk-modal
branch
from
fc4d11b to
b14857b
Compare
david-crespo
force-pushed
the
stop-instance-disk-modal
branch
from
b14857b to
5c139d1
Compare
david-crespo
force-pushed
the
stop-instance-disk-modal
branch
from
5c139d1 to
2f72238
Compare
|
|
||
| const stopInstance = useApiMutation('instanceStop', { | ||
| onSuccess: () => { | ||
| // trigger polling by the top level InstancePage one |
There was a problem hiding this comment.
Oh this is very clever
There was a problem hiding this comment.
The only downside is that you have a delay in the loading state because it doesn't change to loading while the initial stop request goes through. I tried this
const isStoppingInstance = stopInstance.isPending || instance.runState === 'stopping'and it does respond immediately, but it flashes back to off briefly in between the stop request completing and the invalidation running and flipping the instance to 'stopping'. I can't think of a fix other than adding back in the explicit state thing you had. Those things make me nervous because the logic has to be bulletproof to avoid accidentally getting stuck in a loading state, but I'm sure we can do it.
| @@ -44,6 +44,10 @@ | |||
| @apply hidden; | |||
| } | |||
|
|
|||
| .btn-notice.ox-button:after { | |||
There was a problem hiding this comment.
I want to clean up button styles for these mini coloured ghost icons
| @@ -13,6 +13,11 @@ | |||
| animation: rotate 5s linear infinite; | |||
| } | |||
|
|
|||
| .spinner .path, | |||
There was a problem hiding this comment.
This is borrowed from #2742
Disabling buttons (with a tooltip) is a pattern I'm not particularly fond of – outside of the lesser accessibility, it adds friction in a situation we can help the user get what they want done.
Rough implementation of an idea I had around letting the user stop the instance inline within the disk attach modal. Can be abstracted to any similar situation that requires something be done first.
The polling to see if the instance state has change is less than ideal perhaps, and might require some more thinking.
Interested to hear what you think? Maybe the consensus is that the added friction to stop an instance is necessary (especially since we have a confirm modal elsewhere) because of the consequences of stopping a VM accidentally. And the user still needs to start the instance after attaching.