UI fixes

ui/e2e/smoke.spec.ts

@@ -147,8 +147,11 @@ test('editor validates, saves into a new folder, and deletes', async ({
   await expect(
     page.getByRole('heading', { name: 'e2e-scratch/E2EScratch' }),
   ).toBeVisible()
-  page.once('dialog', (dialog) => dialog.accept())
   await page.getByRole('button', { name: /delete this workflow/ }).click()
+  await page
+    .getByRole('alertdialog')
+    .getByRole('button', { name: 'Delete', exact: true })
+    .click()
   await expect(page.getByRole('heading', { name: 'Workflows' })).toBeVisible()
   await expect(page.getByRole('link', { name: /E2EScratch/ })).toHaveCount(0)
 })
@@ -193,8 +196,11 @@ test('prompts page lists, creates at the root, and deletes', async ({
     'an e2e scratch prompt',
     { timeout: 15_000 },
   )
-  page.once('dialog', (dialog) => dialog.accept())
   await page.getByRole('button', { name: /Delete/ }).click()
+  await page
+    .getByRole('alertdialog')
+    .getByRole('button', { name: 'Delete', exact: true })
+    .click()
   await expect(page.getByRole('heading', { name: 'Prompts' })).toBeVisible()
   await expect(
     page.getByRole('link', { name: 'E2EScratchPrompt' }),

ui/src/App.svelte

@@ -22,6 +22,7 @@
   import KeyboardHelp from './lib/KeyboardHelp.svelte'
   import StatusPopover from './lib/StatusPopover.svelte'
   import TokenPopover from './lib/TokenPopover.svelte'
+  import ConfirmDialog from './lib/ConfirmDialog.svelte'
   import WorkflowsPage from './lib/pages/WorkflowsPage.svelte'
   import WorkflowPage from './lib/pages/WorkflowPage.svelte'
   import JobsPage from './lib/pages/JobsPage.svelte'
@@ -298,6 +299,7 @@
 </header>
 
 <Toaster position="bottom-right" closeButton {theme} duration={4000} />
+<ConfirmDialog />
 
 <KeyboardHelp bind:open={helpOpen} />
 

ui/src/lib/ConfirmDialog.svelte

@@ -0,0 +1,63 @@
+<script lang="ts">
+  import { confirmState, resolveConfirm } from './confirm.svelte'
+  import { focusTrap } from './focusTrap'
+
+  function onKeydown(event: KeyboardEvent) {
+    if (confirmState.open && event.key === 'Escape') {
+      event.preventDefault()
+      resolveConfirm(false)
+    }
+  }
+</script>
+
+<svelte:window onkeydown={onKeydown} />
+
+{#if confirmState.open}
+  <!-- svelte-ignore a11y_click_events_have_key_events -->
+  <div class="scrim" role="presentation" onclick={() => resolveConfirm(false)}>
+    <div
+      class="sheet panel"
+      role="alertdialog"
+      aria-modal="true"
+      aria-label="confirm"
+      tabindex="-1"
+      use:focusTrap
+      onclick={(e) => e.stopPropagation()}
+    >
+      <p>{confirmState.message}</p>
+      <div class="actions">
+        <button class="quiet" onclick={() => resolveConfirm(false)}
+          >{confirmState.cancelLabel}</button
+        >
+        <button onclick={() => resolveConfirm(true)}
+          >{confirmState.confirmLabel}</button
+        >
+      </div>
+    </div>
+  </div>
+{/if}
+
+<style>
+  .scrim {
+    position: fixed;
+    inset: 0;
+    background: color-mix(in srgb, var(--bg) 65%, transparent);
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    z-index: 60;
+  }
+  .sheet {
+    min-width: min(360px, 92vw);
+    max-width: 440px;
+  }
+  p {
+    margin: 0 0 var(--space-4);
+    white-space: pre-line;
+  }
+  .actions {
+    display: flex;
+    justify-content: flex-end;
+    gap: var(--space-2);
+  }
+</style>

ui/src/lib/confirm.svelte.ts

@@ -0,0 +1,42 @@
+// A window.confirm replacement that renders through the app's own modal
+// styling instead of the browser chrome. One state object plus a single
+// <ConfirmDialog> mounted in App.svelte, mirroring the notify()/Toaster
+// pair in toast.ts - callers just `await confirmDialog(...)` in place of
+// `window.confirm(...)`.
+
+type ConfirmState = {
+  open: boolean
+  message: string
+  confirmLabel: string
+  cancelLabel: string
+}
+
+export const confirmState = $state<ConfirmState>({
+  open: false,
+  message: '',
+  confirmLabel: 'OK',
+  cancelLabel: 'Cancel',
+})
+
+// At most one confirm is ever in flight - a second call while one is open
+// would have no dialog to land in anyway, so it simply replaces the first.
+let resolver: ((value: boolean) => void) | null = null
+
+export function confirmDialog(
+  message: string,
+  options?: { confirmLabel?: string; cancelLabel?: string },
+): Promise<boolean> {
+  confirmState.open = true
+  confirmState.message = message
+  confirmState.confirmLabel = options?.confirmLabel ?? 'OK'
+  confirmState.cancelLabel = options?.cancelLabel ?? 'Cancel'
+  return new Promise((resolve) => {
+    resolver = resolve
+  })
+}
+
+export function resolveConfirm(value: boolean): void {
+  confirmState.open = false
+  resolver?.(value)
+  resolver = null
+}

ui/src/lib/pages/EditorPage.svelte

@@ -15,6 +15,7 @@
   } from '@lucide/svelte'
   import { api } from '../api'
   import { notify } from '../toast'
+  import { confirmDialog } from '../confirm.svelte'
   import { go } from '../router.svelte'
   import {
     emptyWorkflow,
@@ -132,10 +133,16 @@
     '#/workflows/' + name.split('/').map(encodeURIComponent).join('/'),
   )
 
-  // Leaving with unsaved edits used to drop them without a word
-  function confirmLeave(event: MouseEvent) {
-    if (dirty && !window.confirm('Discard unsaved changes?'))
-      event.preventDefault()
+  // Leaving with unsaved edits used to drop them without a word. The
+  // confirm is async, so the default navigation is always prevented first
+  // and replayed by hand once the answer comes back.
+  async function confirmLeave(event: MouseEvent) {
+    if (!dirty) return
+    event.preventDefault()
+    const target = (event.currentTarget as HTMLAnchorElement).href
+    if (await confirmDialog('Discard unsaved changes?')) {
+      window.location.href = target
+    }
   }
 
   const savePreview = $derived.by(() => {

ui/src/lib/pages/GalleryPage.svelte

@@ -14,6 +14,7 @@
   import { go } from '../router.svelte'
   import { SvelteSet } from 'svelte/reactivity'
   import { notify } from '../toast'
+  import { confirmDialog } from '../confirm.svelte'
   import type { GalleryFile } from '../types'
   import WorkspacePicker from '../WorkspacePicker.svelte'
   import { workspace } from '../workspace.svelte'
@@ -29,6 +30,7 @@
   let anchor = $state<string | null>(null)
   let busy = $state(false)
   let metadata = $state<Record<string, unknown> | null>(null)
+  let metadataLoading = $state(false)
   let sourceJob = $state<{ id: string; status: string } | null>(null)
 
   $effect(() => {
@@ -64,7 +66,9 @@
       // choice to replace belongs to the person, not the button
       if (
         message.includes('already exists') &&
-        window.confirm(`${message}\n\nReplace it?`)
+        (await confirmDialog(`${message}\n\nReplace it?`, {
+          confirmLabel: 'Replace',
+        }))
       ) {
         try {
           const result = await api.keepOutput(selected.name, assetName, true)
@@ -145,9 +149,10 @@
   async function removePicked() {
     const names = pickedNames
     if (
-      !window.confirm(
+      !(await confirmDialog(
         `Delete ${names.length} file${names.length === 1 ? '' : 's'}? This removes them on disk.`,
-      )
+        { confirmLabel: 'Delete' },
+      ))
     )
       return
     busy = true
@@ -179,19 +184,29 @@
   function select(file: GalleryFile) {
     selected = file
     metadata = null
+    metadataLoading = true
     sourceJob = null
-    api.galleryMetadata(file.name).then((r) => {
-      if (selected?.name === file.name) {
-        metadata = r.metadata
-        sourceJob = r.job
-      }
-    })
+    api
+      .galleryMetadata(file.name)
+      .then((r) => {
+        if (selected?.name === file.name) {
+          metadata = r.metadata
+          sourceJob = r.job
+        }
+      })
+      .catch(() => {})
+      .finally(() => {
+        if (selected?.name === file.name) metadataLoading = false
+      })
   }
 
   async function removeFile() {
     if (!selected) return
     if (
-      !window.confirm(`Delete ${selected.name}? This removes the file on disk.`)
+      !(await confirmDialog(
+        `Delete ${selected.name}? This removes the file on disk.`,
+        { confirmLabel: 'Delete' },
+      ))
     )
       return
     const name = selected.name
@@ -346,7 +361,6 @@
   <div class="detail panel">
     <div class="bar">
       <strong class="selname">{selected.name}</strong>
-      <span class="num muted">{mb(selected.size)} · {day(selected.mtime)}</span>
       <span class="flex"></span>
       {#if embeddedWorkflow}
         <button
@@ -370,6 +384,7 @@
         class="muted"
         title="open the file itself in a new tab">open file</a
       >
+      <span class="num muted">{mb(selected.size)} · {day(selected.mtime)}</span>
       <DownloadLink href={api.outputDownloadUrl(selected.name)} />
       <button
         class="quiet icon danger"
@@ -379,6 +394,7 @@
       >
         <Trash2 size={14} />
       </button>
+      <span class="flex"></span>
       <button
         class="quiet icon"
         onclick={() => (selected = null)}
@@ -441,8 +457,12 @@
             </div>
           {/if}
         </div>
-      {:else if selected.kind === 'image'}
+      {:else if selected.kind === 'image' && metadataLoading}
         <div class="meta muted">reading metadata…</div>
+      {:else if selected.kind === 'image'}
+        <div class="meta muted">
+          no embedded metadata - enable embed_metadata in the step's result
+        </div>
       {/if}
     </div>
   </div>

ui/src/lib/pages/GalleryPage.test.ts

@@ -1,9 +1,16 @@
-import { cleanup, render, screen, waitFor } from '@testing-library/svelte'
+import {
+  cleanup,
+  render,
+  screen,
+  waitFor,
+  within,
+} from '@testing-library/svelte'
 import { afterEach, beforeEach, expect, it, vi } from 'vitest'
 // Hoisted above the imports so the static import of the component below -
 // itself hoisted - sees an initialized mock. Importing the component inside
 // the test instead would charge its (multi-second) compile to the test timeout
 import GalleryPage from './GalleryPage.svelte'
+import ConfirmDialog from '../ConfirmDialog.svelte'
 import type { GalleryFile } from '../types'
 
 const file = (name: string): GalleryFile => ({
@@ -68,12 +75,26 @@ const checkbox = (name: string) =>
   screen.getByRole('checkbox', { name: `select ${name}` })
 
 async function renderGallery(first = 'a.png') {
+  // ConfirmDialog is normally mounted once in App.svelte and driven through
+  // the shared confirm.svelte.ts state - render it alongside so a test can
+  // answer the dialogs GalleryPage's delete/replace flows open.
+  render(ConfirmDialog)
   render(GalleryPage)
   await waitFor(() =>
     expect(screen.getByLabelText(`select ${first}`)).toBeTruthy(),
   )
 }
 
+/** Answers the confirm dialog opened by a delete/replace action - scoped to
+ * the dialog itself, since its "Delete" button shares a name with whatever
+ * trigger button opened it. */
+async function answerConfirm(accept: boolean) {
+  const dialog = await waitFor(() => screen.getByRole('alertdialog'))
+  within(dialog)
+    .getByRole('button', { name: accept ? /^delete$/i : /^cancel$/i })
+    .click()
+}
+
 it('fetches the gallery listing exactly once on mount', async () => {
   render(GalleryPage)
   // Let the request settle and any (wrongly) re-triggered effects run
@@ -130,13 +151,13 @@ it('archives every selected file in one request', async () => {
 })
 
 it('drops deleted files from the grid and the selection', async () => {
-  vi.stubGlobal('confirm', () => true)
   await renderGallery()
 
   checkbox('a.png').click()
   checkbox('b.png').click()
   await waitFor(() => expect(screen.getByText('2 selected')).toBeTruthy())
   screen.getByRole('button', { name: /^delete/i }).click()
+  await answerConfirm(true)
 
   await waitFor(() =>
     expect(screen.queryByLabelText('select a.png')).toBeNull(),
@@ -147,7 +168,6 @@ it('drops deleted files from the grid and the selection', async () => {
 })
 
 it('keeps a file that failed to delete selected and reports it', async () => {
-  vi.stubGlobal('confirm', () => true)
   deleteOutput.mockImplementation((name: string) =>
     name === 'b.png' ? Promise.reject(new Error('busy')) : Promise.resolve(),
   )
@@ -157,6 +177,7 @@ it('keeps a file that failed to delete selected and reports it', async () => {
   checkbox('b.png').click()
   await waitFor(() => expect(screen.getByText('2 selected')).toBeTruthy())
   screen.getByRole('button', { name: /^delete/i }).click()
+  await answerConfirm(true)
 
   await waitFor(() => expect(screen.getByText('1 selected')).toBeTruthy())
   expect(screen.queryByLabelText('select a.png')).toBeNull()
@@ -165,19 +186,18 @@ it('keeps a file that failed to delete selected and reports it', async () => {
 })
 
 it('does not delete anything when the confirmation is declined', async () => {
-  vi.stubGlobal('confirm', () => false)
   await renderGallery()
 
   checkbox('a.png').click()
   await waitFor(() => expect(screen.getByText('1 selected')).toBeTruthy())
   screen.getByRole('button', { name: /^delete/i }).click()
+  await answerConfirm(false)
 
   await new Promise((r) => setTimeout(r, 10))
   expect(deleteOutput).not.toHaveBeenCalled()
 })
 
 it('drops a file deleted from the detail panel out of the selection', async () => {
-  vi.stubGlobal('confirm', () => true)
   await renderGallery()
 
   checkbox('a.png').click()
@@ -196,6 +216,7 @@ it('drops a file deleted from the detail panel out of the selection', async () =
   screen
     .getByRole('button', { name: 'delete this file from the output directory' })
     .click()
+  await answerConfirm(true)
 
   await waitFor(() => expect(screen.getByText('1 selected')).toBeTruthy())
   expect(screen.getByLabelText('select b.png')).toBeTruthy()