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

Prototype: Stop instance disk modal #2747

Draft
wants to merge 6 commits into
base: attach-disk-modal
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
64 changes: 64 additions & 0 deletions app/components/StopInstancePrompt.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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 { type ReactNode } from 'react'

import { apiQueryClient, useApiMutation, type Instance } from '@oxide/api'

import { HL } from '~/components/HL'
import { addToast } from '~/stores/toast'
import { Button } from '~/ui/lib/Button'
import { Message } from '~/ui/lib/Message'

type StopInstancePromptProps = {
instance: Instance
children: ReactNode
}

export function StopInstancePrompt({ instance, children }: StopInstancePromptProps) {
const isStoppingInstance = instance.runState === 'stopping'

const stopInstance = useApiMutation('instanceStop', {
onSuccess: () => {
// trigger polling by the top level InstancePage one
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh this is very clever

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

apiQueryClient.invalidateQueries('instanceView')
addToast(<>Stopping instance <HL>{instance.name}</HL></>) // prettier-ignore
},
onError: (error) => {
addToast({
variant: 'error',
title: `Error stopping instance '${instance.name}'`,
content: error.message,
})
},
})

return (
<Message
variant="notice"
content={
<>
{children}{' '}
<Button
size="xs"
className="mt-3"
variant="notice"
onClick={() =>
stopInstance.mutateAsync({
path: { instance: instance.name },
query: { project: instance.projectId },
})
}
loading={isStoppingInstance}
>
Stop instance
</Button>
</>
}
/>
)
}
15 changes: 14 additions & 1 deletion app/forms/disk-attach.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
import { useMemo } from 'react'
import { useForm } from 'react-hook-form'

import { useApiQuery, type ApiError } from '@oxide/api'
import { instanceCan, useApiQuery, type ApiError, type Instance } from '@oxide/api'

import { ComboboxField } from '~/components/form/fields/ComboboxField'
import { ModalForm } from '~/components/form/ModalForm'
import { StopInstancePrompt } from '~/components/StopInstancePrompt'
import { useProjectSelector } from '~/hooks/use-params'
import { toComboboxItems } from '~/ui/lib/Combobox'
import { ALL_ISH } from '~/util/consts'
Expand All @@ -25,6 +26,7 @@ type AttachDiskProps = {
diskNamesToExclude?: string[]
loading?: boolean
submitError?: ApiError | null
instance?: Instance
}

/**
Expand All @@ -37,6 +39,7 @@ export function AttachDiskModalForm({
diskNamesToExclude = [],
loading = false,
submitError = null,
instance,
}: AttachDiskProps) {
const { project } = useProjectSelector()

Expand Down Expand Up @@ -66,7 +69,17 @@ export function AttachDiskModalForm({
title="Attach disk"
onSubmit={onSubmit}
width="medium"
submitDisabled={
instance && !instanceCan.attachDisk(instance)
? 'Instance must be stopped'
: undefined
}
>
{instance && ['stopping', 'running'].includes(instance.runState) && (
<StopInstancePrompt instance={instance}>
An instance must be stopped to attach a disk.
</StopInstancePrompt>
)}
<ComboboxField
label="Disk name"
placeholder="Select a disk"
Expand Down
14 changes: 2 additions & 12 deletions app/pages/project/instances/StorageTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -334,18 +334,7 @@ export default function StorageTab() {

<CardBlock>
<CardBlock.Header title="Additional disks" titleId="other-disks-label">
<Button
variant="secondary"
size="sm"
onClick={() => setShowDiskAttach(true)}
disabledReason={
<>
Instance must be <span className="text-raise">stopped</span> to attach a
disk
</>
}
disabled={!instanceCan.attachDisk(instance)}
>
<Button variant="secondary" size="sm" onClick={() => setShowDiskAttach(true)}>
Attach existing disk
</Button>
<Button
Expand Down Expand Up @@ -393,6 +382,7 @@ export default function StorageTab() {
}}
loading={attachDisk.isPending}
submitError={attachDisk.error}
instance={instance}
/>
)}
</div>
Expand Down
7 changes: 4 additions & 3 deletions app/ui/lib/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@
import { Tooltip } from '~/ui/lib/Tooltip'
import { Wrap } from '~/ui/util/wrap'

export const buttonSizes = ['sm', 'icon', 'base'] as const
export const variants = ['primary', 'secondary', 'ghost', 'danger'] as const
export const buttonSizes = ['xs', 'sm', 'icon', 'base'] as const
export const variants = ['primary', 'secondary', 'ghost', 'danger', 'notice'] as const

export type ButtonSize = (typeof buttonSizes)[number]
export type Variant = (typeof variants)[number]

const sizeStyle: Record<ButtonSize, string> = {
xs: 'h-6 px-2 text-mono-xs',
sm: 'h-8 px-3 text-mono-sm [&>svg]:w-4',
// meant for buttons that only contain a single icon
icon: 'h-8 w-8 text-mono-sm [&>svg]:w-4',
Expand Down Expand Up @@ -115,9 +116,9 @@
animate={{ opacity: 1, y: '-50%', x: '-50%' }}
initial={{ opacity: 0, y: 'calc(-50% - 25px)', x: '-50%' }}
transition={{ type: 'spring', duration: 0.3, bounce: 0 }}
className="absolute left-1/2 top-1/2"
className="absolute left-1/2 top-1/2 flex items-center justify-center"
>
<Spinner variant={variant} />

Check failure on line 121 in app/ui/lib/Button.tsx

View workflow job for this annotation

GitHub Actions / ci

Type '"primary" | "danger" | "notice" | "secondary" | "ghost" | undefined' is not assignable to type '"primary" | "danger" | "secondary" | "ghost" | undefined'.
</m.span>
)}
<m.span
Expand Down
4 changes: 4 additions & 0 deletions app/ui/styles/components/button.css
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
@apply hidden;
}

.btn-notice.ox-button:after {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to clean up button styles for these mini coloured ghost icons

@apply opacity-40;
}

/**
* A class to make it very visually obvious that a button style is missing
*/
Expand Down
24 changes: 5 additions & 19 deletions app/ui/styles/components/spinner.css
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
animation: rotate 5s linear infinite;
}

.spinner .path,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is borrowed from #2742

.spinner .bg {
stroke: currentColor;
}

.spinner.spinner-md {
--radius: 8;
--circumference: calc(var(--PI) * var(--radius) * 2px);
Expand All @@ -27,7 +32,6 @@
stroke-dasharray: var(--circumference);
transform-origin: center;
animation: dash 8s ease-in-out infinite;
stroke: var(--content-accent-tertiary);
}

@media (prefers-reduced-motion) {
Expand All @@ -50,24 +54,6 @@
}
}

.spinner-ghost .bg,
.spinner-secondary .bg {
stroke: var(--content-default);
}

.spinner-secondary .path {
stroke: var(--content-secondary);
}

.spinner-primary .bg {
stroke: var(--content-accent);
}

.spinner-danger .bg,
.spinner-danger .path {
stroke: var(--content-destructive-tertiary);
}

@keyframes rotate {
100% {
transform: rotate(360deg);
Expand Down
Loading