Instance action buttons (#2508)

* Instance action buttons * Re-add serial console link * Remove test styles * Fix broken `stopInstance` test util * be fussy * Button tooltip position default bottom (also flips to top) * Use correct info icon size * Add tooltip padding from edge of viewport * Removed disabled cursor for disabled button * Add confirm start instance * fix e2es * Only "running" instances can be stopped * `disabled:cursor-default` on button * use helper for symmetry --------- Co-authored-by: David Crespo <[email protected]>
oxidecomputer · Oct 30, 2024 · 2382425 · 2382425
1 parent 78e7e26
commit 2382425
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 43 deletions.
diff --git a/app/components/DocsPopover.tsx b/app/components/DocsPopover.tsx
@@ -9,7 +9,7 @@
 import { Popover, PopoverButton, PopoverPanel } from '@headlessui/react'
 import cn from 'classnames'
 
-import { OpenLink12Icon, Question12Icon } from '@oxide/design-system/icons/react'
+import { Info16Icon, OpenLink12Icon } from '@oxide/design-system/icons/react'
 
 import { buttonStyle } from '~/ui/lib/Button'
 
@@ -45,7 +45,7 @@ export const DocsPopover = ({ heading, icon, summary, links }: DocsPopoverProps)
   return (
     <Popover>
       <PopoverButton className={cn(buttonStyle({ size: 'sm', variant: 'ghost' }), 'w-8')}>
-        <Question12Icon aria-label="Links to docs" className="shrink-0" />
+        <Info16Icon aria-label="Links to docs" className="shrink-0" />
       </PopoverButton>
       <PopoverPanel
         // DocsPopoverPanel needed for enter animation

diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx
@@ -64,8 +64,7 @@ const POLL_INTERVAL_SLOW = 60 * sec
 
 export function InstancesPage() {
   const { project } = useProjectSelector()
-
-  const makeActions = useMakeInstanceActions(
+  const { makeButtonActions, makeMenuActions } = useMakeInstanceActions(
     { project },
     { onSuccess: refetchInstances, onDelete: refetchInstances }
   )
@@ -182,9 +181,12 @@ export function InstancesPage() {
         }
       ),
       colHelper.accessor('timeCreated', Columns.timeCreated),
-      getActionsCol(makeActions),
+      getActionsCol((instance: Instance) => [
+        ...makeButtonActions(instance),
+        ...makeMenuActions(instance),
+      ]),
     ],
-    [project, makeActions]
+    [project, makeButtonActions, makeMenuActions]
   )
 
   if (!instances) return null

diff --git a/app/pages/project/instances/actions.tsx b/app/pages/project/instances/actions.tsx
@@ -14,7 +14,6 @@ import { HL } from '~/components/HL'
 import { confirmAction } from '~/stores/confirm-action'
 import { confirmDelete } from '~/stores/confirm-delete'
 import { addToast } from '~/stores/toast'
-import type { MakeActions } from '~/table/columns/action-col'
 import { pb } from '~/util/path-builder'
 
 import { fancifyStates } from './instance/tabs/common'
@@ -31,40 +30,49 @@ type Options = {
 export const useMakeInstanceActions = (
   { project }: { project: string },
   options: Options = {}
-): MakeActions<Instance> => {
+) => {
   const navigate = useNavigate()
-
   // if you also pass onSuccess to mutate(), this one is not overridden — this
   // one runs first, then the one passed to mutate().
   //
   // We pull out the mutate functions because they are referentially stable,
   // while the whole useMutation result object is not. The async ones are used
   // when we need to confirm because the confirm modals want that.
   const opts = { onSuccess: options.onSuccess }
-  const { mutate: startInstance } = useApiMutation('instanceStart', opts)
+  const { mutateAsync: startInstanceAsync } = useApiMutation('instanceStart', opts)
   const { mutateAsync: stopInstanceAsync } = useApiMutation('instanceStop', opts)
   const { mutate: rebootInstance } = useApiMutation('instanceReboot', opts)
   // delete has its own
   const { mutateAsync: deleteInstanceAsync } = useApiMutation('instanceDelete', {
     onSuccess: options.onDelete,
   })
 
-  return useCallback(
-    (instance) => {
-      const instanceSelector = { project, instance: instance.name }
+  const makeButtonActions = useCallback(
+    (instance: Instance) => {
       const instanceParams = { path: { instance: instance.name }, query: { project } }
       return [
         {
           label: 'Start',
           onActivate() {
-            startInstance(instanceParams, {
-              onSuccess: () => addToast(<>Starting instance <HL>{instance.name}</HL></>), // prettier-ignore
-              onError: (error) =>
-                addToast({
-                  variant: 'error',
-                  title: `Error starting instance '${instance.name}'`,
-                  content: error.message,
+            confirmAction({
+              actionType: 'primary',
+              doAction: () =>
+                startInstanceAsync(instanceParams, {
+                  onSuccess: () => addToast(<>Starting instance <HL>{instance.name}</HL></>), // prettier-ignore
+                  onError: (error) =>
+                    addToast({
+                      variant: 'error',
+                      title: `Error starting instance '${instance.name}'`,
+                      content: error.message,
+                    }),
                 }),
+              modalTitle: 'Confirm start instance',
+              modalContent: (
+                <p>
+                  Are you sure you want to start <HL>{instance.name}</HL>?
+                </p>
+              ),
+              errorTitle: `Error starting ${instance.name}`,
             })
           },
           disabled: !instanceCan.start(instance) && (
@@ -97,9 +105,20 @@ export const useMakeInstanceActions = (
             })
           },
           disabled: !instanceCan.stop(instance) && (
-            <>Only {fancifyStates(instanceCan.stop.states)} instances can be stopped</>
+            // don't list all the states, it's overwhelming
+            <>Only {fancifyStates(['running'])} instances can be stopped</>
           ),
         },
+      ]
+    },
+    [project, startInstanceAsync, stopInstanceAsync]
+  )
+
+  const makeMenuActions = useCallback(
+    (instance: Instance) => {
+      const instanceSelector = { project, instance: instance.name }
+      const instanceParams = { path: { instance: instance.name }, query: { project } }
+      return [
         {
           label: 'Reboot',
           onActivate() {
@@ -143,13 +162,8 @@ export const useMakeInstanceActions = (
         },
       ]
     },
-    [
-      project,
-      navigate,
-      deleteInstanceAsync,
-      rebootInstance,
-      startInstance,
-      stopInstanceAsync,
-    ]
+    [project, deleteInstanceAsync, navigate, rebootInstance]
   )
+
+  return { makeButtonActions, makeMenuActions }
 }
diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx
@@ -26,6 +26,7 @@ import { RouteTabs, Tab } from '~/components/RouteTabs'
 import { InstanceStateBadge } from '~/components/StateBadge'
 import { getInstanceSelector, useInstanceSelector } from '~/hooks/use-params'
 import { EmptyCell } from '~/table/cells/EmptyCell'
+import { Button } from '~/ui/lib/Button'
 import { DateTime } from '~/ui/lib/DateTime'
 import { PageHeader, PageTitle } from '~/ui/lib/PageHeader'
 import { PropertiesTable } from '~/ui/lib/PropertiesTable'
@@ -92,7 +93,8 @@ export function InstancePage() {
   const instanceSelector = useInstanceSelector()
 
   const navigate = useNavigate()
-  const makeActions = useMakeInstanceActions(instanceSelector, {
+
+  const { makeButtonActions, makeMenuActions } = useMakeInstanceActions(instanceSelector, {
     onSuccess: refreshData,
     // go to project instances list since there's no more instance
     onDelete: () => {
@@ -132,17 +134,17 @@ export function InstancePage() {
     { enabled: !!primaryVpcId }
   )
 
-  const actions = useMemo(
+  const allMenuActions = useMemo(
     () => [
       {
         label: 'Copy ID',
         onActivate() {
           window.navigator.clipboard.writeText(instance.id || '')
         },
       },
-      ...makeActions(instance),
+      ...makeMenuActions(instance),
     ],
-    [instance, makeActions]
+    [instance, makeMenuActions]
   )
 
   const memory = filesize(instance.memory, { output: 'object', base: 2 })
@@ -152,9 +154,23 @@ export function InstancePage() {
       <PageHeader>
         <PageTitle icon={<Instances24Icon />}>{instance.name}</PageTitle>
         <div className="inline-flex gap-2">
-          <InstanceDocsPopover />
           <RefreshButton onClick={refreshData} />
-          <MoreActionsMenu label="Instance actions" actions={actions} />
+          <InstanceDocsPopover />
+          <div className="flex space-x-2 border-l pl-2 border-default">
+            {makeButtonActions(instance).map((action) => (
+              <Button
+                key={action.label}
+                variant="ghost"
+                size="sm"
+                onClick={action.onActivate}
+                disabled={!!action.disabled}
+                disabledReason={action.disabled}
+              >
+                {action.label}
+              </Button>
+            ))}
+          </div>
+          <MoreActionsMenu label="Instance actions" actions={allMenuActions} />
         </div>
       </PageHeader>
       <PropertiesTable.Group className="-mt-8 mb-16">

diff --git a/app/table/columns/action-col.tsx b/app/table/columns/action-col.tsx
@@ -16,7 +16,7 @@ import { Tooltip } from '~/ui/lib/Tooltip'
 import { Wrap } from '~/ui/util/wrap'
 import { kebabCase } from '~/util/str'
 
-export type MakeActions<Item> = (item: Item) => Array<MenuAction>
+type MakeActions<Item> = (item: Item) => Array<MenuAction>
 
 export type MenuAction = {
   label: string

diff --git a/app/ui/lib/Button.tsx b/app/ui/lib/Button.tsx
@@ -35,7 +35,7 @@ export const buttonStyle = ({
   variant = 'primary',
 }: ButtonStyleProps = {}) => {
   return cn(
-    'ox-button elevation-1 rounded inline-flex items-center justify-center align-top disabled:cursor-not-allowed shrink-0',
+    'ox-button elevation-1 rounded inline-flex items-center justify-center align-top disabled:cursor-default shrink-0',
     `btn-${variant}`,
     sizeStyle[size],
     variant === 'danger'
@@ -87,7 +87,7 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
     return (
       <Wrap
         when={isDisabled && disabledReason}
-        with={<Tooltip content={disabledReason} ref={ref} />}
+        with={<Tooltip content={disabledReason} ref={ref} placement="bottom" />}
       >
         <button
           className={cn(buttonStyle({ size, variant }), className, {

diff --git a/app/ui/lib/Tooltip.tsx b/app/ui/lib/Tooltip.tsx
@@ -13,6 +13,7 @@ import {
   FloatingArrow,
   FloatingPortal,
   offset,
+  shift,
   useDismiss,
   useFloating,
   useFocus,
@@ -68,6 +69,7 @@ export const Tooltip = forwardRef(
          */
         placement ? flip() : autoPlacement(),
         offset(12),
+        shift({ padding: 16 }),
         arrow({ element: arrowRef, padding: 12 }),
       ],
     })

diff --git a/package-lock.json b/package-lock.json
diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts
@@ -46,6 +46,7 @@ test('can start a failed instance', async ({ page }) => {
   // now start the failed one
   await expectInstanceState(page, 'you-fail', 'failed')
   await clickRowAction(page, 'you-fail', 'Start')
+  await page.getByRole('button', { name: 'Confirm' }).click()
   await expectInstanceState(page, 'you-fail', 'starting')
 })
 
@@ -93,6 +94,7 @@ test('can stop a starting instance, then start it again', async ({ page }) => {
   await expectInstanceState(page, 'not-there-yet', 'stopped')
 
   await clickRowAction(page, 'not-there-yet', 'Start')
+  await page.getByRole('button', { name: 'Confirm' }).click()
   await expectInstanceState(page, 'not-there-yet', 'starting')
   await expectInstanceState(page, 'not-there-yet', 'running')
 })

diff --git a/test/e2e/utils.ts b/test/e2e/utils.ts
@@ -107,8 +107,7 @@ export async function expectRowVisible(
 }
 
 export async function stopInstance(page: Page) {
-  await page.getByRole('button', { name: 'Instance actions' }).click()
-  await page.getByRole('menuitem', { name: 'Stop' }).click()
+  await page.getByRole('button', { name: 'Stop' }).click()
   await page.getByRole('button', { name: 'Confirm' }).click()
   await closeToast(page)
   // don't need to manually refresh because of polling