feat(desktop): let ConfirmDialog carry a secondary action

The worktree removal prompt offers a third way out — hide the lane but leave the worktree on disk — which is why it was still hand-rolled. One optional slot between Cancel and Confirm covers it, and it keeps Confirm as the focused button so Enter still means the destructive action.
2026-08-19 23:13:17 -05:00
parent 1e26c02de6
commit 5df9cd27ea
3 changed files with 94 additions and 47 deletions
@@ -2,16 +2,8 @@ import { useStore } from '@nanostores/react'
 import type * as React from 'react'
 import { useMemo, useState } from 'react'

-import { Button } from '@/components/ui/button'
 import { Codicon } from '@/components/ui/codicon'
-import {
-  Dialog,
-  DialogContent,
-  DialogDescription,
-  DialogFooter,
-  DialogHeader,
-  DialogTitle
-} from '@/components/ui/dialog'
+import { ConfirmDialog } from '@/components/ui/confirm-dialog'
 import type { HermesGitWorktree } from '@/global'
 import type { SessionInfo } from '@/hermes'
 import { useI18n } from '@/i18n'
@@ -190,43 +182,26 @@ function RepoFlatSection({
    destructiveLabel: string,
    onDestructive: (group: SidebarSessionGroup) => void
  ) => (
-    <Dialog onOpenChange={isOpen => !isOpen && setTarget(null)} open={Boolean(target)}>
-      <DialogContent>
-        <DialogHeader>
-          <DialogTitle>{`${s.projects.removeWorktree} "${target?.label ?? ''}"?`}</DialogTitle>
-          <DialogDescription>{description}</DialogDescription>
-        </DialogHeader>
-        <DialogFooter>
-          <Button onClick={() => setTarget(null)} variant="ghost">
-            {t.common.cancel}
-          </Button>
-          <Button
-            onClick={() => {
-              if (target) {
-                dismissWorktree(target.id)
-              }
-
-              setTarget(null)
-            }}
-            variant="secondary"
-          >
-            {s.projects.removeFromSidebar}
-          </Button>
-          <Button
-            onClick={() => {
-              setTarget(null)
-
-              if (target) {
-                onDestructive(target)
-              }
-            }}
-            variant="destructive"
-          >
-            {destructiveLabel}
-          </Button>
-        </DialogFooter>
-      </DialogContent>
-    </Dialog>
+    <ConfirmDialog
+      confirmLabel={destructiveLabel}
+      description={description}
+      destructive
+      // removeViaGit finishes on its own: it either dismisses the lane, escalates
+      // to the force prompt, or toasts. Nothing left for the dialog to wait on.
+      dismissOnConfirm
+      onClose={() => setTarget(null)}
+      onConfirm={() => {
+        if (target) {
+          onDestructive(target)
+        }
+      }}
+      open={Boolean(target)}
+      secondaryAction={{
+        label: s.projects.removeFromSidebar,
+        onClick: () => target && dismissWorktree(target.id)
+      }}
+      title={`${s.projects.removeWorktree} "${target?.label ?? ''}"?`}
+    />
  )

  const removeDialog = (
@@ -0,0 +1,50 @@
+import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+
+import { ConfirmDialog } from './confirm-dialog'
+
+afterEach(cleanup)
+
+describe('ConfirmDialog secondary action', () => {
+  function renderWithSecondary() {
+    const onConfirm = vi.fn()
+    const onClose = vi.fn()
+    const onSecondary = vi.fn()
+
+    render(
+      <ConfirmDialog
+        onClose={onClose}
+        onConfirm={onConfirm}
+        open
+        secondaryAction={{ label: 'Remove from sidebar', onClick: onSecondary }}
+        title="Remove worktree?"
+      />
+    )
+
+    return { onClose, onConfirm, onSecondary }
+  }
+
+  it('runs the secondary action and closes without confirming', async () => {
+    const { onClose, onConfirm, onSecondary } = renderWithSecondary()
+
+    fireEvent.click(await screen.findByRole('button', { name: 'Remove from sidebar' }))
+
+    expect(onSecondary).toHaveBeenCalledTimes(1)
+    expect(onClose).toHaveBeenCalledTimes(1)
+    expect(onConfirm).not.toHaveBeenCalled()
+  })
+
+  it('still opens focused on Confirm, so Enter confirms rather than picking the secondary', async () => {
+    const { onConfirm, onSecondary } = renderWithSecondary()
+
+    const dialog = await screen.findByRole('dialog')
+
+    // eslint-disable-next-line no-restricted-globals -- asserting real focus requires the live document
+    await waitFor(() => expect(dialog.contains(document.activeElement)).toBe(true))
+    // eslint-disable-next-line no-restricted-globals -- asserting real focus requires the live document
+    fireEvent.keyDown(document.activeElement!, { key: 'Enter' })
+
+    await waitFor(() => expect(onConfirm).toHaveBeenCalledTimes(1))
+    expect(onSecondary).not.toHaveBeenCalled()
+  })
+})
@@ -28,6 +28,14 @@ interface ConfirmDialogProps {
  destructive?: boolean
  /** Close as soon as onConfirm resolves — for optimistic actions that finish in the background. */
  dismissOnConfirm?: boolean
+  /** A third, non-destructive way out, shown between Cancel and Confirm (e.g.
+   *  "Remove from sidebar" beside "Delete worktree"). Closes on click. */
+  secondaryAction?: ConfirmSecondaryAction
+}
+
+interface ConfirmSecondaryAction {
+  label: string
+  onClick: () => void
 }

 // Shared confirmation dialog: opens focused on Confirm, Enter confirms (from
@@ -45,7 +53,8 @@ export function ConfirmDialog({
  doneLabel,
  cancelLabel,
  destructive = false,
-  dismissOnConfirm = false
+  dismissOnConfirm = false,
+  secondaryAction
 }: ConfirmDialogProps) {
  const { t } = useI18n()
  const confirmRef = useRef<HTMLButtonElement>(null)
@@ -131,6 +140,19 @@ export function ConfirmDialog({
          <Button disabled={busy} onClick={onClose} type="button" variant="ghost">
            {resolvedCancelLabel}
          </Button>
+          {secondaryAction && (
+            <Button
+              disabled={busy}
+              onClick={() => {
+                secondaryAction.onClick()
+                onClose()
+              }}
+              type="button"
+              variant="secondary"
+            >
+              {secondaryAction.label}
+            </Button>
+          )}
          <Button
            disabled={busy}
            onClick={() => void run()}