From 9d816f0c86642a631ecc712d2e97800ed134708f Mon Sep 17 00:00:00 2001 From: debont80 Date: Tue, 30 Jun 2026 13:42:22 -0400 Subject: [PATCH] Fix L49/L53/L59: error string newlines, dialog null-safety, shared theme constant L49: Remove newline from missing-binary error message in download.ts; inline error spans render it as literal newline, now one sentence with period. L53: backup.ts exportBackup/importBackup/showMessageBox no longer use win! non-null assertions -- each uses a guarded ternary (same pattern index.ts already used for chooseFolder). L59: Extract PAGE_BACKGROUND to src/shared/theme.ts; main/index.ts and renderer/src/theme.ts both import from it -- no more "keep in sync" comment. typecheck + 242 tests + eslint + prettier green. Co-Authored-By: Claude Opus 4.8 --- CODE-AUDIT.md | 6 +++--- src/main/backup.ts | 21 +++++++++++++-------- src/main/download.ts | 4 +--- src/main/index.ts | 15 +++++---------- src/renderer/src/theme.ts | 10 ++-------- src/shared/theme.ts | 11 +++++++++++ 6 files changed, 35 insertions(+), 32 deletions(-) create mode 100644 src/shared/theme.ts diff --git a/CODE-AUDIT.md b/CODE-AUDIT.md index 3c28ddf..7a74d70 100644 --- a/CODE-AUDIT.md +++ b/CODE-AUDIT.md @@ -475,13 +475,13 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n other yt-dlp timeout ("Timed out …"). - [ ] **L48 — `cleanError` strips a leading `error:`** for live failures, while raw `ERROR:` text shows elsewhere (errorlog seed / terminal) — inconsistent error normalization. -- [ ] **L49 — Newlines in user-facing error strings.** download.ts missing-binary messages embed +- [x] **L49 — Newlines in user-facing error strings.** download.ts missing-binary messages embed `\n`, which renders awkwardly in inline/Caption error spans. - [x] **L50 — "Cookies saved" shown for 0 cookies.** Closing the sign-in window without logging in still writes the file and reports `ok` with `cookieCount: 0`. - [ ] **L51 — Scheduled daily sync time hardcoded to 09:00** (schedule.ts) — on/off toggle only, no time picker. - [ ] **L52 — Installer handoff is a fixed `setTimeout(app.quit, 1500)`** (updater.ts) — a race on slow machines. -- [ ] **L53 — `dialog.show*Dialog(win!, …)` non-null assertions** on a possibly-null window +- [x] **L53 — `dialog.show*Dialog(win!, …)` non-null assertions** on a possibly-null window (chooseFolder, backup export/import) — can throw if the sender window is gone. - [ ] **L54 — DownloadBar preview `` has no `onError` fallback** (inconsistent with `MediaThumb`'s retry). - [x] **L55 — QueueItem meta line can show size twice** — a probed-format `quality` label already @@ -491,7 +491,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n - [x] **L57 — Scheduling a past datetime silently downloads now** (`buildItem` future-only guard), no feedback that the schedule was ignored. - [x] **L58 — `chooseFolder` passes the macOS-only `createDirectory` property** — dead option on Windows. -- [ ] **L59 — `THEME_BACKGROUND` (index.ts) duplicates `pageBackground` (theme.ts)** — two +- [x] **L59 — `THEME_BACKGROUND` (index.ts) duplicates `pageBackground` (theme.ts)** — two hand-synced color constants (a "keep in sync" comment guards them). - [x] **L60 — `'external-url'` channel name lacks the `category:` prefix** every other IPC channel uses. - [ ] **L61 — `taskbarProgress` is the lone `ipcMain.on`** (fire-and-forget) amid all-`invoke` channels. diff --git a/src/main/backup.ts b/src/main/backup.ts index 5e051ae..9aa58f4 100644 --- a/src/main/backup.ts +++ b/src/main/backup.ts @@ -12,10 +12,11 @@ interface BackupFile { } export async function exportBackup(win: BrowserWindow | undefined): Promise { - const res = await dialog.showSaveDialog(win!, { + const opts = { defaultPath: 'aerofetch-backup.json', filters: [{ name: 'JSON', extensions: ['json'] }] - }) + } + const res = await (win ? dialog.showSaveDialog(win, opts) : dialog.showSaveDialog(opts)) if (res.canceled || !res.filePath) return { ok: false } // Strip credentials (proxy creds, API tokens) so a backup file shared or synced // to the cloud doesn't leak secrets in plaintext (M22). The user re-enters them @@ -32,10 +33,11 @@ export async function exportBackup(win: BrowserWindow | undefined): Promise { - const res = await dialog.showOpenDialog(win!, { - properties: ['openFile'], + const openOpts = { + properties: ['openFile' as const], filters: [{ name: 'JSON', extensions: ['json'] }] - }) + } + const res = await (win ? dialog.showOpenDialog(win, openOpts) : dialog.showOpenDialog(openOpts)) if (res.canceled || !res.filePaths[0]) return { ok: false } let parsed: unknown @@ -86,8 +88,8 @@ export async function importBackup(win: BrowserWindow | undefined): Promise 10 ? `\n…and ${commandTemplates.length - 10} more.` : '' - const choice = await dialog.showMessageBox(win!, { - type: 'warning', + const msgOpts = { + type: 'warning' as const, buttons: ['Enable custom commands', 'Import without enabling', 'Cancel'], defaultId: 1, cancelId: 2, @@ -99,7 +101,10 @@ export async function importBackup(win: BrowserWindow | undefined): Promise 0) { return { ok: false, - error: - `${missingBins.join(' and ')} not found in ${getBinDir()}\n` + - `Add the ffmpeg build's binaries to resources/bin/ (see the README there).` + error: `${missingBins.join(' and ')} not found in ${getBinDir()}. Add the ffmpeg build's binaries to resources/bin/ (see the README there).` } } // Reject anything that isn't an http(s) URL before it reaches yt-dlp's argv, diff --git a/src/main/index.ts b/src/main/index.ts index 0cd3401..b26ed97 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -22,6 +22,7 @@ import { type SystemThemeInfo, type TaskbarProgress } from '@shared/ipc' +import { PAGE_BACKGROUND } from '@shared/theme' import { getYtdlpVersion, updateYtdlp, runStartupYtdlpAutoUpdate } from './ytdlp' import { getFfmpegVersions } from './ffmpeg' import { checkForAppUpdate, downloadAppUpdate, runAppUpdate } from './updater' @@ -109,12 +110,6 @@ if (is.dev && devScript) { let mainWindow: BrowserWindow | null = null -// Page background per theme — keep in sync with `pageBackground` in -// src/renderer/src/theme.ts. Set as the window's NATIVE background so the -// one-frame compositor repaint (when a tooltip/dropdown overlay first paints on -// Windows) shows the app's current color instead of a mismatched white flash. -const THEME_BACKGROUND = { light: '#f7f7f8', dark: '#161618' } as const - // Resolve 'system' against the OS preference so callers always get a concrete color. function resolveBackgroundMode(theme: Settings['theme']): 'light' | 'dark' { return theme === 'system' ? (nativeTheme.shouldUseDarkColors ? 'dark' : 'light') : theme @@ -177,7 +172,7 @@ function createWindow(): void { minWidth: 640, minHeight: 480, show: false, - backgroundColor: THEME_BACKGROUND[resolveBackgroundMode(getSettings().theme)], + backgroundColor: PAGE_BACKGROUND[resolveBackgroundMode(getSettings().theme)], autoHideMenuBar: true, webPreferences: { preload: join(__dirname, '../preload/index.cjs'), @@ -342,7 +337,7 @@ function registerIpcHandlers(): void { // caption always matches the in-app theme (W3). Use the validated result. if (partial.theme) { BrowserWindow.fromWebContents(e.sender)?.setBackgroundColor( - THEME_BACKGROUND[resolveBackgroundMode(result.theme)] + PAGE_BACKGROUND[resolveBackgroundMode(result.theme)] ) applyNativeTheme(result.theme) } @@ -394,7 +389,7 @@ function registerIpcHandlers(): void { // background and title bar in sync the same way settingsSet does (W3). if (result.ok) { const theme = getSettings().theme - win?.setBackgroundColor(THEME_BACKGROUND[resolveBackgroundMode(theme)]) + win?.setBackgroundColor(PAGE_BACKGROUND[resolveBackgroundMode(theme)]) applyNativeTheme(theme) } return result @@ -461,7 +456,7 @@ function registerIpcHandlers(): void { function registerSystemThemeBridge(): void { nativeTheme.on('updated', () => { const info = getSystemThemeInfo() - const bg = THEME_BACKGROUND[resolveBackgroundMode(getSettings().theme)] + const bg = PAGE_BACKGROUND[resolveBackgroundMode(getSettings().theme)] for (const win of BrowserWindow.getAllWindows()) { win.webContents.send(IpcChannels.systemThemeUpdate, info) if (getSettings().theme === 'system') win.setBackgroundColor(bg) diff --git a/src/renderer/src/theme.ts b/src/renderer/src/theme.ts index 7c65b5f..1f666e0 100644 --- a/src/renderer/src/theme.ts +++ b/src/renderer/src/theme.ts @@ -1,3 +1,4 @@ +import { PAGE_BACKGROUND } from '@shared/theme' import { createLightTheme, createDarkTheme, @@ -168,14 +169,7 @@ export function getTheme(accent: AccentColor, mode: 'light' | 'dark'): Theme { return mode === 'dark' ? preset.dark : preset.light } -// Page background behind the app shell. Light: a clean, barely-cool off-white. -// Dark: neutral charcoal. Intentionally accent-INDEPENDENT — the accent shows up -// only on buttons/links, so this stays neutral whichever preset is chosen. -// Keep light/dark in sync with THEME_BACKGROUND in src/main/index.ts. -export const pageBackground = { - light: '#f7f7f8', - dark: '#161618' -} +export const pageBackground = PAGE_BACKGROUND // Per-kind thumbnail tints. Video vs audio differ by ramp DEPTH rather than a // competing hue, keeping a monochrome look. Theme-aware (light/dark) but diff --git a/src/shared/theme.ts b/src/shared/theme.ts new file mode 100644 index 0000000..7d7d07d --- /dev/null +++ b/src/shared/theme.ts @@ -0,0 +1,11 @@ +/** + * Native window background colors per theme mode. Light: barely-cool off-white. + * Dark: neutral charcoal. These match the renderer's page background so there's + * no flash when a compositor overlay (tooltip, dropdown) first renders on Windows. + * Intentionally accent-independent — the accent shows on buttons/links only. + * + * Shared between main/index.ts (BrowserWindow backgroundColor) and + * renderer/src/theme.ts (CSS backgroundColor on the root div) so the two never + * drift apart. + */ +export const PAGE_BACKGROUND = { light: '#f7f7f8', dark: '#161618' } as const