File viewer: copy dialog UX and select-all sizing fix

vdavid · vdavid · commit b6542e7b2457 · 2026-05-22T23:40:43.000+02:00
- `isWholeFileSelection` short-circuits the per-line byte estimator so ⌘A on a
  multi-MB file routes to the right size tier. The line cache only contains
  scrolled lines, so the old walk bailed to null on ⌘A and the page fell into
  the unknown-size confirm dialog instead of the proper refuse dialog for
  &gt;100 MB files. `totalBytes` from `viewer_open` is the exact answer.
- `Button` gains an `autoFocus` prop that focuses the underlying element after
  a `requestAnimationFrame` so it lands after `ModalDialog`'s post-`tick()`
  overlay focus. The copy confirm and refuse dialogs use it on the primary
  action, so Enter triggers Copy (or Save as file…) by default.
- `ModalDialog` × close button gets `tabindex="-1"` so Tab cycles only the
  action buttons. Keyboard users start on the primary action, not on ×.
- Copy dialogs get `containerStyle="max-width: 480px"` per the design-system
  Dialogs spec, and a `.copy-dialog-body-wrap` with the standard
  `0 24px 24px` padding so text and buttons no longer hit the dialog edge.
diff --git a/apps/desktop/src/lib/ui/Button.svelte b/apps/desktop/src/lib/ui/Button.svelte
@@ -1,5 +1,5 @@
 <script lang="ts">
-    import type { Snippet } from 'svelte'
+    import { onMount, type Snippet } from 'svelte'
 
     type Variant = 'primary' | 'secondary' | 'danger'
     type Size = 'regular' | 'mini'
@@ -11,6 +11,12 @@
         type?: 'button' | 'submit'
         onclick?: (e: MouseEvent) => void
         'aria-label'?: string
+        /**
+         * Focus this button after mount. Uses `requestAnimationFrame` so it lands
+         * after a parent `ModalDialog`'s post-`tick()` overlay focus, which would
+         * otherwise win and steal focus to the scrim.
+         */
+        autoFocus?: boolean
         children: Snippet
     }
 
@@ -21,11 +27,27 @@
         type = 'button',
         onclick,
         'aria-label': ariaLabel,
+        autoFocus = false,
         children,
     }: Props = $props()
+
+    let buttonEl: HTMLButtonElement | undefined
+    onMount(() => {
+        if (!autoFocus) return
+        requestAnimationFrame(() => {
+            buttonEl?.focus()
+        })
+    })
 </script>
 
-<button {type} class="btn btn-{variant} btn-{size}" {disabled} {onclick} aria-label={ariaLabel}>
+<button
+    bind:this={buttonEl}
+    {type}
+    class="btn btn-{variant} btn-{size}"
+    {disabled}
+    {onclick}
+    aria-label={ariaLabel}
+>
     {@render children()}
 </button>
 
diff --git a/apps/desktop/src/lib/ui/ModalDialog.svelte b/apps/desktop/src/lib/ui/ModalDialog.svelte
@@ -131,7 +131,13 @@
 >
     <div class="modal-dialog" class:dragging={isDragging} style={dialogStyle}>
         {#if onclose}
-            <button class="modal-close-button" onclick={onclose} aria-label="Close">×</button>
+            <!--
+                tabindex=-1 keeps the × out of the tab cycle. The dialog's action buttons
+                should be the only tab stops; × is a mouse / Escape-key affordance. Without
+                this, Tab from the overlay lands on × first, which surprises keyboard users
+                expecting the primary or first action to be the entry point.
+            -->
+            <button class="modal-close-button" onclick={onclose} aria-label="Close" tabindex="-1">×</button>
         {/if}
         <!-- svelte-ignore a11y_no_static_element_interactions -->
         <div class="dialog-title-bar" class:draggable onmousedown={handleTitleMouseDown}>
diff --git a/apps/desktop/src/routes/viewer/+page.svelte b/apps/desktop/src/routes/viewer/+page.svelte
@@ -31,6 +31,7 @@
         describeSelectionForAt,
         estimateSelectionBytes,
         getLineSegmentBounds,
+        isWholeFileSelection,
         normaliseSelection,
     } from './selection.svelte'
     import { createViewerCopy } from './viewer-copy.svelte'
@@ -182,8 +183,18 @@
      * the "unknown size" branch and confirm before reading).
      */
     function estimateCurrentSelectionBytes(): number | null {
+        const sel = selection.selection
+        if (sel === null) return 0
+        // Whole-file shortcut: ⌘A on a multi-MB file selects lines the user never scrolled
+        // through, so the line cache can't service the per-line walk. `totalBytes` is the
+        // exact answer (it's the file size from `viewer_open`) and avoids the bail-to-null
+        // that would otherwise route to the "unknown size" confirm dialog instead of the
+        // correct refuse / confirm tier.
+        if (isWholeFileSelection(sel, totalLines)) {
+            return totalBytes
+        }
         return estimateSelectionBytes(
-            selection.selection,
+            sel,
             (n) => {
                 const txt = scroll.lineCache.get(n)
                 if (txt === undefined) return null
@@ -1111,6 +1122,7 @@
         dialogId="viewer-copy-confirm"
         titleId="viewer-copy-confirm-title"
         onclose={cancelCopyConfirm}
+        containerStyle="max-width: 480px"
     >
         {#snippet title()}
             <h2 id="viewer-copy-confirm-title" class="copy-dialog-title">
@@ -1121,23 +1133,26 @@
                 {/if}
             </h2>
         {/snippet}
-        <p class="copy-dialog-body">
-            Large pastes can slow down other apps. Try search (⌘F) to narrow it down.
-        </p>
-        <div class="copy-dialog-actions">
-            <Button variant="secondary" onclick={cancelCopyConfirm}>Cancel</Button>
-            <Button
-                variant="secondary"
-                onclick={() => {
-                    void handleSaveAs()
-                }}>Save as file…</Button
-            >
-            <Button
-                variant="primary"
-                onclick={() => {
-                    if (copyConfirmProceed) void copyConfirmProceed()
-                }}>Copy</Button
-            >
+        <div class="copy-dialog-body-wrap">
+            <p class="copy-dialog-body">
+                Large pastes can slow down other apps. Try search (⌘F) to narrow it down.
+            </p>
+            <div class="copy-dialog-actions">
+                <Button variant="secondary" onclick={cancelCopyConfirm}>Cancel</Button>
+                <Button
+                    variant="secondary"
+                    onclick={() => {
+                        void handleSaveAs()
+                    }}>Save as file…</Button
+                >
+                <Button
+                    variant="primary"
+                    autoFocus
+                    onclick={() => {
+                        if (copyConfirmProceed) void copyConfirmProceed()
+                    }}>Copy</Button
+                >
+            </div>
         </div>
     </ModalDialog>
 {/if}
@@ -1148,24 +1163,28 @@
         dialogId="viewer-copy-refuse"
         titleId="viewer-copy-refuse-title"
         onclose={dismissCopyRefuse}
+        containerStyle="max-width: 480px"
     >
         {#snippet title()}
             <h2 id="viewer-copy-refuse-title" class="copy-dialog-title">
                 Copy {formatBytes(refuseBytes)} to the clipboard?
             </h2>
         {/snippet}
-        <p class="copy-dialog-body">
-            That's larger than the 100 MB clipboard limit. Try search (⌘F) to find what you need, or save the
-            selection as a file.
-        </p>
-        <div class="copy-dialog-actions">
-            <Button variant="secondary" onclick={dismissCopyRefuse}>Cancel</Button>
-            <Button
-                variant="primary"
-                onclick={() => {
-                    void handleSaveAs()
-                }}>Save as file…</Button
-            >
+        <div class="copy-dialog-body-wrap">
+            <p class="copy-dialog-body">
+                That's larger than the 100 MB clipboard limit. Try search (⌘F) to find what you need, or save the
+                selection as a file.
+            </p>
+            <div class="copy-dialog-actions">
+                <Button variant="secondary" onclick={dismissCopyRefuse}>Cancel</Button>
+                <Button
+                    variant="primary"
+                    autoFocus
+                    onclick={() => {
+                        void handleSaveAs()
+                    }}>Save as file…</Button
+                >
+            </div>
         </div>
     </ModalDialog>
 {/if}
@@ -1542,17 +1561,21 @@
         margin: 0;
     }
 
+    /* Matches the AlertDialog body wrapper: design-system § Dialogs body padding 0 24px 24px. */
+    .copy-dialog-body-wrap {
+        padding: 0 var(--spacing-xl) var(--spacing-xl);
+    }
+
     .copy-dialog-body {
         font-size: var(--font-size-md);
         line-height: 1.4;
         color: var(--color-text-secondary);
-        margin-top: var(--spacing-md);
+        margin: 0 0 var(--spacing-xl);
     }
 
     .copy-dialog-actions {
         display: flex;
         gap: var(--spacing-md);
         justify-content: flex-end;
-        margin-top: var(--spacing-xl);
     }
 </style>
diff --git a/apps/desktop/src/routes/viewer/selection.svelte.ts b/apps/desktop/src/routes/viewer/selection.svelte.ts
@@ -138,6 +138,28 @@ export function makeSelectAll(totalLines: number, lastLineLength: number): Selec
   }
 }
 
+/**
+ * Returns `true` if the selection covers the whole file. Used by the copy flow to
+ * short-circuit the per-line byte estimator with the known file size: walking lines
+ * fails for ⌘A on large files because the line cache only contains lines the user
+ * has scrolled past, but the file size is known at viewer-open time.
+ *
+ * Matches three cases:
+ * 1. ByteSeek-no-index sentinel: `end.line === Number.MAX_SAFE_INTEGER`.
+ * 2. Known total lines: `end.line >= totalLines - 1` (the last line is included).
+ * 3. `start === (0, 0)` is required in all cases.
+ *
+ * Reversed selections (anchor below focus) normalise first.
+ */
+export function isWholeFileSelection(sel: Selection | null, totalLines: number | null): boolean {
+  if (sel === null) return false
+  const { start, end } = normaliseSelection(sel)
+  if (start.line !== 0 || start.offset !== 0) return false
+  if (end.line === Number.MAX_SAFE_INTEGER) return true
+  if (totalLines !== null && end.line >= totalLines - 1) return true
+  return false
+}
+
 /**
  * Maximum number of intermediate lines the AT (VoiceOver) announcement loop walks
  * before falling back to a generic "extends past visible content" message. Caps the
diff --git a/apps/desktop/src/routes/viewer/selection.test.ts b/apps/desktop/src/routes/viewer/selection.test.ts
@@ -8,6 +8,7 @@ import {
   getLineSegmentBounds,
   isEmpty,
   isLineInRange,
+  isWholeFileSelection,
   lineOffsetEquals,
   makeSelectAll,
   MAX_ANNOUNCE_LINES,
@@ -233,6 +234,55 @@ describe('makeSelectAll', () => {
   })
 })
 
+describe('isWholeFileSelection', () => {
+  it('matches the output of makeSelectAll', () => {
+    expect(isWholeFileSelection(makeSelectAll(100, 50), 100)).toBe(true)
+  })
+
+  it('matches the ByteSeek-no-index sentinel (focus.line = MAX_SAFE_INTEGER)', () => {
+    const sel: Selection = {
+      anchor: { line: 0, offset: 0 },
+      focus: { line: Number.MAX_SAFE_INTEGER, offset: 0 },
+    }
+    expect(isWholeFileSelection(sel, null)).toBe(true)
+    expect(isWholeFileSelection(sel, 50)).toBe(true)
+  })
+
+  it('rejects non-zero start line', () => {
+    const sel: Selection = { anchor: { line: 1, offset: 0 }, focus: { line: 99, offset: 0 } }
+    expect(isWholeFileSelection(sel, 100)).toBe(false)
+  })
+
+  it('rejects non-zero start offset', () => {
+    const sel: Selection = { anchor: { line: 0, offset: 1 }, focus: { line: 99, offset: 5 } }
+    expect(isWholeFileSelection(sel, 100)).toBe(false)
+  })
+
+  it('rejects end before the last line', () => {
+    const sel: Selection = { anchor: { line: 0, offset: 0 }, focus: { line: 50, offset: 0 } }
+    expect(isWholeFileSelection(sel, 100)).toBe(false)
+  })
+
+  it('treats end at last-line-start as whole-file (over-include is fine for tier classification)', () => {
+    const sel: Selection = { anchor: { line: 0, offset: 0 }, focus: { line: 99, offset: 0 } }
+    expect(isWholeFileSelection(sel, 100)).toBe(true)
+  })
+
+  it('normalises reversed selections', () => {
+    const reversed: Selection = { anchor: { line: 99, offset: 50 }, focus: { line: 0, offset: 0 } }
+    expect(isWholeFileSelection(reversed, 100)).toBe(true)
+  })
+
+  it('returns false for null selection', () => {
+    expect(isWholeFileSelection(null, 100)).toBe(false)
+  })
+
+  it('without totalLines and without sentinel, never matches', () => {
+    const sel: Selection = { anchor: { line: 0, offset: 0 }, focus: { line: 99, offset: 5 } }
+    expect(isWholeFileSelection(sel, null)).toBe(false)
+  })
+})
+
 describe('estimateSelectionBytes', () => {
   // Helper that builds fixed byte / UTF-16 lookups.
   function makeLookups(lines: { bytes: number; utf16: number }[]) {
