diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 179fa89..0c6f77d 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1320,10 +1320,12 @@ fn aggregate_media_byte_progress( ) -> Option<(u64, u64, bool)> { if track_changed { if let Some(total_bytes) = state.current_track_total_bytes { - let completed_downloaded = state - .current_track_downloaded_bytes - .unwrap_or(total_bytes) - .clamp(0.0, total_bytes.max(0.0)); + let completed_downloaded = state.current_track_downloaded_bytes.unwrap_or(total_bytes); + let completed_downloaded = if state.current_track_total_is_estimate { + completed_downloaded.max(0.0) + } else { + completed_downloaded.clamp(0.0, total_bytes.max(0.0)) + }; *state .completed_tracks_total_bytes .get_or_insert(0.0) += total_bytes; @@ -1339,14 +1341,38 @@ fn aggregate_media_byte_progress( state.current_track_total_bytes = None; state.current_track_total_is_estimate = false; } - state.current_track_total_bytes = progress.total_bytes; + // yt-dlp's estimated total is a moving bitrate-based guess. Keep the + // first total observed for a stream stable so the UI does not make the + // download's size appear to change on every progress update. A new media + // track gets its own stable total when split audio/video downloads switch + // tracks. + let next_total = progress + .total_bytes + .filter(|total_bytes| total_bytes.is_finite() && *total_bytes > 0.0); + if state.current_track_total_bytes.is_none() + || (state.current_track_total_is_estimate && !progress.total_is_estimate) + { + if let Some(total_bytes) = next_total { + state.current_track_total_bytes = Some(total_bytes); + state.current_track_total_is_estimate = progress.total_is_estimate; + } + } + let stable_total_bytes = state.current_track_total_bytes; state.current_track_downloaded_bytes = progress .downloaded_bytes .or_else(|| progress.total_bytes.map(|total| total * progress.fraction)) - .zip(progress.total_bytes) - .map(|(downloaded, total)| downloaded.clamp(0.0, total.max(0.0))) + .zip(stable_total_bytes) + .map(|(downloaded, total)| { + // An estimated total is only a display anchor. yt-dlp can + // legitimately download more bytes than that first estimate, + // so never turn a moving estimate into a false 100% byte count. + if state.current_track_total_is_estimate { + downloaded.max(0.0) + } else { + downloaded.clamp(0.0, total.max(0.0)) + } + }) .or(progress.downloaded_bytes); - state.current_track_total_is_estimate = progress.total_is_estimate; match ( state.completed_tracks_downloaded_bytes, @@ -1388,6 +1414,12 @@ fn emit_media_progress( } let byte_progress = aggregate_media_byte_progress(&progress, track_changed, state); let (speed, eta) = media_progress_speed(&progress, Instant::now(), &mut state.speed_sampler); + let size = byte_progress + .map(|(_, total, total_is_estimate)| { + let prefix = if total_is_estimate { "~" } else { "" }; + format!("{prefix}{}", crate::download::format_size(total as f64)) + }) + .or(progress.size); let now = Instant::now(); if now.duration_since(state.last_progress_at) >= MEDIA_PROGRESS_EMIT_INTERVAL { @@ -1398,7 +1430,7 @@ fn emit_media_progress( fraction: overall_fraction, speed, eta, - size: progress.size, + size, size_is_final: false, downloaded_bytes: byte_progress.map(|value| value.0 as f64), total_bytes: byte_progress.map(|value| value.1 as f64), @@ -7641,6 +7673,84 @@ mod tests { ); } + #[test] + fn freezes_a_media_track_estimate_across_progress_updates() { + let mut state = MediaProgressEmitterState::new(); + let first = MediaProgress { + fraction: 0.25, + speed: "-".to_string(), + eta: "-".to_string(), + size: Some("~100B".to_string()), + downloaded_bytes: Some(25.0), + total_bytes: Some(100.0), + total_is_estimate: true, + }; + let later = MediaProgress { + fraction: 0.75, + speed: "-".to_string(), + eta: "-".to_string(), + size: Some("~200B".to_string()), + downloaded_bytes: Some(150.0), + total_bytes: Some(200.0), + total_is_estimate: true, + }; + + assert_eq!( + aggregate_media_byte_progress(&first, false, &mut state), + Some((25, 100, true)) + ); + assert_eq!( + aggregate_media_byte_progress(&later, false, &mut state), + Some((150, 100, true)) + ); + + let exact = MediaProgress { + fraction: 0.9, + speed: "-".to_string(), + eta: "-".to_string(), + size: Some("200B".to_string()), + downloaded_bytes: Some(180.0), + total_bytes: Some(200.0), + total_is_estimate: false, + }; + assert_eq!( + aggregate_media_byte_progress(&exact, false, &mut state), + Some((180, 200, false)) + ); + } + + #[test] + fn preserves_estimated_bytes_when_a_split_track_exceeds_its_estimate() { + let mut state = MediaProgressEmitterState::new(); + let first = MediaProgress { + fraction: 0.75, + speed: "-".to_string(), + eta: "-".to_string(), + size: Some("~100B".to_string()), + downloaded_bytes: Some(150.0), + total_bytes: Some(100.0), + total_is_estimate: true, + }; + let second = MediaProgress { + fraction: 0.01, + speed: "-".to_string(), + eta: "-".to_string(), + size: Some("50B".to_string()), + downloaded_bytes: Some(1.0), + total_bytes: Some(50.0), + total_is_estimate: false, + }; + + assert_eq!( + aggregate_media_byte_progress(&first, false, &mut state), + Some((150, 100, true)) + ); + assert_eq!( + aggregate_media_byte_progress(&second, true, &mut state), + Some((151, 150, true)) + ); + } + #[test] fn derives_main_window_speed_from_downloaded_byte_delta() { let first = MediaProgress { diff --git a/src/App.tsx b/src/App.tsx index eafc2c5..7eb1e4c 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -198,7 +198,7 @@ function App() { {actionLabel} in 10 seconds. Cancel diff --git a/src/components/DeleteConfirmationModal.tsx b/src/components/DeleteConfirmationModal.tsx index 06d7333..21b132a 100644 --- a/src/components/DeleteConfirmationModal.tsx +++ b/src/components/DeleteConfirmationModal.tsx @@ -90,7 +90,7 @@ export const DeleteConfirmationModal: React.FC = () => { Cancel diff --git a/src/components/DownloadItem.tsx b/src/components/DownloadItem.tsx index 4887938..590de6e 100644 --- a/src/components/DownloadItem.tsx +++ b/src/components/DownloadItem.tsx @@ -85,25 +85,25 @@ export const DownloadItem = React.memo(({ - - + {hasDownloadedAmount ? ( + + {sizeDisplay.downloaded} + / + + ) : null} + + {hasDownloadedAmount + ? `${sizeDisplay.totalIsEstimate ? '~' : ''}${sizeDisplay.total} ${sizeDisplay.unit}` : completedSizeLabel} - aria-label={hasDownloadedAmount - ? `${sizeDisplay.downloaded} downloaded of ${sizeDisplay.totalIsEstimate ? 'approximately ' : ''}${sizeDisplay.total} ${sizeDisplay.unit}` - : completedSizeLabel} - > - {hasDownloadedAmount ? ( - <> - {sizeDisplay.downloaded} - / - - {sizeDisplay.totalIsEstimate ? '~' : ''}{sizeDisplay.total} {sizeDisplay.unit} - - > - ) : completedSizeLabel} diff --git a/src/components/DownloadTable.tsx b/src/components/DownloadTable.tsx index 3554091..2ba1d96 100644 --- a/src/components/DownloadTable.tsx +++ b/src/components/DownloadTable.tsx @@ -538,7 +538,7 @@ export const DownloadTable: React.FC = ({ filter }) => { className={`${index === 5 ? 'download-cell-right' : ''} cursor-pointer hover:text-text-primary transition-colors flex items-center justify-between`} onClick={() => handleSort(label as DownloadSortColumn)} > - + {label} {(isQueueFilter ? queueSortConfig : sortConfig)?.column === label && ( (isQueueFilter ? queueSortConfig : sortConfig)?.direction === 'asc' diff --git a/src/components/DuplicateResolutionModal.tsx b/src/components/DuplicateResolutionModal.tsx index 11f7f68..7bd7ca1 100644 --- a/src/components/DuplicateResolutionModal.tsx +++ b/src/components/DuplicateResolutionModal.tsx @@ -68,7 +68,7 @@ export const DuplicateResolutionModal = ({ conflicts: initialConflicts, onConfir - + Cancel { setSelectedPropertiesDownloadId(null)} - className="app-button border-transparent bg-transparent px-4 text-xs text-text-secondary" + className="app-button app-button-cancel px-4 text-xs" > Cancel diff --git a/src/index.css b/src/index.css index 10fd99b..1f1c361 100644 --- a/src/index.css +++ b/src/index.css @@ -441,7 +441,7 @@ html[data-list-density="relaxed"] { color: hsl(var(--text-primary)); font-weight: 500; font-size: 13px; - transition: background-color 100ms ease, transform 100ms ease; + transition: background-color 100ms ease, border-color 100ms ease, box-shadow 100ms ease, transform 100ms ease; box-shadow: 0 1px 1px hsl(var(--shadow-color)); } @@ -464,6 +464,28 @@ html[data-list-density="relaxed"] { background: hsl(var(--accent-color) / 0.9); } + .app-button-cancel { + border-color: hsl(var(--border-modal)); + background: + linear-gradient(180deg, hsl(0 0% 100% / 0.055), transparent), + hsl(var(--bg-input) / 0.88); + color: hsl(var(--text-secondary)); + box-shadow: + inset 0 1px 0 hsl(0 0% 100% / 0.055), + 0 1px 2px hsl(var(--shadow-color)); + } + + .app-button-cancel:hover:not(:disabled) { + border-color: hsl(var(--text-muted) / 0.65); + background: + linear-gradient(hsl(var(--item-hover)), hsl(var(--item-hover))), + hsl(var(--bg-input)); + color: hsl(var(--text-primary)); + box-shadow: + inset 0 1px 0 hsl(0 0% 100% / 0.07), + 0 2px 5px hsl(var(--shadow-color)); + } + .app-icon-button { display: inline-flex; width: 28px; @@ -844,12 +866,25 @@ html[data-list-density="relaxed"] { } .add-download-button-cancel { + border-color: hsl(var(--border-modal)); + background: + linear-gradient(180deg, hsl(0 0% 100% / 0.055), transparent), + hsl(var(--bg-input) / 0.88); color: hsl(var(--text-secondary)); + box-shadow: + inset 0 1px 0 hsl(0 0% 100% / 0.055), + 0 1px 2px hsl(var(--shadow-color)); } .add-download-button-cancel:hover:not(:disabled) { - background: hsl(var(--item-hover)); + border-color: hsl(var(--text-muted) / 0.65); + background: + linear-gradient(hsl(var(--item-hover)), hsl(var(--item-hover))), + hsl(var(--bg-input)); color: hsl(var(--text-primary)); + box-shadow: + inset 0 1px 0 hsl(0 0% 100% / 0.07), + 0 2px 5px hsl(var(--shadow-color)); } .add-download-button-primary { @@ -982,11 +1017,27 @@ html[data-list-density="relaxed"] { box-shadow: none; } + :is(.theme-dark, .theme-dracula, .theme-nord) .add-download-button-cancel { + border-color: hsl(var(--add-keyline)); + background: hsl(var(--add-section-bg)); + box-shadow: + inset 0 1px 0 hsl(0 0% 100% / 0.055), + 0 1px 2px hsl(var(--add-shadow)); + } + :is(.theme-dark, .theme-dracula, .theme-nord) .add-download-button-secondary:hover:not(:disabled) { border-color: hsl(var(--add-keyline-strong)); background: hsl(var(--item-hover)); } + :is(.theme-dark, .theme-dracula, .theme-nord) .add-download-button-cancel:hover:not(:disabled) { + border-color: hsl(var(--add-keyline-strong)); + background: hsl(var(--item-hover)); + box-shadow: + inset 0 1px 0 hsl(0 0% 100% / 0.07), + 0 2px 5px hsl(var(--add-shadow)); + } + :is(.theme-dark, .theme-dracula, .theme-nord) .add-download-button-primary { border-color: hsl(var(--accent-color)); background: hsl(var(--accent-color)); @@ -1980,6 +2031,39 @@ html[data-list-density="relaxed"] .download-ghost-row { overflow: hidden; } +.download-size-cell { + justify-content: flex-end; + text-align: right; +} + +.download-size-cell > .download-size-progress { + flex: 1 1 auto; + text-align: right; +} + +.download-size-cell > .download-size-total { + flex: 0 0 auto; + text-align: left; + white-space: nowrap; +} + +.download-size-cell > .download-size-progress, +.download-size-cell > .download-size-total { + max-width: 100%; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.download-size-cell > .download-size-progress { + text-align: right; +} + +.download-row > .download-size-cell { + padding-right: calc(var(--download-column-padding-x) + 6px); +} + .download-cell-truncate > span, .download-cell-right > span { display: block; diff --git a/src/store/downloadStore.test.ts b/src/store/downloadStore.test.ts index c87b4f5..1f136bd 100644 --- a/src/store/downloadStore.test.ts +++ b/src/store/downloadStore.test.ts @@ -1,6 +1,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { initDownloadListener, useDownloadProgressStore } from './downloadStore'; -import { useDownloadStore } from './useDownloadStore'; +import { + clearDownloadControlIntents, + setDownloadControlIntent, + useDownloadStore +} from './useDownloadStore'; import * as ipc from '../ipc'; vi.mock('../ipc', () => ({ @@ -13,6 +17,7 @@ describe('useDownloadProgressStore', () => { vi.clearAllMocks(); vi.mocked(ipc.invokeCommand).mockResolvedValue(undefined); useDownloadProgressStore.setState({ progressMap: {} }); + clearDownloadControlIntents(); }); it('prunes terminal progress entries', () => { @@ -233,4 +238,70 @@ describe('useDownloadProgressStore', () => { expect(useDownloadStore.getState().downloads[0].status).toBe('completed'); release(); }); + + it('ignores a stale paused event during resume and accepts the new active state', async () => { + const handlers: Record void> = {}; + vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => { + handlers[event] = handler as (event: any) => void; + return Promise.resolve(vi.fn()); + }); + useDownloadStore.setState({ + downloads: [{ + id: 'resume-race', + url: 'https://example.com/file', + fileName: 'file.bin', + status: 'queued', + category: 'Other', + dateAdded: '' + }] + }); + setDownloadControlIntent('resume-race', 'resume'); + + const release = await initDownloadListener(); + handlers['download-state']({ payload: { + id: 'resume-race', + status: 'paused' + } }); + expect(useDownloadStore.getState().downloads[0].status).toBe('queued'); + + handlers['download-state']({ payload: { + id: 'resume-race', + status: 'downloading' + } }); + expect(useDownloadStore.getState().downloads[0].status).toBe('downloading'); + release(); + }); + + it('allows a later genuine pause after consuming the stale resume event', async () => { + const handlers: Record void> = {}; + vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => { + handlers[event] = handler as (event: any) => void; + return Promise.resolve(vi.fn()); + }); + useDownloadStore.setState({ + downloads: [{ + id: 'resume-pause-race', + url: 'https://example.com/file', + fileName: 'file.bin', + status: 'queued', + category: 'Other', + dateAdded: '' + }] + }); + setDownloadControlIntent('resume-pause-race', 'resume'); + + const release = await initDownloadListener(); + handlers['download-state']({ payload: { + id: 'resume-pause-race', + status: 'paused' + } }); + expect(useDownloadStore.getState().downloads[0].status).toBe('queued'); + + handlers['download-state']({ payload: { + id: 'resume-pause-race', + status: 'paused' + } }); + expect(useDownloadStore.getState().downloads[0].status).toBe('paused'); + release(); + }); }); diff --git a/src/store/downloadStore.ts b/src/store/downloadStore.ts index 1f1704b..fde59bd 100644 --- a/src/store/downloadStore.ts +++ b/src/store/downloadStore.ts @@ -5,7 +5,11 @@ import type { DownloadItem } from '../bindings/DownloadItem'; import { categoryForFileName } from '../utils/downloads'; import { useDownloadProgressStore } from './downloadProgressStore'; -import { useDownloadStore } from './useDownloadStore'; +import { + clearDownloadControlIntent, + downloadControlIntentFor, + useDownloadStore +} from './useDownloadStore'; export { useDownloadProgressStore } from './downloadProgressStore'; @@ -78,6 +82,28 @@ const startDownloadListeners = async () => { } const status = payload.status as DownloadStatus; + // resume_download queues the row before the backend can emit its new + // active state. A paused event already emitted by the old lifecycle may + // arrive in that gap. Do not let it overwrite the queued transition; + // otherwise the guard below would reject the legitimate downloading + // event and leave the row visibly paused forever. + if (status === 'paused' && + current.status === 'queued' && + downloadControlIntentFor(payload.id) === 'resume') { + // Consume only the stale pause event that caused the resume race. + // A later real pause must be allowed through even if the backend has + // not emitted a new active state yet. + clearDownloadControlIntent(payload.id, 'resume'); + return; + } + if (status === 'downloading' || status === 'processing' || + status === 'completed' || status === 'failed') { + clearDownloadControlIntent(payload.id, 'resume'); + } + if (status === 'paused') { + clearDownloadControlIntent(payload.id, 'pause'); + } + // Prevent stale lifecycle events from moving a paused row back into an // active state. A pause request can finish before one already-emitted // worker event reaches the frontend. Resume paths set the row to queued diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index 4c0e87e..43d7dac 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -24,6 +24,29 @@ const queueStartPromises = new Map>(); const queueControlGenerations = new Map(); let pendingStartupResume: Promise | null = null; +type DownloadControlIntent = 'pause' | 'resume'; +const downloadControlIntents = new Map(); + +// State events do not carry a lifecycle generation. Keep the intent that +// initiated a control transition long enough for the listener to discard an +// already-emitted event from the previous transition. +export const setDownloadControlIntent = (id: string, intent: DownloadControlIntent): void => { + downloadControlIntents.set(id, intent); +}; + +export const clearDownloadControlIntent = (id: string, intent?: DownloadControlIntent): void => { + if (intent === undefined || downloadControlIntents.get(id) === intent) { + downloadControlIntents.delete(id); + } +}; + +export const downloadControlIntentFor = (id: string): DownloadControlIntent | undefined => + downloadControlIntents.get(id); + +export const clearDownloadControlIntents = (): void => { + downloadControlIntents.clear(); +}; + const waitForPendingStartupResume = async (): Promise => { const pending = pendingStartupResume; if (pending) await pending.catch(() => undefined); @@ -871,6 +894,7 @@ export const useDownloadStore = create((set, get) => ({ }, removeDownload: async (id, deleteFile = false, preserveResumable = false) => { await waitForPendingStartupResume(); + clearDownloadControlIntent(id); const { pendingDispatch } = await invalidateDispatch(id); if (pendingDispatch) { await pendingDispatch; @@ -898,17 +922,23 @@ export const useDownloadStore = create((set, get) => ({ }, pauseDownload: async (id) => { await waitForPendingStartupResume(); + if (!get().downloads.some(download => download.id === id)) return; + setDownloadControlIntent(id, 'pause'); const { generation, pendingDispatch } = await invalidateDispatch(id); - if (pendingDispatch) { - await pendingDispatch; - } + try { + if (pendingDispatch) { + await pendingDispatch; + } - await invoke('pause_download', { id }); + await invoke('pause_download', { id }); - if (!isCurrentDownloadLifecycle(id, generation)) return; - const current = get().downloads.find(download => download.id === id); - if (current && current.status !== 'completed' && current.status !== 'failed') { - get().updateDownload(id, { status: 'paused', speed: '-', eta: '-' }); + if (!isCurrentDownloadLifecycle(id, generation)) return; + const current = get().downloads.find(download => download.id === id); + if (current && current.status !== 'completed' && current.status !== 'failed') { + get().updateDownload(id, { status: 'paused', speed: '-', eta: '-' }); + } + } finally { + clearDownloadControlIntent(id, 'pause'); } }, redownload: async (id) => { @@ -925,6 +955,7 @@ export const useDownloadStore = create((set, get) => ({ const url = targetItem.url?.trim(); if (!url) throw new Error('Cannot redownload: original URL is missing.'); + setDownloadControlIntent(id, 'resume'); await invalidateAndWaitForDispatch(id); // Remove from backend to clear its state and delete the existing file so we can overwrite @@ -933,6 +964,7 @@ export const useDownloadStore = create((set, get) => ({ get().unregisterBackendIds([id]); } catch (e) { console.warn("Could not remove old download from backend", e); + clearDownloadControlIntent(id, 'resume'); throw e; } @@ -951,6 +983,7 @@ export const useDownloadStore = create((set, get) => ({ if (!await dispatchItem(id)) { console.error("Failed to enqueue redownload"); get().updateDownload(id, { status: 'failed' }); + clearDownloadControlIntent(id, 'resume'); } else { get().updateDownload(id, { hasBeenDispatched: true }); info(`Download ${id} redownloaded (queued)`); @@ -961,6 +994,7 @@ export const useDownloadStore = create((set, get) => ({ const targetItem = get().downloads.find(d => d.id === id); if (!targetItem) return false; + setDownloadControlIntent(id, 'resume'); try { if (targetItem.status === 'ready' || targetItem.status === 'staged') { get().updateDownload(id, { status: 'queued', hasBeenDispatched: true }); @@ -968,6 +1002,7 @@ export const useDownloadStore = create((set, get) => ({ return true; } get().updateDownload(id, { status: targetItem.status }); + clearDownloadControlIntent(id, 'resume'); return false; } @@ -1002,11 +1037,13 @@ export const useDownloadStore = create((set, get) => ({ } else { console.error("Failed to re-enqueue for resume"); get().updateDownload(id, { status: prevStatus }); + clearDownloadControlIntent(id, 'resume'); return false; } } catch (e) { console.error("Failed to resume download:", e); get().updateDownload(id, { status: targetItem.status }); + clearDownloadControlIntent(id, 'resume'); return false; } },