feat(audit): Batch 17 — screen scroll contract + History virtualization (L161, L104)

L161: <main> is now a non-scrolling flex slot; each screen owns its scroll via shared shells in ui/Screen.tsx — 'page' (Library/Settings/empty History scroll the shell) and 'fill' (Downloads/Terminal/History scroll their own list/log, shell scrolls only as short-window fallback). height:100% couplings replaced with flexGrow+minHeight:0; list regions floored at 160px so they can't collapse on short windows. L104: HistoryView renders through VirtualList; Ctrl+A re-homed onto a wrapper around the scroller, per-row Delete unchanged on rows. Verified with an Electron probe against the vite preview at 1100x720 and 800x520 (probe metrics + screenshots): one active scroller per screen, main/body never scroll, floors hold. Adds .claude/launch.json for the preview server. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-02 11:43:34 -04:00
parent dbbb0c70f9
commit 17d42ed4fe
9 changed files with 755 additions and 642 deletions
@@ -0,0 +1,11 @@
+{
+  "version": "0.0.1",
+  "configurations": [
+    {
+      "name": "ui",
+      "runtimeExecutable": "npm",
+      "runtimeArgs": ["run", "ui", "--", "--port", "4321", "--strictPort"],
+      "port": 4321
+    }
+  ]
+}
@@ -992,13 +992,13 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
  `<Spinner size="tiny">` (Fetch/Index/Check-for-new/yt-dlp update/app-update/PO-mint/version-check); the only
  remaining `<Spinner>`s are standalone status-row indicators (the "Fetching…"/"Waiting to start…"/indexing
  rows), not a button + adjacent spinner. No separate-adjacent-spinner pattern survives.*
- [ ] **L104 — History isn't virtualized.** It renders all filtered rows, while Downloads and Library use `VirtualList`.
-  *Deferred (bounded; needs the L161 layout rework + live verification): History is capped at 500 rows (L141),
-  so the un-virtualized render is bounded — not a real perf problem. Virtualizing it requires giving the list a
-  fixed-height internal scroll container (the same `height:100%`/`overflow` rework L161 flags as needing
-  live verification at multiple window sizes) *and* re-homing its container-level Ctrl+A and per-row Delete
-  keyboard handling onto the virtualized scroller. That's a layout+interaction change best done watched, so it
-  rides with L161 rather than shipping unattended for a list that's already capped.*
+- [x] **L104 — History isn't virtualized.** It renders all filtered rows, while Downloads and Library use `VirtualList`.
+  *Fixed (Batch 17, with L161): HistoryView renders through the shared `VirtualList` inside the new `fill`
+  screen shell — the list owns an internal scroll region (flex + 160px floor, DownloadsView's contract). The
+  container-level Ctrl+A moved onto a wrapper around the virtualized scroller (key events bubble up from the
+  rows), and per-row Delete stays on each row so it works wherever the row mounts. Verified live in an
+  Electron probe against the vite preview at 1100×720 and 800×520: rows render via `[data-index]` virtual
+  wrappers, the list scrolls internally, and `<main>`/body never scroll (probe metrics + screenshots).*
 - [x] **L105 — `MediaKind` declared twice.** In both [ipc.ts](src/shared/ipc.ts) and
  [store/downloads.ts](src/renderer/src/store/downloads.ts); components import it from both. Single source it.
  *Fixed: the store now imports `MediaKind` from `@shared/ipc` and re-exports it, so the duplicate local
@@ -1301,12 +1301,18 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
  8px) instead of style classes (extends L5). *Fixed: the margin literals now draw from `SPACE`
  (`hairline`/`xtight`/`tight`) — same values, one source. (The broader inline-`style` vs makeStyles split is
  still L5.)*
- [ ] **L161 — `height: 100%` screens inside a padded scroll container.** Downloads/Terminal set
+- [x] **L161 — `height: 100%` screens inside a padded scroll container.** Downloads/Terminal set
  `height: 100%` while App's `<main>` is `overflowY: auto` + 24/28px padding — a fragile coupling that can
-  double-scroll or misalign the virtualized list's viewport. *Deferred (layout, needs live verification): it
-  works today (the virtualized queue scrolls correctly), and reworking the `<main>` scroll/height contract is
-  a layout change best validated with the app open at several window sizes — a watched pass, not an unattended
-  edit.*
+  double-scroll or misalign the virtualized list's viewport. *Fixed (Batch 17): `<main>` no longer scrolls or
+  pads — it's a plain flex slot, and each screen owns its scrolling through two shared shells in
+  [ui/Screen.tsx](src/renderer/src/components/ui/Screen.tsx): `page` (document screens — Library, Settings,
+  empty History: the shell scrolls) and `fill` (internally-scrolling screens — Downloads, Terminal, History:
+  the screen's list/log is the scroller, with the shell scrolling only as the short-window fallback). The
+  `height:100%` couplings are gone (`flexGrow:1; minHeight:0` inside the shell instead). The rework surfaced a
+  real pre-existing edge: on a very short window the flexed list collapsed to 0px — now floored at 160px with
+  the shell absorbing the overflow. Verified with an Electron probe at 1100×720 and 800×520 per screen:
+  exactly one active scroller per screen in the normal case, `<main>`/body never scroll, floors hold
+  (probe metrics + screenshots).*
 - [ ] **L162 — Per-thumbnail store subscription.** Every `MediaThumb` calls `useResolvedDark()` (two store
  subscriptions) instead of receiving a resolved theme prop — N subscriptions per list. *Deferred — same as
  PERF6: virtualization/the 500-cap bounds the mounted count, and a prop conflicts with PERF5's hoisted
@@ -7,7 +7,6 @@ import { CommandPalette, type PaletteAction } from './components/CommandPalette'
 import { LiveRegion } from './components/LiveRegion'
 import { Toaster } from './components/ui/Toaster'
 import { getTheme, pageBackground } from './theme'
-import { SPACE } from './components/ui/tokens'
 import { useSettings } from './store/settings'
 import { useNav } from './store/nav'
 import { useDownloads } from './store/downloads'
@@ -55,9 +54,13 @@ const useStyles = makeStyles({
  content: {
    flexGrow: 1,
    minWidth: 0,
-    overflowY: 'auto',
-    // Symmetric page padding (UI4): the old 28px horizontal appeared nowhere else.
-    padding: SPACE.page
+    // L161: <main> no longer scrolls or pads — each screen owns its scroll region
+    // via the shared `page`/`fill` shells (ui/Screen.tsx), so a page scroll and an
+    // internal list scroll can never stack. Padding (SPACE.page, UI4) lives on the
+    // shells.
+    display: 'flex',
+    flexDirection: 'column',
+    overflow: 'hidden'
  }
 })

@@ -26,14 +26,15 @@ const renderQueueRow = (item: DownloadItem): React.JSX.Element => <QueueItem ite

 const useStyles = makeStyles({
  root: {
-    // Fill the scroll area so the queue list below can flex to the remaining
-    // height and scroll internally (it's virtualized), instead of the whole page
-    // growing to thousands of rows.
+    // Fill the `fill` shell (L161) so the queue list below can flex to the
+    // remaining height and scroll internally (it's virtualized), instead of the
+    // whole page growing to thousands of rows.
    display: 'flex',
    flexDirection: 'column',
    // One section gap on every screen (UI2).
    gap: SPACE.section,
-    height: '100%'
+    flexGrow: 1,
+    minHeight: 0
  },
  queueHeader: {
    display: 'flex',
@@ -63,9 +64,10 @@ const useStyles = makeStyles({
  },
  listScroll: {
    flexGrow: 1,
-    // min-height:0 lets this flex child shrink below its content height so it,
-    // not the page, owns the scrolling.
-    minHeight: 0
+    // The floor keeps the virtualized queue visible on a very short window (the
+    // `fill` shell scrolls the overflow); above it, the list shrinks below its
+    // content height so it, not the page, owns the scrolling.
+    minHeight: '160px'
  }
 })

@@ -95,6 +97,7 @@ export function DownloadsView(): React.JSX.Element {
  const cancelable = items.filter((i) => i.status === 'downloading' || i.status === 'queued').length

  return (
+    <div className={screen.fill}>
      <div className={mergeClasses(styles.root, screen.width)}>
        <ScreenHeader title="Downloads" description="Fetch a video, playlist, or channel by URL." />

@@ -131,7 +134,12 @@ export function DownloadsView(): React.JSX.Element {
              </Button>
            )}
            {cancelable > 0 && (
-            <Button size="small" appearance="subtle" icon={<DismissRegular />} onClick={cancelAll}>
+              <Button
+                size="small"
+                appearance="subtle"
+                icon={<DismissRegular />}
+                onClick={cancelAll}
+              >
                Cancel all ({cancelable})
              </Button>
            )}
@@ -160,5 +168,6 @@ export function DownloadsView(): React.JSX.Element {
          />
        )}
      </div>
+    </div>
  )
 }
@@ -30,6 +30,7 @@ import { thumbUrl } from '../thumb'
 import { MediaThumb } from './MediaThumb'
 import { Hint } from './Hint'
 import { Select } from './Select'
+import { VirtualList } from './VirtualList'
 import { ScreenHeader, useScreenStyles } from './ui/Screen'
 import { EmptyState } from './ui/EmptyState'
 import { useFocusStyles } from './ui/focusRing'
@@ -43,6 +44,21 @@ const useStyles = makeStyles({
    flexDirection: 'column',
    gap: SPACE.section
  },
+  // Fills the `fill` shell (L161) so the virtualized list below owns the scroll.
+  fill: {
+    flexGrow: 1,
+    minHeight: 0
+  },
+  listScroll: {
+    flexGrow: 1,
+    // The floor keeps the virtualized list visible on a very short window (the
+    // `fill` shell scrolls the overflow); above it, the list shrinks below its
+    // content height so it, not the page, owns the scrolling (same contract as
+    // DownloadsView).
+    minHeight: '160px',
+    display: 'flex',
+    flexDirection: 'column'
+  },
  header: {
    display: 'flex',
    alignItems: 'center',
@@ -65,11 +81,6 @@ const useStyles = makeStyles({
  spacer: {
    flexGrow: 1
  },
-  list: {
-    display: 'flex',
-    flexDirection: 'column',
-    gap: '8px'
-  },
  row: {
    display: 'flex',
    alignItems: 'center',
@@ -127,6 +138,11 @@ const KIND_FILTER_OPTIONS = [
  { value: 'audio', label: 'Audio' }
 ]

+// Hoisted so the virtualizer gets stable function identities (PERF5). The row
+// renderer itself stays a component-scoped closure — it needs select-mode state.
+const estimateRowSize = (): number => 76
+const getEntryKey = (h: HistoryEntry): string => h.id
+
 export function HistoryView(): React.JSX.Element {
  const styles = useStyles()
  const screen = useScreenStyles()
@@ -214,8 +230,82 @@ export function HistoryView(): React.JSX.Element {
    exitSelectMode()
  }

+  // One history row; rendered through the virtualized list (L104). The per-row
+  // Delete key handling stays on the row itself, so it works wherever the row is
+  // mounted in the window.
+  function renderRow(h: HistoryEntry): React.JSX.Element {
+    return (
+      <div
+        className={mergeClasses(styles.row, focus.focusRing)}
+        tabIndex={0}
+        onKeyDown={(e) => {
+          // Delete removes the focused row (not when a child button is focused).
+          if (e.key === 'Delete' && e.target === e.currentTarget) {
+            e.preventDefault()
+            remove(h.id)
+          }
+        }}
+      >
+        {selectMode && (
+          <Checkbox
+            checked={selected.has(h.id)}
+            onChange={(_, d) => toggleSelected(h.id, !!d.checked)}
+            aria-label={`Select ${h.title}`}
+          />
+        )}
+        <MediaThumb
+          className={styles.thumb}
+          src={thumbUrl({ thumbnail: h.thumbnail, url: h.url })}
+          kind={h.kind}
+          iconSize={22}
+        />
+        <div className={styles.body}>
+          <Text className={text.title}>{h.title}</Text>
+          <Caption1 className={text.muted}>
+            {[h.quality, h.sizeLabel, formatWhen(h.completedAt)].filter(Boolean).join(META_SEP)}
+          </Caption1>
+        </div>
+        <div className={styles.actions}>
+          <Hint label="Re-download" placement="top" align="end">
+            <Button
+              appearance="subtle"
+              icon={<ArrowClockwiseRegular />}
+              onClick={() => redownload(h)}
+              aria-label="Re-download"
+            />
+          </Hint>
+          <Hint label="Open file" placement="top" align="end">
+            <Button
+              appearance="subtle"
+              icon={<OpenRegular />}
+              onClick={() => openFile(h.id)}
+              aria-label="Open file"
+            />
+          </Hint>
+          <Hint label="Show in folder" placement="top" align="end">
+            <Button
+              appearance="subtle"
+              icon={<FolderRegular />}
+              onClick={() => showInFolder(h.id)}
+              aria-label="Show in folder"
+            />
+          </Hint>
+          <Hint label="Remove from history" placement="top" align="end">
+            <Button
+              appearance="subtle"
+              icon={<DeleteRegular />}
+              onClick={() => remove(h.id)}
+              aria-label="Remove from history"
+            />
+          </Hint>
+        </div>
+      </div>
+    )
+  }
+
  if (entries.length === 0) {
    return (
+      <div className={screen.page}>
        <div className={mergeClasses(styles.root, screen.width)}>
          <ScreenHeader title="History" description="Files you've finished downloading." />
          <EmptyState
@@ -231,11 +321,13 @@ export function HistoryView(): React.JSX.Element {
            hint="Finished downloads will show up here."
          />
        </div>
+      </div>
    )
  }

  return (
-    <div className={mergeClasses(styles.root, screen.width)}>
+    <div className={screen.fill}>
+      <div className={mergeClasses(styles.root, styles.fill, screen.width)}>
        <ScreenHeader title="History" description="Files you've finished downloading." />
        <div className={styles.header}>
          {selectMode ? (
@@ -352,11 +444,13 @@ export function HistoryView(): React.JSX.Element {
          <Caption1 className={styles.noMatches}>No downloads match your search.</Caption1>
        ) : (
          <div
-          className={styles.list}
+            className={styles.listScroll}
            onKeyDown={(e) => {
              // W7: Ctrl/Cmd+A within the list selects every visible row (entering
              // select mode if needed). Scoped to the list, so it never hijacks
-            // Ctrl+A in the search field, which lives in the header above.
+              // Ctrl+A in the search field, which lives in the header above. Lives
+              // on this wrapper (key events bubble up from the rows) because the
+              // virtualized scroller below renders rows through a render prop (L104).
              if ((e.ctrlKey || e.metaKey) && (e.key === 'a' || e.key === 'A')) {
                e.preventDefault()
                setSelectMode(true)
@@ -364,80 +458,18 @@ export function HistoryView(): React.JSX.Element {
              }
            }}
          >
-          {filtered.map((h: HistoryEntry) => {
-            return (
-              <div
-                key={h.id}
-                className={mergeClasses(styles.row, focus.focusRing)}
-                tabIndex={0}
-                onKeyDown={(e) => {
-                  // Delete removes the focused row (not when a child button is focused).
-                  if (e.key === 'Delete' && e.target === e.currentTarget) {
-                    e.preventDefault()
-                    remove(h.id)
-                  }
-                }}
-              >
-                {selectMode && (
-                  <Checkbox
-                    checked={selected.has(h.id)}
-                    onChange={(_, d) => toggleSelected(h.id, !!d.checked)}
-                    aria-label={`Select ${h.title}`}
+            <VirtualList
+              items={filtered}
+              className={styles.listScroll}
+              gap={8}
+              overscan={6}
+              estimateSize={estimateRowSize}
+              getKey={getEntryKey}
+              renderItem={renderRow}
            />
-                )}
-                <MediaThumb
-                  className={styles.thumb}
-                  src={thumbUrl({ thumbnail: h.thumbnail, url: h.url })}
-                  kind={h.kind}
-                  iconSize={22}
-                />
-                <div className={styles.body}>
-                  <Text className={text.title}>{h.title}</Text>
-                  <Caption1 className={text.muted}>
-                    {[h.quality, h.sizeLabel, formatWhen(h.completedAt)]
-                      .filter(Boolean)
-                      .join(META_SEP)}
-                  </Caption1>
-                </div>
-                <div className={styles.actions}>
-                  <Hint label="Re-download" placement="top" align="end">
-                    <Button
-                      appearance="subtle"
-                      icon={<ArrowClockwiseRegular />}
-                      onClick={() => redownload(h)}
-                      aria-label="Re-download"
-                    />
-                  </Hint>
-                  <Hint label="Open file" placement="top" align="end">
-                    <Button
-                      appearance="subtle"
-                      icon={<OpenRegular />}
-                      onClick={() => openFile(h.id)}
-                      aria-label="Open file"
-                    />
-                  </Hint>
-                  <Hint label="Show in folder" placement="top" align="end">
-                    <Button
-                      appearance="subtle"
-                      icon={<FolderRegular />}
-                      onClick={() => showInFolder(h.id)}
-                      aria-label="Show in folder"
-                    />
-                  </Hint>
-                  <Hint label="Remove from history" placement="top" align="end">
-                    <Button
-                      appearance="subtle"
-                      icon={<DeleteRegular />}
-                      onClick={() => remove(h.id)}
-                      aria-label="Remove from history"
-                    />
-                  </Hint>
-                </div>
-              </div>
-            )
-          })}
          </div>
        )}
      </div>
+    </div>
  )
 }
@@ -505,6 +505,7 @@ export function LibraryView(): React.JSX.Element {
  }

  return (
+    <div className={screen.page}>
      <div className={mergeClasses(styles.root, screen.width)}>
        <ScreenHeader
          title="Library"
@@ -605,7 +606,9 @@ export function LibraryView(): React.JSX.Element {
            <Spinner size="tiny" />
            <Text>
              {indexing.message ?? 'Adding…'}
-            {indexing.current && indexing.total ? `  (${indexing.current}/${indexing.total})` : ''}
+              {indexing.current && indexing.total
+                ? `  (${indexing.current}/${indexing.total})`
+                : ''}
            </Text>
            {/* B2: a big channel's index walk can take minutes — let the user abort it. */}
            <Button
@@ -766,6 +769,7 @@ export function LibraryView(): React.JSX.Element {
          </div>
        )}
      </div>
+    </div>
  )
 }

@@ -66,8 +66,12 @@ export function SettingsView(): React.JSX.Element {
  const noResults = search.trim() !== '' && hidden.length > 0 && hidden.every(Boolean)

  return (
+    <div className={screen.page}>
      <div className={mergeClasses(styles.root, screen.width)}>
-      <ScreenHeader title="Settings" description="Folders, formats, network, cookies, and more." />
+        <ScreenHeader
+          title="Settings"
+          description="Folders, formats, network, cookies, and more."
+        />
        <div>
          <Input
            input={{ 'aria-label': 'Search settings' }}
@@ -97,5 +101,6 @@ export function SettingsView(): React.JSX.Element {
          </div>
        ))}
      </div>
+    </div>
  )
 }
@@ -17,7 +17,8 @@ import { SPACE } from './ui/tokens'
 import { useTerminalRun, type LineKind } from './useTerminalRun'

 const useStyles = makeStyles({
-  root: { display: 'flex', flexDirection: 'column', gap: SPACE.section, height: '100%' },
+  // Fills the `fill` shell (L161); the log <pre> below is the screen's scroller.
+  root: { display: 'flex', flexDirection: 'column', gap: SPACE.section, flexGrow: 1, minHeight: 0 },
  gate: {
    display: 'flex',
    flexDirection: 'column',
@@ -86,6 +87,7 @@ export function TerminalView(): React.JSX.Element {
          : undefined

  return (
+    <div className={screen.fill}>
      <div className={mergeClasses(styles.root, screen.width)}>
        <ScreenHeader
          title="Terminal"
@@ -100,8 +102,8 @@ export function TerminalView(): React.JSX.Element {
        {!customCommandEnabled && (
          <div className={styles.gate}>
            <Body1>
-            The terminal runs the bundled yt-dlp with your own arguments — part of custom commands,
-            which can pass arbitrary yt-dlp flags. Turn it on to use the terminal.
+              The terminal runs the bundled yt-dlp with your own arguments — part of custom
+              commands, which can pass arbitrary yt-dlp flags. Turn it on to use the terminal.
            </Body1>
            <Button
              appearance="primary"
@@ -156,9 +158,16 @@ export function TerminalView(): React.JSX.Element {
          </div>
        </div>

-      <pre className={mergeClasses(styles.log, lines.length === 0 && styles.logEmpty)} ref={logRef}>
+        <pre
+          className={mergeClasses(styles.log, lines.length === 0 && styles.logEmpty)}
+          ref={logRef}
+        >
          {lines.length === 0 ? (
-          <EmptyState compact message="No output yet." hint="Run yt-dlp above to see its output." />
+            <EmptyState
+              compact
+              message="No output yet."
+              hint="Run yt-dlp above to see its output."
+            />
          ) : (
            lines.map((l) => (
              <div key={l.id} className={lineClass(l.kind)}>
@@ -168,5 +177,6 @@ export function TerminalView(): React.JSX.Element {
          )}
        </pre>
      </div>
+    </div>
  )
 }
@@ -1,7 +1,9 @@
 import { Subtitle2, Caption1, makeStyles, tokens } from '@fluentui/react-components'
+import { SPACE } from './tokens'

 /**
- * One shared content max-width for every screen (UI1).
+ * One shared content max-width for every screen (UI1), plus the screen scroll
+ * contract (L161).
 *
 * Settings used to be a 640px column while the list screens (Downloads, Library,
 * History, Terminal) were full-width, so on a wide window Settings read as an odd
@@ -10,7 +12,19 @@ import { Subtitle2, Caption1, makeStyles, tokens } from '@fluentui/react-compone
 * is already under the cap, so the list screens are visually unchanged — only the
 * wide-window upper bound is now shared.
 *
- * Merge `screen.width` onto each screen's existing root with `mergeClasses`.
+ * Scroll contract (L161): App's `<main>` is a non-scrolling flex column — each
+ * screen owns its scrolling through one of two shells, so a page scroll and an
+ * internal list scroll can never stack:
+ *
+ *   - `page` — document screens (Library, Settings, empty History): the shell
+ *     scrolls; content is natural height.
+ *   - `fill` — internally-scrolling screens (Downloads, Terminal, History): the
+ *     shell fills the viewport and hides overflow; the screen's own list/log
+ *     region (VirtualList, the terminal <pre>) is the only scroller.
+ *
+ * Usage: shell div outside, `mergeClasses(styles.root, screen.width)` inside.
+ * Inside a `fill` shell the inner root uses `flexGrow: 1; minHeight: 0` (not
+ * `height: 100%`, which is what coupled screens to `<main>`'s padding before).
 */
 export const useScreenStyles = makeStyles({
  width: {
@@ -18,6 +32,25 @@ export const useScreenStyles = makeStyles({
    maxWidth: '1200px',
    marginLeft: 'auto',
    marginRight: 'auto'
+  },
+  page: {
+    flexGrow: 1,
+    minHeight: 0,
+    overflowY: 'auto',
+    padding: SPACE.page
+  },
+  fill: {
+    flexGrow: 1,
+    minHeight: 0,
+    display: 'flex',
+    flexDirection: 'column',
+    // Normally content fits and only the screen's internal list/log scrolls; on a
+    // very short window the fixed-height rows above the list (header, download
+    // bar) plus the list's min-height floor overflow, and the shell scrolls as
+    // the fallback instead of the floor collapsing to 0px.
+    overflowY: 'auto',
+    overflowX: 'hidden',
+    padding: SPACE.page
  }
 })