fix(ui): harden modal lifecycle and keychain access

This commit is contained in:
NimBold
2026-07-27 19:23:29 +03:30
parent bff7d2782c
commit eb73dde911
14 changed files with 318 additions and 48 deletions
+12 -2
View File
@@ -50,6 +50,7 @@ import {
type AddDownloadDraftRow,
type MediaSelection
} from '../utils/addDownloadMetadata';
import { isTopmostModal, useModalFocus } from '../hooks/useModalFocus';
const formatBytes = (bytes: number) => {
const k = 1024;
@@ -165,6 +166,7 @@ export const AddDownloadsModal = () => {
const [conflicts, setConflicts] = useState<DuplicateConflict[]>([]);
const [showingDuplicates, setShowingDuplicates] = useState(false);
const modalRef = useModalFocus(isAddModalOpen);
const [pendingAction, setPendingAction] = useState<AddDownloadAction>({ type: 'start-now' });
const [pendingUseSharedDestination, setPendingUseSharedDestination] = useState(false);
const [pendingDestinationOverrides, setPendingDestinationOverrides] = useState<Record<number, string>>({});
@@ -363,7 +365,9 @@ export const AddDownloadsModal = () => {
if (!isAddModalOpen) return;
const closeOnEscape = (event: KeyboardEvent) => {
if (event.key !== 'Escape') return;
if (!isTopmostModal(modalRef.current)) return;
if (showKeychainModal) return;
event.preventDefault();
if (showingDuplicates) {
setShowingDuplicates(false);
} else if (isQueueMenuOpen) {
@@ -1583,8 +1587,14 @@ export const AddDownloadsModal = () => {
}}
role="dialog"
aria-modal="true"
aria-labelledby="add-downloads-modal-title"
>
<div className="app-modal add-download-modal flex flex-col overflow-hidden text-sm">
<div
ref={modalRef}
tabIndex={-1}
data-modal-surface="true"
className="app-modal add-download-modal flex flex-col overflow-hidden text-sm"
>
{/* Main Content Split */}
<div className="flex flex-1 min-h-0 overflow-hidden">
@@ -1597,7 +1607,7 @@ export const AddDownloadsModal = () => {
<div className="flex items-center justify-between">
<div className="add-download-section-title flex items-center gap-2">
<Link size={16} className="text-blue-500" />
{t($ => $.addDownloads.downloadLinks)}
<span id="add-downloads-modal-title">{t($ => $.addDownloads.downloadLinks)}</span>
</div>
</div>
<textarea
+13 -4
View File
@@ -2,12 +2,14 @@ import React, { useState, useEffect } from 'react';
import { useDownloadStore } from '../store/useDownloadStore';
import { AlertTriangle } from 'lucide-react';
import { useTranslation } from 'react-i18next';
import { isTopmostModal, useModalFocus } from '../hooks/useModalFocus';
export const DeleteConfirmationModal: React.FC = () => {
const { t } = useTranslation();
const { deleteModalState, closeDeleteModal, removeDownload } = useDownloadStore();
const [errorMessage, setErrorMessage] = useState('');
const [isRemoving, setIsRemoving] = useState(false);
const modalRef = useModalFocus(deleteModalState.isOpen);
useEffect(() => {
if (deleteModalState.isOpen) {
@@ -19,7 +21,10 @@ export const DeleteConfirmationModal: React.FC = () => {
useEffect(() => {
if (!deleteModalState.isOpen || isRemoving) return;
const handleEscape = (event: KeyboardEvent) => {
if (event.key === 'Escape') closeDeleteModal();
if (event.key === 'Escape' && isTopmostModal(modalRef.current)) {
event.preventDefault();
closeDeleteModal();
}
};
window.addEventListener('keydown', handleEscape);
return () => window.removeEventListener('keydown', handleEscape);
@@ -70,22 +75,26 @@ export const DeleteConfirmationModal: React.FC = () => {
return (
<div
className="app-modal-backdrop fixed inset-0 z-50 flex items-center justify-center bg-black/40"
className="app-modal-backdrop fixed inset-0 z-50 flex items-center justify-center"
onClick={(event) => {
if (event.target === event.currentTarget && !isRemoving) handleCancel();
}}
role="dialog"
aria-modal="true"
aria-labelledby="remove-download-title"
>
<div
className="window-safe-modal bg-bg-modal rounded-xl w-full max-w-md overflow-hidden flex flex-col shadow-2xl border border-border-modal scale-in"
ref={modalRef}
tabIndex={-1}
data-modal-surface="true"
className="app-modal keychain-modal flex w-full max-w-md flex-col overflow-hidden text-sm"
onClick={(e) => e.stopPropagation()}
>
<div className="px-5 py-4 border-b border-border-modal flex items-center gap-3">
<div className="p-2 bg-red-500/10 rounded-full flex items-center justify-center">
<AlertTriangle size={20} className="text-red-400" />
</div>
<h2 className="text-lg font-semibold text-text-primary m-0">{t($ => $.dialogs.removeDownload.title)}</h2>
<h2 id="remove-download-title" className="text-lg font-semibold text-text-primary m-0">{t($ => $.dialogs.removeDownload.title)}</h2>
</div>
<div className="px-5 py-6 flex-1 text-sm text-text-secondary leading-relaxed">
+14 -3
View File
@@ -1,5 +1,6 @@
import { useEffect, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { isTopmostModal, useModalFocus } from '../hooks/useModalFocus';
export type DuplicateReason = { type: 'url', msg: string } | { type: 'file', msg: string };
type DuplicateResolution = 'rename' | 'replace' | 'skip';
@@ -22,10 +23,14 @@ interface Props {
export const DuplicateResolutionModal = ({ conflicts: initialConflicts, onConfirm, onCancel }: Props) => {
const { t } = useTranslation();
const [conflicts, setConflicts] = useState<DuplicateConflict[]>(initialConflicts);
const modalRef = useModalFocus();
useEffect(() => {
const handleEscape = (event: KeyboardEvent) => {
if (event.key === 'Escape') onCancel();
if (event.key === 'Escape' && isTopmostModal(modalRef.current)) {
event.preventDefault();
onCancel();
}
};
window.addEventListener('keydown', handleEscape);
return () => window.removeEventListener('keydown', handleEscape);
@@ -43,10 +48,16 @@ export const DuplicateResolutionModal = ({ conflicts: initialConflicts, onConfir
}}
role="dialog"
aria-modal="true"
aria-labelledby="duplicate-downloads-title"
>
<div className="app-modal w-[500px] flex flex-col overflow-hidden text-sm">
<div
ref={modalRef}
tabIndex={-1}
data-modal-surface="true"
className="app-modal w-[500px] flex flex-col overflow-hidden text-sm"
>
<div className="p-4 border-b border-border-modal flex flex-col gap-2">
<h2 className="text-lg font-semibold text-text-primary">{t($ => $.dialogs.duplicateDownloads.title)}</h2>
<h2 id="duplicate-downloads-title" className="text-lg font-semibold text-text-primary">{t($ => $.dialogs.duplicateDownloads.title)}</h2>
<p className="text-xs text-text-muted">{t($ => $.dialogs.duplicateDownloads.description)}</p>
</div>
+44 -12
View File
@@ -3,10 +3,9 @@ import { useSettingsStore } from '../store/useSettingsStore';
import { invokeCommand as invoke } from '../ipc';
import { KeyRound, ShieldAlert } from 'lucide-react';
import { usePlatformInfo } from '../utils/platform';
import { getKeychainConsentVersion } from '../utils/keychainStartup';
import { getVersion } from '@tauri-apps/api/app';
import type { PairingTokenHydration } from '../bindings/PairingTokenHydration';
import { useTranslation } from 'react-i18next';
import { isTopmostModal, useModalFocus } from '../hooks/useModalFocus';
const KEYCHAIN_GRANT_TIMEOUT_MS = 30_000;
@@ -25,15 +24,20 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
const [error, setError] = useState<string | null>(null);
const isMountedRef = useRef(true);
const grantRequestRef = useRef<Promise<PairingTokenHydration> | null>(null);
const grantAttemptRef = useRef(0);
const modalRef = useModalFocus(showKeychainModal);
useEffect(() => () => {
isMountedRef.current = false;
grantAttemptRef.current += 1;
}, []);
useEffect(() => {
if (!showKeychainModal || isGranting || grantRequestPending) return;
if (!showKeychainModal) return;
const handleEscape = (event: KeyboardEvent) => {
if (event.key !== 'Escape') return;
if (event.key !== 'Escape' || !isTopmostModal(modalRef.current)) return;
event.preventDefault();
grantAttemptRef.current += 1;
if (consentVersion.trim()) {
dismissKeychainPrompt(consentVersion);
} else {
@@ -42,7 +46,7 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
};
window.addEventListener('keydown', handleEscape);
return () => window.removeEventListener('keydown', handleEscape);
}, [consentVersion, dismissKeychainPrompt, grantRequestPending, isGranting, showKeychainModal]);
}, [consentVersion, dismissKeychainPrompt, showKeychainModal]);
if (!showKeychainModal) {
return null;
@@ -72,7 +76,11 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
// A native credential-store call cannot be cancelled from the webview.
// Keep the request identity until it settles so a UI timeout cannot
// launch a second OS prompt while the first one is still outstanding.
// The native command owns the process-wide guard, so a remounted dialog
// receives an explicit in-progress error instead of silently doing
// nothing while an earlier native request is still outstanding.
if (grantRequestRef.current) return;
const grantAttempt = ++grantAttemptRef.current;
setIsGranting(true);
setGrantRequestTimedOut(false);
setError(null);
@@ -81,8 +89,15 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
let persistentGrantApplied = false;
const applyPersistentGrant = async (result: PairingTokenHydration): Promise<boolean> => {
if (!result.persistent || persistentGrantApplied) return result.persistent;
// A native prompt cannot be cancelled by the webview. If the user
// dismisses this dialog after a timeout, a late result must not turn
// that explicit choice into a silent authorization.
if (!isMountedRef.current || grantAttemptRef.current !== grantAttempt) return false;
persistentGrantApplied = true;
const grantedVersion = consentVersion || getKeychainConsentVersion(await getVersion().catch(() => ''));
// App startup normally provides the version. Do not issue another IPC
// request here when it is temporarily unknown: a successful grant must
// finish the modal even if version lookup is unavailable.
const grantedVersion = consentVersion.trim() || useSettingsStore.getState().keychainAccessVersion;
// Keep state in sync with the grant result instead of rehydrating
// before Zustand has persisted keychainAccessGranted.
useSettingsStore.setState({
@@ -96,7 +111,16 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
});
return true;
};
const grantRequest = invoke('grant_keychain_access');
let grantRequest: Promise<PairingTokenHydration>;
try {
grantRequest = invoke('grant_keychain_access');
} catch (error) {
if (isMountedRef.current) {
setIsGranting(false);
setError(error instanceof Error ? error.message : String(error));
}
return;
}
grantRequestRef.current = grantRequest;
setGrantRequestPending(true);
void grantRequest.then(
@@ -147,6 +171,7 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
};
const handleLater = () => {
grantAttemptRef.current += 1;
if (consentVersion.trim()) {
dismissKeychainPrompt(consentVersion);
} else {
@@ -160,16 +185,19 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
return (
<div
className="app-modal-backdrop fixed inset-0 z-50 flex items-center justify-center bg-black/40"
className="app-modal-backdrop fixed inset-0 z-[80] flex items-center justify-center"
onClick={(event) => {
if (event.target === event.currentTarget && !isGranting && !grantRequestPending) handleLater();
if (event.target === event.currentTarget && isTopmostModal(modalRef.current)) handleLater();
}}
role="dialog"
aria-modal="true"
aria-labelledby="keychain-permission-title"
>
<div
className="window-safe-modal bg-bg-modal rounded-xl w-full max-w-md overflow-hidden flex flex-col shadow-2xl border border-border-modal scale-in"
ref={modalRef}
tabIndex={-1}
data-modal-surface="true"
className="app-modal keychain-modal flex w-full max-w-md flex-col overflow-hidden text-sm"
onClick={(e) => e.stopPropagation()}
>
<div className="px-5 py-4 border-b border-border-modal flex items-center gap-3">
@@ -219,7 +247,7 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
<button
type="button"
onClick={handleLater}
disabled={isGranting || (grantRequestPending && !grantRequestTimedOut)}
data-modal-autofocus="true"
className="keychain-modal-action px-4 py-2 rounded-lg text-sm font-medium text-text-secondary hover:bg-item-hover hover:text-text-primary disabled:opacity-50"
>
{t($ => $.keychain.later)}
@@ -230,7 +258,11 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
disabled={isGranting || grantRequestPending}
className="keychain-modal-action px-4 py-2 rounded-lg text-sm font-medium bg-accent text-accent-foreground hover:bg-accent/90 disabled:opacity-50"
>
{isGranting || grantRequestPending ? t($ => $.keychain.enabling) : grantLabel}
{isGranting
? t($ => $.keychain.enabling)
: grantRequestTimedOut && grantRequestPending
? t($ => $.keychain.waitingForPrompt)
: grantLabel}
</button>
</div>
</div>
+73 -15
View File
@@ -1,4 +1,4 @@
import { useState, useEffect } from 'react';
import { useState, useEffect, useRef } from 'react';
import { useDownloadStore, DownloadItem } from '../store/useDownloadStore';
import { useDownloadProgressStore } from '../store/downloadProgressStore';
import { useShallow } from 'zustand/react/shallow';
@@ -19,6 +19,7 @@ import {
import { resolveDownloadConnections } from '../utils/downloads';
import { useTranslation } from 'react-i18next';
import { formatDateTime, type CalendarPreference } from '../utils/dateTime';
import { isTopmostModal, useModalFocus } from '../hooks/useModalFocus';
type LoginMode = 'matching' | 'custom' | 'none';
@@ -89,6 +90,14 @@ export const PropertiesModal = () => {
const [errorMessage, setErrorMessage] = useState('');
const [isPauseResumePending, setIsPauseResumePending] = useState(false);
const actionRequestRef = useRef(0);
const modalRef = useModalFocus(Boolean(selectedPropertiesDownloadId && item));
useEffect(() => {
// Invalidate native pickers and transfer-control results when the modal
// switches items, closes, or reopens for the same download.
actionRequestRef.current += 1;
}, [selectedPropertiesDownloadId]);
useEffect(() => {
if (selectedPropertiesDownloadId) {
@@ -99,10 +108,19 @@ export const PropertiesModal = () => {
if (activeItem.destination) {
setSaveLocation(activeItem.destination);
} else {
const propertiesDownloadId = selectedPropertiesDownloadId;
const requestId = actionRequestRef.current;
void resolveCategoryDestination(
useSettingsStore.getState(),
activeItem.category
).then(setSaveLocation);
).then(location => {
if (
requestId === actionRequestRef.current
&& useDownloadStore.getState().selectedPropertiesDownloadId === propertiesDownloadId
) {
setSaveLocation(location);
}
});
}
setConnections(resolveDownloadConnections(activeItem.connections, perServerConnections));
setConnectionsDirty(false);
@@ -161,7 +179,10 @@ export const PropertiesModal = () => {
useEffect(() => {
if (!selectedPropertiesDownloadId) return;
const handleEscape = (event: KeyboardEvent) => {
if (event.key === 'Escape') setSelectedPropertiesDownloadId(null);
if (event.key === 'Escape' && isTopmostModal(modalRef.current)) {
event.preventDefault();
setSelectedPropertiesDownloadId(null);
}
};
window.addEventListener('keydown', handleEscape);
return () => window.removeEventListener('keydown', handleEscape);
@@ -171,13 +192,20 @@ export const PropertiesModal = () => {
const handleBrowse = async () => {
if (identityLocked) return;
const requestId = ++actionRequestRef.current;
const propertiesDownloadId = item.id;
try {
const selected = await open({
directory: true,
multiple: false,
defaultPath: saveLocation.startsWith('~') ? undefined : saveLocation
});
if (selected && typeof selected === 'string') {
if (
selected
&& typeof selected === 'string'
&& requestId === actionRequestRef.current
&& useDownloadStore.getState().selectedPropertiesDownloadId === propertiesDownloadId
) {
setSaveLocation(selected);
}
} catch (e) {
@@ -211,12 +239,20 @@ export const PropertiesModal = () => {
: {}),
};
const requestId = ++actionRequestRef.current;
try {
setErrorMessage('');
await useDownloadStore.getState().applyProperties(item.id, updates);
setSelectedPropertiesDownloadId(null);
if (
requestId === actionRequestRef.current
&& useDownloadStore.getState().selectedPropertiesDownloadId === item.id
) {
setSelectedPropertiesDownloadId(null);
}
} catch (e) {
setErrorMessage(e instanceof Error ? e.message : String(e));
if (requestId === actionRequestRef.current && useDownloadStore.getState().selectedPropertiesDownloadId === item.id) {
setErrorMessage(e instanceof Error ? e.message : String(e));
}
}
};
@@ -231,6 +267,7 @@ export const PropertiesModal = () => {
}
setErrorMessage('');
const requestId = ++actionRequestRef.current;
setIsPauseResumePending(true);
try {
if (action === 'pause') {
@@ -246,9 +283,13 @@ export const PropertiesModal = () => {
const message = action === 'pause'
? t($ => $.downloadTable.pauseFailed)
: t($ => $.downloadTable.resumeFailed, { fileName: currentItem.fileName });
setErrorMessage(t($ => $.downloadTable.interactionError, { message, detail }));
if (requestId === actionRequestRef.current && useDownloadStore.getState().selectedPropertiesDownloadId === item.id) {
setErrorMessage(t($ => $.downloadTable.interactionError, { message, detail }));
}
} finally {
setIsPauseResumePending(false);
if (requestId === actionRequestRef.current && useDownloadStore.getState().selectedPropertiesDownloadId === item.id) {
setIsPauseResumePending(false);
}
}
};
@@ -256,16 +297,27 @@ export const PropertiesModal = () => {
if (isLiveSpeedLimitPending || item.isMedia || !['downloading', 'retrying'].includes(item.status)) return;
setErrorMessage('');
const requestId = ++actionRequestRef.current;
setIsLiveSpeedLimitPending(true);
try {
await useDownloadStore.getState().setDownloadSpeedLimit(item.id, limit);
if (limit === null) setLiveSpeedLimitValue('');
if (
limit === null
&& requestId === actionRequestRef.current
&& useDownloadStore.getState().selectedPropertiesDownloadId === item.id
) {
setLiveSpeedLimitValue('');
}
} catch (error) {
setErrorMessage(t($ => $.properties.liveSpeedLimitFailed, {
detail: error instanceof Error ? error.message : String(error)
}));
if (requestId === actionRequestRef.current && useDownloadStore.getState().selectedPropertiesDownloadId === item.id) {
setErrorMessage(t($ => $.properties.liveSpeedLimitFailed, {
detail: error instanceof Error ? error.message : String(error)
}));
}
} finally {
setIsLiveSpeedLimitPending(false);
if (requestId === actionRequestRef.current && useDownloadStore.getState().selectedPropertiesDownloadId === item.id) {
setIsLiveSpeedLimitPending(false);
}
}
};
@@ -361,13 +413,19 @@ export const PropertiesModal = () => {
}}
role="dialog"
aria-modal="true"
aria-labelledby="properties-modal-title"
>
<div className="app-modal w-[720px] h-[580px] flex flex-col overflow-hidden text-sm">
<div
ref={modalRef}
tabIndex={-1}
data-modal-surface="true"
className="app-modal properties-modal w-[720px] h-[580px] flex flex-col overflow-hidden text-sm"
>
{/* Header Summary */}
<div className="p-4 px-5 bg-sidebar-bg/50">
<div className="flex items-center justify-between mb-3">
<h2 className="text-base font-semibold truncate text-text-primary pr-4">{item.fileName}</h2>
<h2 id="properties-modal-title" className="text-base font-semibold truncate text-text-primary pr-4">{item.fileName}</h2>
<span className={`flex items-center gap-1.5 text-xs font-semibold tracking-wide uppercase ${statusColor}`}>
<StatusIcon size={14} />
{statusLabel}