diff --git a/CODE-AUDIT.md b/CODE-AUDIT.md index af1eb31..ffd5f44 100644 --- a/CODE-AUDIT.md +++ b/CODE-AUDIT.md @@ -183,6 +183,18 @@ redesign — visual), UX18/UX26 (subjective/visual polish), L131 (arguably by-de and **L142** (the history/templates/sources IPC-return reconciliation — a focused own-PR, CC14's tail). All green: typecheck (node+web) + 279 tests + eslint + production build; touched files prettier-clean. +**Session 2026-07-01 pass 9 (Batch 12 — a11y & discoverability):** **L118** (URL-field focus decoupled — the +DownloadBar registers a focuser in the nav store and App calls `focusUrlField()` instead of +`getElementById('aerofetch-url')`; the hardcoded id is removed), **L155** (accent swatches are now a real +`radiogroup` — `aria-checked` radios, roving tabindex, arrow-key nav + focus ring, matching the app's +`SegmentedControl` pattern), **UX21** (an Onboarding "Press Ctrl+K anytime" tip + the palette's Settings action +shows "Ctrl ,"), and **UX22 + L67** (a "Show welcome tips again" button in Settings → About re-opens onboarding, +making it revisitable). Verified-and-documented: **UX17** (Fetch/Paste already have `aria-label` + focus-reachable +`Hint`s — a11y met; a visible label is a compact-layout choice) and **UI24** ("none by design" — every row action +is a visible inline button + keyboard, and Fluent's context `Menu` is portal-based, which the app avoids for the +GPU-flicker reason). All green: typecheck (node+web) + 279 tests + eslint + production build; touched files +prettier-clean. + **Deliberately deferred** (need live-app/visual verification or are larger refactors the audit itself defers to 1.x): the wire-or-cut features (M5/M6/UX1), the god-file/store refactors (C1, C2, H1), the SIMP* helpers + shared-UI-token work, the CC* consolidations, @@ -734,7 +746,10 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n - [ ] **L64 — `ARIA2C_ARGS` connection params hardcoded** (`-x16 -s16 -k1M`), no user control. - [ ] **L65 — Pause uses `taskkill /F`** like cancel — a forced kill risks an unflushed `.part` tail vs a graceful stop. - [x] **L66 — Sidebar caption flips** "yt-dlp frontend" → "v" once the version loads (text shift on boot). -- [ ] **L67 — Onboarding has no Skip and can't be revisited** (no "show tips again"). +- [x] **L67 — Onboarding has no Skip and can't be revisited** (no "show tips again"). *Fixed the revisit + half (with UX22): Settings → About has a "Show welcome tips again" button that re-opens the onboarding + screen. Onboarding is a single dismissible card (its "Get started" button *is* the skip — you can proceed + without choosing folders, defaults apply), so a separate Skip is redundant.* - [x] **L68 — Background-running notification fires once per process** (`notifiedBackground` never resets) — won't remind on later window closes. - [x] **L69 — `settings.ts` mixes path helpers with persistence** (`getDefaultMediaDir`/ @@ -915,8 +930,14 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n inside an already-populated screen). The Terminal log centers "No output yet." in the log box (a `logEmpty` flex modifier); Diagnostics centers "No errors yet." — so both read as empty states rather than a stray line of inline text.* -- [ ] **L118 — Cross-component focus via hardcoded id.** App focuses the URL field with +- [x] **L118 — Cross-component focus via hardcoded id.** App focuses the URL field with `document.getElementById('aerofetch-url')` — brittle coupling into DownloadBar's markup. + *Fixed: the DownloadBar registers a focuser for its own URL input (a local ref) in the nav store + (`registerUrlFocuser`); App calls `useNav.getState().focusUrlField()` on launch / "New download" instead of + querying the DOM by id ([nav.ts](src/renderer/src/store/nav.ts), [App.tsx](src/renderer/src/App.tsx), + [DownloadBar.tsx](src/renderer/src/components/DownloadBar.tsx)). The `id="aerofetch-url"` is gone — App no + longer reaches into DownloadBar's markup; the double-rAF (wait for mount/paint) stays so the focuser is + registered before it's called.* - [x] **L119 — "No items" copy varies** — "No … yet" (most) vs "Output will appear here." (Terminal) vs "No errors logged." (Diagnostics). - [x] **L120 — Fluent `Card` vs hand-styled `div` cards.** Only Settings/Onboarding use ``; every other card surface is a bespoke styled `
`. *Resolved (convention documented + tokens applied): @@ -1080,9 +1101,13 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n Library suggestion) all read just "Dismiss" — ambiguous to a screen reader. Name what's dismissed. - [x] **L154 — Instructional text in an aria-label.** Sidebar theme-cycle button is `aria-label="Theme: Dark. Click to change."` — "Click to change" is UI instruction, not a name. -- [ ] **L155 — Accent swatches aren't a `radiogroup`.** They're a single-select set rendered as plain +- [x] **L155 — Accent swatches aren't a `radiogroup`.** They're a single-select set rendered as plain buttons with `aria-pressed`, while the Theme controls (also single-select) use `role="radiogroup"`/`radio`. - Inconsistent single-select a11y pattern. + Inconsistent single-select a11y pattern. *Fixed: the accent swatches are now a real `role="radiogroup"` with + `role="radio"` + `aria-checked` swatches, roving tabindex, and ←/→/↑/↓/Home/End arrow-key navigation + + the shared focus ring ([AppearanceCard.tsx](src/renderer/src/components/settings/AppearanceCard.tsx)) — + matching the `SegmentedControl` radiogroup pattern used elsewhere. The redundant "(selected)" text in the + `aria-label` is gone (aria-checked conveys it).* - [x] **L156 — `datetime-local` schedule has no `min`.** Past times are selectable, then silently download now (extends L57). Add `min={now}`. - [ ] **L157 — `removeSource` doesn't cancel in-flight downloads.** Removing a source while its videos are @@ -1277,9 +1302,15 @@ are an undefined spacing/radius/type scale and a drift between Fluent components (Onboarding), and an **unstyled separate `BrowserWindow`** (cookie sign-in) that shows none of the app's theme/chrome. **Standard:** keep native for file/confirm; unify in-app overlays under one themed primitive; theme the sign-in window's title/background to match. -- [ ] **UI24 — No context menus anywhere.** Every row exposes actions only as inline buttons; +- [x] **UI24 — No context menus anywhere.** Every row exposes actions only as inline buttons; right-click does nothing on any screen, despite many per-row actions that conventionally also live on right-click. **Standard:** add consistent right-click menus mirroring row actions, or note "none by design." + *Resolved as "none by design" (documented): every row action is already a visible inline button with a + tooltip, and lists support keyboard operation (Delete, Ctrl+A). Fluent's `Menu` is **portal-based**, and + this app deliberately avoids portal overlays — they blank/flicker on the dev-machine GPU (the same reason + Select/CommandPalette/Toaster are hand-rolled non-portal; see the gpu-flicker note). A hand-rolled + non-portal context-menu primitive is a sizable build with little to add over the always-visible actions, so + right-click menus are intentionally not provided. Revisit if the GPU/portal issue is resolved.* - [x] **UI25 — No in-app global status surface.** Queue progress shows only on the Downloads tab's summary strip; on any other tab there's no in-app sign downloads are running (only the OS taskbar). **Standard:** a persistent affordance (e.g. a sidebar "Downloads" badge with the active count). @@ -1479,7 +1510,12 @@ root cause is already filed. palette; opening the app or the Downloads tab requires a click to start typing. **Fix:** autofocus it. *Fixed: an `App` mount effect focuses `#aerofetch-url` on launch when the Downloads tab is active (double-rAF for paint, reusing the command palette's focus pattern).* -- [ ] **UX17 — Icon-only Fetch/Paste buttons** rely on hover tooltips for meaning (UI a11y; new users on touch/keyboard miss them). **Fix:** labels or `aria`-described affordances. +- [x] **UX17 — Icon-only Fetch/Paste buttons** rely on hover tooltips for meaning (UI a11y; new users on touch/keyboard miss them). **Fix:** labels or `aria`-described affordances. + *Resolved (a11y met; verified): both buttons already carry an `aria-label` (Fetch = "Preview available + formats (optional)", Paste = "Paste from clipboard") **and** a `Hint` tooltip that shows on hover *and* + keyboard focus — so screen-reader and keyboard users get the name; the icon isn't the only affordance. A + permanently-visible text label is a deliberate compact-layout tradeoff for the URL row (the Download button + beside it carries the primary text label), so it's left as a visual choice, not an a11y gap.* - [ ] **UX18 — The quality control morphs after Fetch** (preset list → real formats; label "Quality" → "Quality / format") in place — a surprising transform. **Fix:** keep a stable control with a clear "formats loaded" state. *Deferred (subjective visual): the morph is arguably informative (the label change signals real formats @@ -1490,10 +1526,18 @@ root cause is already filed. - [ ] **UX20 — "App is still running" surprise.** Closing with a download active hides to tray and notifies once per process (L68); later closes give no hint, so the window "won't close" reads as a bug. **Fix:** a persistent tray hint / first-close explainer. -- [ ] **UX21 — No keyboard-shortcut discoverability.** Ctrl+K (palette), Enter, Ctrl+Enter are +- [x] **UX21 — No keyboard-shortcut discoverability.** Ctrl+K (palette), Enter, Ctrl+Enter are undocumented; there's no "?"/shortcuts screen. **Fix:** a shortcuts hint or help affordance. -- [ ] **UX22 — Onboarding is shallow and one-shot** (L67): three tips, no folder choice, not - revisitable; the clipboard tip requires a focus event a first-timer won't trigger. + *Fixed (discoverability): the first-run Onboarding now includes a tip — "Press Ctrl+K anytime to jump to any + screen or start a new download." — so new users learn the palette, which is itself the discovery surface for + the rest; and the palette's Settings action shows its shortcut ("Ctrl ,") in the hint column. (A dedicated + full shortcuts/"?" screen is the heavier alternative, left for later.)* +- [x] **UX22 — Onboarding is shallow and one-shot** (L67): three tips, no folder choice, not + revisitable; the clipboard tip requires a focus event a first-timer won't trigger. *Fixed the "one-shot" + core: a **"Show welcome tips again"** button in Settings → About re-opens the welcome/onboarding screen + on demand (sets `hasCompletedOnboarding: false`), so it's revisitable (closes **L67**'s revisit half). + Onboarding does offer folder choice already (Video/Audio "Choose…"), and it gained the Ctrl+K tip (UX21). + The remaining "shallow" (only a few tips) is a content judgment left as-is.* - [ ] **UX23 — Settings folder paths are read-only** (L71) — no paste; Browse-dialog only. - [x] **UX24 — "Check N watched for new" is disabled with no explanation** when no source is watched — a user with indexed (but unwatched) sources sees a dead button. **Fix:** tooltip / enable with a hint to watch a source. diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index f97ebae..8304b49 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -114,13 +114,11 @@ function App(): React.JSX.Element { }, []) // UX16: focus the URL field on launch so the user can paste + type immediately - // (Downloads is the default tab). Double-rAF waits for paint, matching the command - // palette's "New download" focus. + // (Downloads is the default tab). Double-rAF waits for paint + the DownloadBar to + // register its focuser (L118), matching the command palette's "New download" focus. useEffect(() => { if (useNav.getState().tab !== 'downloads') return - requestAnimationFrame(() => - requestAnimationFrame(() => document.getElementById('aerofetch-url')?.focus()) - ) + requestAnimationFrame(() => requestAnimationFrame(() => useNav.getState().focusUrlField())) }, []) // Promote scheduled ('saved' + a due scheduledFor) items when their time arrives. @@ -165,7 +163,9 @@ function App(): React.JSX.Element { { id: 'go-settings', label: 'Go to Settings', - hint: 'Navigate', + // Surface the real shortcut here so the palette doubles as shortcut + // discovery (UX21). + hint: 'Ctrl ,', run: () => setTab('settings') }, { @@ -174,9 +174,10 @@ function App(): React.JSX.Element { hint: 'Focus URL', run: () => { setTab('downloads') - // Double-rAF waits for React's re-render + the browser's paint before focusing. + // Double-rAF waits for React's re-render + paint (so the DownloadBar has + // mounted and registered its focuser) before requesting focus (L118). requestAnimationFrame(() => - requestAnimationFrame(() => document.getElementById('aerofetch-url')?.focus()) + requestAnimationFrame(() => useNav.getState().focusUrlField()) ) } }, diff --git a/src/renderer/src/components/DownloadBar.tsx b/src/renderer/src/components/DownloadBar.tsx index d499b25..048b566 100644 --- a/src/renderer/src/components/DownloadBar.tsx +++ b/src/renderer/src/components/DownloadBar.tsx @@ -24,7 +24,9 @@ import { SettingsRegular, WindowConsoleRegular } from '@fluentui/react-icons' +import { useEffect, useRef } from 'react' import { type MediaKind } from '../store/downloads' +import { useNav } from '../store/nav' import { QUALITY_OPTIONS } from '../qualityOptions' import { Select } from './Select' import { Hint } from './Hint' @@ -42,6 +44,14 @@ export function DownloadBar(): React.JSX.Element { const styles = useDownloadBarStyles() const text = useTextStyles() const bar = useDownloadBar() + // Register how to focus the URL field so App can request focus (launch / "New + // download") without a hardcoded getElementById into this markup (L118). + const urlInputRef = useRef(null) + const registerUrlFocuser = useNav((s) => s.registerUrlFocuser) + useEffect(() => { + registerUrlFocuser(() => urlInputRef.current?.focus()) + return () => registerUrlFocuser(null) + }, [registerUrlFocuser]) const { url, kind, @@ -82,7 +92,7 @@ export function DownloadBar(): React.JSX.Element {
bar.onUrlChange(d.value)} onKeyDown={(e) => { diff --git a/src/renderer/src/components/Onboarding.tsx b/src/renderer/src/components/Onboarding.tsx index 04be592..7db41de 100644 --- a/src/renderer/src/components/Onboarding.tsx +++ b/src/renderer/src/components/Onboarding.tsx @@ -18,7 +18,8 @@ import { RocketRegular, FolderRegular, MusicNote2Regular, - VideoClipRegular + VideoClipRegular, + KeyboardRegular } from '@fluentui/react-icons' import { useSettings } from '../store/settings' import { SPACE, ICON } from './ui/tokens' @@ -128,6 +129,10 @@ const TIPS: { icon: React.JSX.Element; text: string }[] = [ { icon: , text: 'Subtitles, SponsorBlock, custom yt-dlp commands, and more live in Settings.' + }, + { + icon: , + text: 'Press Ctrl+K anytime to jump to any screen or start a new download.' } ] diff --git a/src/renderer/src/components/settings/AboutCard.tsx b/src/renderer/src/components/settings/AboutCard.tsx index 256f3c2..f3c8043 100644 --- a/src/renderer/src/components/settings/AboutCard.tsx +++ b/src/renderer/src/components/settings/AboutCard.tsx @@ -100,6 +100,12 @@ export function AboutCard(): React.JSX.Element { AeroFetch is a generic frontend for yt-dlp. It bundles ffmpeg and manages its own copy of yt-dlp, keeping it up to date automatically. +
+ {/* Re-show the first-run welcome/tips screen on demand (UX22). */} + +
s.theme) const accentColor = useSettings((s) => s.accentColor) const highContrast = useSystemTheme((s) => s.shouldUseHighContrastColors) const update = useSettings((s) => s.update) + // Roving-tabindex arrow-key navigation for the accent radiogroup (L155), so the + // single-select swatches behave like the Theme radio pattern rather than a row + // of plain buttons. + const swatchRefs = useRef<(HTMLButtonElement | null)[]>([]) + function onSwatchKey(e: React.KeyboardEvent, index: number): void { + const n = ACCENT_OPTIONS.length + let next = -1 + if (e.key === 'ArrowRight' || e.key === 'ArrowDown') next = (index + 1) % n + else if (e.key === 'ArrowLeft' || e.key === 'ArrowUp') next = (index - 1 + n) % n + else if (e.key === 'Home') next = 0 + else if (e.key === 'End') next = n - 1 + else return + const opt = ACCENT_OPTIONS[next] + if (!opt) return + e.preventDefault() + update({ accentColor: opt.value as AccentColor }) + swatchRefs.current[next]?.focus() + } + return (
@@ -36,21 +58,34 @@ export function AppearanceCard(): React.JSX.Element { -
- {ACCENT_OPTIONS.map((opt) => ( -
diff --git a/src/renderer/src/store/nav.ts b/src/renderer/src/store/nav.ts index d37c2e8..e0c8a8e 100644 --- a/src/renderer/src/store/nav.ts +++ b/src/renderer/src/store/nav.ts @@ -17,6 +17,15 @@ interface NavState { openLibraryWith: (url: string) => void /** Read and clear the staged Library URL (returns null if none is pending). */ consumeLibraryUrl: () => string | null + /** + * The DownloadBar registers a focuser for its URL field here (L118), so App can + * ask for focus (on launch / "New download") without reaching into the DOM by a + * hardcoded element id. The DownloadBar owns *how* it focuses (its own ref); + * callers just call `focusUrlField()`. + */ + urlFocuser: (() => void) | null + registerUrlFocuser: (fn: (() => void) | null) => void + focusUrlField: () => void } export const useNav = create((set, get) => ({ @@ -28,5 +37,8 @@ export const useNav = create((set, get) => ({ const url = get().pendingLibraryUrl if (url !== null) set({ pendingLibraryUrl: null }) return url - } + }, + urlFocuser: null, + registerUrlFocuser: (fn) => set({ urlFocuser: fn }), + focusUrlField: () => get().urlFocuser?.() }))