ui: data-id queue lookup + persisted active tab + Esc/Ctrl+N shortcuts
Renderer-side polish bundle. - updateQueueItemProgress now looks up items by [data-id] selector instead of array index. Resilient against queue/DOM divergence between renders. Determinate vs indeterminate progress logic tightened. - Active tab persisted to localStorage on every showTab; restored on init via loadPersistedActiveTab (whitelisted to known tab IDs so a future rename cannot strand the user on a missing tab). Page title now only shows the streamer name on the VODs tab — it no longer leaks into Settings / Cutter / Merge. - Escape closes the topmost open modal regardless of focus (clip dialog, template guide, update modal — in that priority order). - Ctrl+1..5 (Cmd+1..5 on macOS) jumps directly to a tab. The existing Del (delete selected) and S (start/pause) shortcuts still work and remain blocked while typing in inputs. Adds docs/IMPROVEMENT_LOG.md (new, single dated section for this cycle). Build: tsc clean. Full smoke suite green (failures: [], runtimeIssues: []). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8d0cb4cefd
commit
feebfc86a1
45
docs/IMPROVEMENT_LOG.md
Normal file
45
docs/IMPROVEMENT_LOG.md
Normal file
@ -0,0 +1,45 @@
|
|||||||
|
# Improvement Log
|
||||||
|
|
||||||
|
Dated entries from improvement cycles. Newest at top.
|
||||||
|
|
||||||
|
## 2026-05-03 — Cycle 1: stability & UX polish
|
||||||
|
|
||||||
|
Three independent improvements landed this cycle.
|
||||||
|
|
||||||
|
### 1. Atomic file writes survive power loss / crash mid-write (correctness)
|
||||||
|
|
||||||
|
- **Files**: `src/main.ts` — new `writeFileAtomicSync` helper, `saveConfig`, `writeQueueToDisk`.
|
||||||
|
- **Problem**: `saveConfig` and `writeQueueToDisk` used `writeFileSync` + `renameSync`. Node's `writeFileSync` does NOT call `fsync` — the OS may report the rename complete while the file content still sits in the write cache. A power loss / kernel panic between `writeFileSync` and `renameSync` could leave the renamed file empty or truncated. On next launch, `JSON.parse` throws and the app silently falls back to defaults (config) or `[]` (queue). Users would see "settings reset" / "queue lost" with no diagnostic in the debug log beyond a `console.error`.
|
||||||
|
- **Fix**: `openSync(tmp, 'w')` → `writeSync(fd, buffer, 0, len, 0)` → `fsyncSync(fd)` → `closeSync(fd)` → `renameSync`. The `fsyncSync` is wrapped in an inner try (some filesystems reject it, e.g. network shares); failure there is non-fatal but the close + rename order is always preserved. The Windows copy/unlink fallback for "rename failed because target locked" is kept.
|
||||||
|
|
||||||
|
### 2. Per-item filename claims fix parallel-download race (race condition + dead-code cleanup)
|
||||||
|
|
||||||
|
- **Files**: `src/main.ts` — `ensureUniqueFilename`, new `releaseClaimedFilenamesForItem`, every download call site, `splitMergedFile` signature.
|
||||||
|
- **Problem**: `claimedFilenames` was a global `Set<string>` and `processOneQueueItem` did `claimedFilenames.clear()` in its `finally`. With parallel downloads enabled (max 2), when item A finished, the `clear()` wiped item B's reservations too. In the narrow window between B claiming a filename via `ensureUniqueFilename` and streamlink actually writing the first bytes to disk, a third item entering the freed slot could compute the SAME filename (claim set empty, file not yet on disk) → both downloads would race writing the same path. The dead `releaseClaimedFilename(filePath)` function was defined at line 722 but never called from anywhere.
|
||||||
|
- **Fix**: New `Map<itemId, Set<filename>>` tracks which item claimed which filenames. `ensureUniqueFilename(filePath, itemId)` registers per-item; `releaseClaimedFilenamesForItem(itemId)` removes only that item's claims. `splitMergedFile` gained an `itemId` parameter so split-phase claims register correctly. The dead `releaseClaimedFilename` is gone, replaced by the per-item variant.
|
||||||
|
|
||||||
|
### 3. Renderer UX polish — robust progress lookup, persisted active tab, keyboard shortcuts (client-side feature)
|
||||||
|
|
||||||
|
- **Files**: `src/renderer-queue.ts`, `src/renderer.ts`.
|
||||||
|
- **Problem(s)** (small wins bundled as one coherent UX improvement):
|
||||||
|
- `updateQueueItemProgress` indexed `byId('queueList').children[idx]` by array position — fragile if the queue array and DOM ever diverged for a frame (queue mutated after render-fingerprint shortcut, or during the throttled queue-sync window).
|
||||||
|
- The active tab always reset to `vods` on app launch — annoying for users who live in `settings`, `cutter`, or `merge`.
|
||||||
|
- No way to dismiss any of the three modals (`clipModal`, `templateGuideModal`, `updateModal`) without clicking the close button.
|
||||||
|
- No keyboard navigation between tabs (only `Del` and `S` were wired).
|
||||||
|
- The page title used to show the streamer name even when the user was on Settings or Cutter, because `showTab` always preferred `currentStreamer` over the tab title.
|
||||||
|
- **Fix**:
|
||||||
|
- Look up queue items by `[data-id="..."]` selector instead of array index. Resilient to mutation between renders. Determinate / indeterminate progress class logic tightened (`isDeterminate = progress > 0 && progress <= 100`).
|
||||||
|
- Active tab persisted to `localStorage` on every `showTab`; restored on init via `loadPersistedActiveTab`, whitelisted to known tab IDs (`vods | clips | cutter | merge | settings`) so a future rename can't strand users on a missing tab. Title logic fixed: streamer name only appears in the page title when the VODs tab is active.
|
||||||
|
- `Escape` closes the topmost open modal regardless of focus (priority order: clip dialog → template guide → update modal). Works while typing in a modal input.
|
||||||
|
- `Ctrl+1..5` (or `Cmd+1..5` on macOS) jumps directly to a tab. Existing `Del` (delete selected) and `S` (start/pause) shortcuts continue to work and remain blocked while typing in inputs.
|
||||||
|
|
||||||
|
### Regression
|
||||||
|
|
||||||
|
- `npm run build` — clean (TypeScript strict, 0 errors, 0 new warnings).
|
||||||
|
- `node scripts/smoke-test-update-version-logic.js` — passed.
|
||||||
|
- `node scripts/smoke-test-merge-split-logic.js` — passed.
|
||||||
|
- `node scripts/smoke-test.js` — passed (37 VODs listed, queue add OK, preflight green, `issues: []`).
|
||||||
|
- `node scripts/smoke-test-template-guide.js` — passed (17 variable rows, live preview reactive, `failures: []`).
|
||||||
|
- `node scripts/smoke-test-full.js` — passed (`failures: []`, `runtimeIssues: []`; flows verified: language switch, queue add, duplicate prevention, runtime metrics, clip queue, pause/resume, retry, reorder, media cut/merge, update check).
|
||||||
|
|
||||||
|
ESLint reports 36 pre-existing warnings and 1 pre-existing error (control-character regex in `sanitizeFilenamePart`); none new from this cycle.
|
||||||
@ -180,22 +180,27 @@ async function createMergeGroupFromSelection(): Promise<void> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function updateQueueItemProgress(progress: DownloadProgress): void {
|
function updateQueueItemProgress(progress: DownloadProgress): void {
|
||||||
const items = byId('queueList').children;
|
// Lookup by data-id attribute, not array index — survives queue mutation between renders
|
||||||
const idx = queue.findIndex(i => i.id === progress.id);
|
const safeId = String(progress.id ?? '').replace(/"/g, '\\"');
|
||||||
if (idx < 0 || idx >= items.length) return;
|
if (!safeId) return;
|
||||||
|
const el = byId('queueList').querySelector(`[data-id="${safeId}"]`) as HTMLElement | null;
|
||||||
|
if (!el) return;
|
||||||
|
|
||||||
const el = items[idx];
|
const item = queue.find(i => i.id === progress.id);
|
||||||
const bar = el.querySelector('.queue-progress-bar') as HTMLElement;
|
if (!item) return;
|
||||||
const text = el.querySelector('.queue-progress-text') as HTMLElement;
|
|
||||||
const meta = el.querySelector('.queue-meta') as HTMLElement;
|
const bar = el.querySelector('.queue-progress-bar') as HTMLElement | null;
|
||||||
|
const text = el.querySelector('.queue-progress-text') as HTMLElement | null;
|
||||||
|
const meta = el.querySelector('.queue-meta') as HTMLElement | null;
|
||||||
|
|
||||||
if (bar) {
|
if (bar) {
|
||||||
const pct = progress.progress > 0 ? Math.min(100, progress.progress) : 0;
|
const isDeterminate = progress.progress > 0 && progress.progress <= 100;
|
||||||
|
const pct = isDeterminate ? Math.min(100, progress.progress) : 0;
|
||||||
bar.style.width = `${pct}%`;
|
bar.style.width = `${pct}%`;
|
||||||
bar.className = `queue-progress-bar${progress.progress <= 0 ? ' indeterminate' : ''}`;
|
bar.className = `queue-progress-bar${isDeterminate ? '' : ' indeterminate'}`;
|
||||||
}
|
}
|
||||||
if (text) text.textContent = getQueueProgressText(queue[idx]);
|
if (text) text.textContent = getQueueProgressText(item);
|
||||||
if (meta) meta.textContent = getQueueMetaText(queue[idx]);
|
if (meta) meta.textContent = getQueueMetaText(item);
|
||||||
}
|
}
|
||||||
|
|
||||||
function toggleQueueDetails(id: string): void {
|
function toggleQueueDetails(id: string): void {
|
||||||
|
|||||||
@ -45,6 +45,9 @@ async function init(): Promise<void> {
|
|||||||
initQueueDragDrop();
|
initQueueDragDrop();
|
||||||
updateDownloadButtonState();
|
updateDownloadButtonState();
|
||||||
|
|
||||||
|
// Restore last active tab from previous session (default 'vods')
|
||||||
|
showTab(loadPersistedActiveTab());
|
||||||
|
|
||||||
window.api.onQueueUpdated((q: QueueItem[]) => {
|
window.api.onQueueUpdated((q: QueueItem[]) => {
|
||||||
queue = mergeQueueState(Array.isArray(q) ? q : []);
|
queue = mergeQueueState(Array.isArray(q) ? q : []);
|
||||||
renderQueue();
|
renderQueue();
|
||||||
@ -126,9 +129,28 @@ async function init(): Promise<void> {
|
|||||||
|
|
||||||
// Keyboard shortcuts
|
// Keyboard shortcuts
|
||||||
document.addEventListener('keydown', (e) => {
|
document.addEventListener('keydown', (e) => {
|
||||||
// Skip if user is typing in an input field
|
// Esc closes any open modal — works regardless of focus, so users can dismiss
|
||||||
|
// a modal that took focus from inside an input field
|
||||||
|
if (e.key === 'Escape') {
|
||||||
|
if (closeTopmostOpenModal()) {
|
||||||
|
e.preventDefault();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Skip rest if user is typing in an input field
|
||||||
if (e.target instanceof HTMLInputElement || e.target instanceof HTMLTextAreaElement || e.target instanceof HTMLSelectElement) return;
|
if (e.target instanceof HTMLInputElement || e.target instanceof HTMLTextAreaElement || e.target instanceof HTMLSelectElement) return;
|
||||||
|
|
||||||
|
// Ctrl+1..5 jumps directly to a tab (Cmd on macOS via metaKey)
|
||||||
|
if ((e.ctrlKey || e.metaKey) && !e.altKey && !e.shiftKey && e.key >= '1' && e.key <= '5') {
|
||||||
|
const tabIndex = parseInt(e.key, 10) - 1;
|
||||||
|
if (tabIndex >= 0 && tabIndex < TAB_IDS.length) {
|
||||||
|
e.preventDefault();
|
||||||
|
showTab(TAB_IDS[tabIndex]);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (e.key === 'Delete' && selectedQueueIds.length > 0) {
|
if (e.key === 'Delete' && selectedQueueIds.length > 0) {
|
||||||
// Delete selected queue items
|
// Delete selected queue items
|
||||||
const idsToRemove = [...selectedQueueIds];
|
const idsToRemove = [...selectedQueueIds];
|
||||||
@ -150,6 +172,29 @@ async function init(): Promise<void> {
|
|||||||
scheduleQueueSync(QUEUE_SYNC_DEFAULT_MS);
|
scheduleQueueSync(QUEUE_SYNC_DEFAULT_MS);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function closeTopmostOpenModal(): boolean {
|
||||||
|
// Try each known modal in priority order: clip dialog, template guide, update modal
|
||||||
|
const clipModal = document.getElementById('clipModal');
|
||||||
|
if (clipModal?.classList.contains('show')) {
|
||||||
|
closeClipDialog();
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
const templateGuideModal = document.getElementById('templateGuideModal');
|
||||||
|
if (templateGuideModal?.classList.contains('show')) {
|
||||||
|
closeTemplateGuide();
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
const updateModal = document.getElementById('updateModal');
|
||||||
|
if (updateModal?.classList.contains('show')) {
|
||||||
|
dismissUpdateModal();
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
function formatBytesRenderer(bytes: number): string {
|
function formatBytesRenderer(bytes: number): string {
|
||||||
if (bytes < 1024) return `${bytes} B`;
|
if (bytes < 1024) return `${bytes} B`;
|
||||||
if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB`;
|
if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB`;
|
||||||
@ -330,16 +375,48 @@ async function syncQueueAndDownloadState(): Promise<void> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const TAB_IDS = ['vods', 'clips', 'cutter', 'merge', 'settings'] as const;
|
||||||
|
const ACTIVE_TAB_STORAGE_KEY = 'twitch-vod-manager:active-tab';
|
||||||
|
|
||||||
|
function isKnownTab(value: string): value is typeof TAB_IDS[number] {
|
||||||
|
return (TAB_IDS as readonly string[]).includes(value);
|
||||||
|
}
|
||||||
|
|
||||||
|
function loadPersistedActiveTab(): string {
|
||||||
|
try {
|
||||||
|
const stored = localStorage.getItem(ACTIVE_TAB_STORAGE_KEY);
|
||||||
|
if (stored && isKnownTab(stored)) return stored;
|
||||||
|
} catch { /* localStorage may be unavailable in privacy modes */ }
|
||||||
|
return 'vods';
|
||||||
|
}
|
||||||
|
|
||||||
|
function persistActiveTab(tab: string): void {
|
||||||
|
if (!isKnownTab(tab)) return;
|
||||||
|
try { localStorage.setItem(ACTIVE_TAB_STORAGE_KEY, tab); } catch { }
|
||||||
|
}
|
||||||
|
|
||||||
function showTab(tab: string): void {
|
function showTab(tab: string): void {
|
||||||
queryAll('.nav-item').forEach((i) => i.classList.remove('active'));
|
queryAll('.nav-item').forEach((i) => i.classList.remove('active'));
|
||||||
queryAll('.tab-content').forEach((c) => c.classList.remove('active'));
|
queryAll('.tab-content').forEach((c) => c.classList.remove('active'));
|
||||||
|
|
||||||
query(`.nav-item[data-tab="${tab}"]`).classList.add('active');
|
const navItem = query(`.nav-item[data-tab="${tab}"]`);
|
||||||
|
if (!navItem) {
|
||||||
|
// Unknown tab — fall back to vods so the user is never stuck on an empty screen
|
||||||
|
showTab('vods');
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
navItem.classList.add('active');
|
||||||
byId(tab + 'Tab').classList.add('active');
|
byId(tab + 'Tab').classList.add('active');
|
||||||
|
|
||||||
const titles: Record<string, string> = UI_TEXT.tabs;
|
const titles: Record<string, string> = UI_TEXT.tabs;
|
||||||
|
|
||||||
byId('pageTitle').textContent = currentStreamer || titles[tab] || UI_TEXT.appName;
|
// Only show the streamer name on the VODs tab — otherwise the title would
|
||||||
|
// mismatch the tab content (e.g. "streamer X" while on Settings)
|
||||||
|
byId('pageTitle').textContent = (tab === 'vods' && currentStreamer)
|
||||||
|
? currentStreamer
|
||||||
|
: (titles[tab] || UI_TEXT.appName);
|
||||||
|
|
||||||
|
persistActiveTab(tab);
|
||||||
}
|
}
|
||||||
|
|
||||||
function parseDurationToSeconds(durStr: string): number {
|
function parseDurationToSeconds(durStr: string): number {
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user