From 499700d11942baeca6e0dc1112872572fd19bad0 Mon Sep 17 00:00:00 2001 From: Erwin Wee Date: Sat, 19 Sep 2026 03:25:48 +0800 Subject: [PATCH 1/2] Fix UI audit findings: keyboard access, dialog focus, labels, contrast, safety guards - Dialog: aria-labelledby, focus trap, restore opener on close - List rows (commits, files, stashes): role=option, tabIndex, arrow/Enter/Shift+F10 navigation - TextField/FilterInput/Settings: associate labels, aria-describedby, aria-invalid - Contrast: --fg-subtle both themes, dark --success-hover now >=4.5:1 - prefers-reduced-motion: drop dialog/toast/flash animations, slow spinner - Commit form: summary input gets its own row; AI actions move to the actions row - History/stash file pane: width min(280px, 32%) so the diff gets room at 960px - Use ours/theirs: success toast with Undo (git.conflict.unresolve) - Abort merge/rebase/cherry-pick/revert: confirm first - History list/details, Welcome health, large-file scan: inline error + Retry instead of empty-state copy --- src/renderer/src/components/ChangesTab.tsx | 62 ++++++++++--------- src/renderer/src/components/HealthView.tsx | 8 ++- src/renderer/src/components/HistoryTab.tsx | 36 +++++++++-- src/renderer/src/components/StashesTab.tsx | 8 ++- src/renderer/src/components/Welcome.tsx | 7 +++ .../src/components/dialogs/SettingsDialog.tsx | 57 ++++++++--------- src/renderer/src/components/ui.tsx | 42 ++++++++++--- src/renderer/src/lib/listKeys.ts | 31 ++++++++++ src/renderer/src/state/actions.ts | 45 ++++++++++---- src/renderer/src/state/store.ts | 9 ++- src/renderer/src/styles/components.css | 13 +++- src/renderer/src/styles/global.css | 22 ++++++- 12 files changed, 247 insertions(+), 93 deletions(-) create mode 100644 src/renderer/src/lib/listKeys.ts diff --git a/src/renderer/src/components/ChangesTab.tsx b/src/renderer/src/components/ChangesTab.tsx index 3279162..9319dd7 100644 --- a/src/renderer/src/components/ChangesTab.tsx +++ b/src/renderer/src/components/ChangesTab.tsx @@ -5,6 +5,7 @@ import { invoke, isMac } from '../api'; import * as actions from '../state/actions'; import { openDialog, patchChanges, store, useAppStore } from '../state/store'; import { liveFindings, SEVERITY_ICON, SEVERITY_TONE } from './review/ReviewView'; +import { onListKeyDown } from '../lib/listKeys'; import { Avatar, Button, Checkbox, Icon, PathLabel, Spinner, openContextMenu, statusIcon, statusLabel, type MenuItem } from './ui'; const SUMMARY_LIMIT = 72; @@ -81,7 +82,7 @@ export function ChangesTab(): React.JSX.Element { ) : null} -
{ if ((e.target as HTMLElement).closest('.file-row')) return; if (files.length) openContextMenu(e, [{ label: 'Discard all changes…', danger: true, onClick: () => actions.requestDiscard(files.map((f) => f.path), true) }, { label: 'Stash all changes', onClick: () => void actions.stashAll() }]); }}> +
{ if ((e.target as HTMLElement).closest('.file-row')) return; if (files.length) openContextMenu(e, [{ label: 'Discard all changes…', danger: true, onClick: () => actions.requestDiscard(files.map((f) => f.path), true) }, { label: 'Stash all changes', onClick: () => void actions.stashAll() }]); }}> {status && files.length === 0 ? (
@@ -95,6 +96,9 @@ export function ChangesTab(): React.JSX.Element { return (
actions.selectWorkingFile(file.path, { toggle: e.ctrlKey || e.metaKey, range: e.shiftKey })} onContextMenu={(e) => contextMenu(e, file)} @@ -201,7 +205,7 @@ function PrecommitFindingRow({ finding, stale, active }: { finding: ReviewFindin export function CommitFileRow({ file, selected, onSelect, onContextMenu }: { file: CommitFile; selected: boolean; onSelect: () => void; onContextMenu?: (e: React.MouseEvent) => void }): React.JSX.Element { return ( -
+
{file.lfs ? : null} {file.additions !== null || file.deletions !== null ? ( @@ -299,33 +303,6 @@ function CommitForm(): React.JSX.Element { spellCheck autoComplete="off" /> - {willSign ? : null} - {settings?.ai.provider !== 'disabled' ? ( -
{summaryTooLong && settings?.showCommitLengthWarning !== false ? ( @@ -366,6 +343,33 @@ function CommitForm(): React.JSX.Element {
: null} void actions.setAmend(v)} label="Amend last commit" disabled={changes.committing || !!status?.branch.unborn || inMerge} />
diff --git a/src/renderer/src/components/HealthView.tsx b/src/renderer/src/components/HealthView.tsx index 380830c..3f113ce 100644 --- a/src/renderer/src/components/HealthView.tsx +++ b/src/renderer/src/components/HealthView.tsx @@ -77,20 +77,22 @@ function LargeFilesCard(): React.JSX.Element { const [status, setStatus] = useState('idle'); const [error, setError] = useState(null); const [blobs, setBlobs] = useState([]); + const [cancelled, setCancelled] = useState(false); const [lfsInstalled, setLfsInstalled] = useState(false); const load = async (): Promise => { if (!repo) return; setStatus('loading'); + setCancelled(false); try { const [result, tools] = await Promise.all([invoke('repo.health.largeFiles', repo.path, LARGE_FILE_LIMIT), invoke('app.tools', false)]); setBlobs(result); setLfsInstalled(tools.gitLfs.installed); setStatus('idle'); } catch (err) { - // A cancelled scan returns to idle with no partial results, rather than showing an error. + // A cancelled scan returns to idle, keeping whatever results the previous scan found. if (errorInfo(err).code === 'cancelled') { - setBlobs([]); + setCancelled(true); setStatus('idle'); } else { setError(errorMessage(err)); @@ -111,6 +113,8 @@ function LargeFilesCard(): React.JSX.Element { Scanning history…
+ ) : cancelled && !blobs.length ? ( +

Scan cancelled.

) : !blobs.length ? (

No large blobs found in history.

) : ( diff --git a/src/renderer/src/components/HistoryTab.tsx b/src/renderer/src/components/HistoryTab.tsx index 2cf4f7e..a0fbe18 100644 --- a/src/renderer/src/components/HistoryTab.tsx +++ b/src/renderer/src/components/HistoryTab.tsx @@ -5,6 +5,7 @@ import * as actions from '../state/actions'; import { historyFilterActive, historyReorderDisabled } from '../state/actions'; import { openDialog, patchHistory, setPopover, store, useAppStore } from '../state/store'; import { CommitFileRow } from './ChangesTab'; +import { onListKeyDown } from '../lib/listKeys'; import { Avatar, Badge, Button, Checkbox, FilterInput, Icon, RelativeTime, Spinner, TextField, openContextMenu, type IconName, type MenuItem } from './ui'; const SIGNATURE_BADGES: Partial string }>> = { @@ -133,6 +134,10 @@ export function HistoryTab(): React.JSX.Element { ) : null}
{ if (history.dragging) { e.preventDefault(); @@ -148,7 +153,24 @@ export function HistoryTab(): React.JSX.Element { Loading history…
) : null} - {!history.loading && !history.commits.length ?
{filterActive ? 'No commits match your search.' : status?.branch.unborn ? 'No commits yet.' : 'No history to show.'}
: null} + {!history.loading && !history.commits.length ? ( +
+ {history.error ? ( + <> + {history.error} +
+ +
+ + ) : filterActive ? ( + 'No commits match your search.' + ) : status?.branch.unborn ? ( + 'No commits yet.' + ) : ( + 'No history to show.' + )} +
+ ) : null} {dragOver === 'top' && history.dragging ?
: null} {history.commits.map((c, index) => { const selected = history.selectedShas.includes(c.sha); @@ -158,6 +180,9 @@ export function HistoryTab(): React.JSX.Element { return (
actions.selectCommit(c.sha, { toggle: e.ctrlKey || e.metaKey, range: e.shiftKey })} onContextMenu={(e) => { @@ -307,8 +332,11 @@ export function CommitDetailsPane(): React.JSX.Element { if (!details) { return (
- {history.detailsLoading ? : } -

{history.detailsLoading ? 'Loading commit…' : 'Select a commit to view its changes.'}

+ {history.detailsLoading ? : } +

{history.detailsLoading ? 'Loading commit…' : history.detailsError ?? 'Select a commit to view its changes.'}

+ {history.detailsError ? ( + + ) : null}
); } @@ -370,7 +398,7 @@ export function CommitDetailsPane(): React.JSX.Element { {visibleFiles.length} {history.matchingFiles ? 'matching' : 'changed'} file{visibleFiles.length === 1 ? '' : 's'}
-
+
{visibleFiles.map((f) => (
-
+
{view.loading && !stashes.length ? (
Loading stashes… @@ -64,6 +65,9 @@ export function StashesView(): React.JSX.Element { {sorted.map((st) => (
void actions.selectStash(st.sha)} onContextMenu={(e) => { @@ -90,7 +94,7 @@ export function StashesView(): React.JSX.Element {
{view.files.length} changed file{view.files.length === 1 ? '' : 's'}
-
+
{view.filesLoading ? (
diff --git a/src/renderer/src/components/Welcome.tsx b/src/renderer/src/components/Welcome.tsx index f927b47..a5153ba 100644 --- a/src/renderer/src/components/Welcome.tsx +++ b/src/renderer/src/components/Welcome.tsx @@ -6,6 +6,7 @@ import { Button, Icon, RelativeTime } from './ui'; export function Welcome(): React.JSX.Element { const repos = useAppStore((s) => s.repos); const work = useAppStore((s) => s.work); + const workError = useAppStore((s) => s.workError); const account = useAppStore((s) => s.tools?.ghAccount ?? null); const recent = [...repos].sort((a, b) => b.lastOpened - a.lastOpened).slice(0, 6); @@ -77,6 +78,12 @@ export function Welcome(): React.JSX.Element {
))}
+ ) : workError ? ( +
+

Repository health

+

{workError}

+ +
) : null}
diff --git a/src/renderer/src/components/dialogs/SettingsDialog.tsx b/src/renderer/src/components/dialogs/SettingsDialog.tsx index e3b312d..81d703a 100644 --- a/src/renderer/src/components/dialogs/SettingsDialog.tsx +++ b/src/renderer/src/components/dialogs/SettingsDialog.tsx @@ -179,8 +179,8 @@ function GitTab({ settings, update }: { settings: AppSettings; update: (p: Parti

Pull behavior

- - update({ pullBehavior: e.target.value as AppSettings['pullBehavior'] })}> @@ -188,8 +188,8 @@ function GitTab({ settings, update }: { settings: AppSettings; update: (p: Parti

Background fetch

- - update({ autoFetchIntervalMinutes: Number(e.target.value) })}> @@ -199,12 +199,13 @@ function GitTab({ settings, update }: { settings: AppSettings; update: (p: Parti

Repository health

- - update({ staleBranchDays: Math.max(1, Number(e.target.value) || 90) })} style={{ flex: '0 0 80px' }} /> days without a commit + + update({ staleBranchDays: Math.max(1, Number(e.target.value) || 90) })} style={{ flex: '0 0 80px' }} /> days without a commit
- +

Diff

- - update({ diffViewMode: e.target.value as 'unified' | 'split' })}>
- - update({ diffFontSize: Math.max(9, Math.min(24, Number(e.target.value) || 12)) })} style={{ flex: '0 0 80px' }} /> + + update({ diffFontSize: Math.max(9, Math.min(24, Number(e.target.value) || 12)) })} style={{ flex: '0 0 80px' }} />
update({ diffSyntaxHighlighting: v })} label="Syntax highlighting" /> update({ diffShowIntraline: v })} label="Highlight word-level changes within modified lines" /> @@ -710,8 +711,8 @@ function AiTab({ settings, update }: { settings: AppSettings; update: (p: Partia

AI conflict resolution

One click asks Claude to reconcile both sides of every conflict block in a file. Only the conflicted regions, some surrounding context and the commit subjects on each side are sent. Results are written to the file and can be undone.

- - updateAi({ provider: e.target.value as AppSettings['ai']['provider'] })}> @@ -737,11 +738,11 @@ function AiTab({ settings, update }: { settings: AppSettings; update: (p: Partia {ai.provider !== 'disabled' ? ( <>
- + {customModel ? ( - updateAi({ model: e.target.value })} spellCheck={false} /> + updateAi({ model: e.target.value })} spellCheck={false} /> ) : ( - updateAi({ model: e.target.value })}> {MODELS.map((m) => ( ))} @@ -750,8 +751,8 @@ function AiTab({ settings, update }: { settings: AppSettings; update: (p: Partia
- - updateAi({ effort: e.target.value as AppSettings['ai']['effort'] })}> @@ -763,16 +764,16 @@ function AiTab({ settings, update }: { settings: AppSettings; update: (p: Partia

Pull request review

- - updateAi({ reviewStrictness: e.target.value as AppSettings['ai']['reviewStrictness'] })}>
- - updateAi({ reviewMaxFiles: Math.max(1, Math.min(200, parseInt(e.target.value, 10) || 40)) })} style={{ width: 80 }} /> + + updateAi({ reviewMaxFiles: Math.max(1, Math.min(200, parseInt(e.target.value, 10) || 40)) })} style={{ width: 80 }} /> Files beyond this limit are listed as skipped in the pre-flight card.
updateAi({ reviewPostFooter: v })} label="Append an “AI-assisted” footer to reviews posted to GitHub" /> @@ -782,8 +783,8 @@ function AiTab({ settings, update }: { settings: AppSettings; update: (p: Partia

Triage sends pull request metadata only (titles, bodies, labels, review and check state) — never a diff.

Release notes

- - updateAi({ releaseNotesAudience: e.target.value as AppSettings['ai']['releaseNotesAudience'] })}> @@ -793,8 +794,8 @@ function AiTab({ settings, update }: { settings: AppSettings; update: (p: Partia

Runs the AI review on the exact patch Commit would apply. If it finds anything, a dialog lets you commit anyway or go back; committing is never blocked.

Agent for fixes

- - updateAi({ agentCommand: e.target.value as AppSettings['ai']['agentCommand'] })}> {AGENT_PRESETS.map((p) => )} @@ -889,8 +890,8 @@ function AdvancedTab({ settings, update }: { settings: AppSettings; update: (p: update({ autoDownloadUpdates: v })} label="Automatically download updates once found" disabled={!settings.checkForUpdatesAutomatically} />

This build can only detect and link to new releases; it cannot download or install them yet. Automatic download will take effect once that ships.

- - update({ updateChannel: e.target.value as AppSettings['updateChannel'] })}> diff --git a/src/renderer/src/components/ui.tsx b/src/renderer/src/components/ui.tsx index 056ba46..6d678c8 100644 --- a/src/renderer/src/components/ui.tsx +++ b/src/renderer/src/components/ui.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useLayoutEffect, useMemo, useRef, useState, type ButtonHTMLAttributes, type InputHTMLAttributes, type ReactNode } from 'react'; +import React, { useEffect, useId, useLayoutEffect, useMemo, useRef, useState, type ButtonHTMLAttributes, type InputHTMLAttributes, type ReactNode } from 'react'; import { formatRelativeTime } from '@shared/util'; import { invoke } from '../api'; import { store, useAppStore } from '../state/store'; @@ -131,18 +131,22 @@ interface FieldProps extends InputHTMLAttributes { } export function TextField({ label, hint, error, trailing, className, ...rest }: FieldProps): React.JSX.Element { + const autoId = useId(); + const id = rest.id ?? autoId; + const message = error ?? hint; + const messageId = message ? `${id}-message` : undefined; return (
- {label ? : null} + {label ? : null} {trailing ? (
- + {trailing}
) : ( - + )} - {error ? {error} : hint ? {hint} : null} + {error ? {error} : hint ? {hint} : null}
); } @@ -326,24 +330,42 @@ export function ContextMenuHost(): React.JSX.Element | null { export function Dialog({ title, onClose, children, footer, width, icon, dismissible = true, className }: { title: ReactNode; onClose: () => void; children: ReactNode; footer?: ReactNode; width?: 'default' | 'wide' | 'xwide'; icon?: IconName; dismissible?: boolean; className?: string }): React.JSX.Element { const ref = useRef(null); + const titleId = useId(); useEffect(() => { + const previouslyFocused = document.activeElement as HTMLElement | null; const onKey = (e: KeyboardEvent) => { if (e.key === 'Escape' && dismissible) { e.stopPropagation(); onClose(); + return; + } + if (e.key === 'Tab') { + if (!ref.current || ref.current.closest('.dialog-backdrop') !== [...document.querySelectorAll('.dialog-backdrop')].at(-1)) return; + const focusables = [...ref.current.querySelectorAll('a[href],button:not(:disabled),input:not(:disabled),select:not(:disabled),textarea:not(:disabled),[tabindex]:not([tabindex="-1"])')]; + if (focusables.length === 0) return; + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + const active = document.activeElement; + if (e.shiftKey ? active === first || !ref.current.contains(active) : active === last || !ref.current.contains(active)) { + e.preventDefault(); + (e.shiftKey ? last : first).focus(); + } } }; window.addEventListener('keydown', onKey); const first = ref.current?.querySelector('input:not([type=checkbox]), textarea, select, button.primary, button'); first?.focus(); - return () => window.removeEventListener('keydown', onKey); + return () => { + window.removeEventListener('keydown', onKey); + if (previouslyFocused?.isConnected) previouslyFocused.focus(); + }; }, [dismissible, onClose]); return (
e.target === e.currentTarget && dismissible && onClose()}> -
+
{icon ? : null} -

{title}

+

{title}

{dismissible ?
{children}
@@ -379,11 +401,11 @@ export function useFilter(items: T[], query: string, keys: (item: T) => strin }, [items, query, keys]); } -export function FilterInput({ value, onChange, placeholder, id, autoFocus }: { value: string; onChange: (v: string) => void; placeholder: string; id?: string; autoFocus?: boolean }): React.JSX.Element { +export function FilterInput({ value, onChange, placeholder, label, id, autoFocus }: { value: string; onChange: (v: string) => void; placeholder: string; label?: string; id?: string; autoFocus?: boolean }): React.JSX.Element { return (
- onChange(e.target.value)} spellCheck={false} /> + onChange(e.target.value)} spellCheck={false} />
); } diff --git a/src/renderer/src/lib/listKeys.ts b/src/renderer/src/lib/listKeys.ts new file mode 100644 index 0000000..8b28d97 --- /dev/null +++ b/src/renderer/src/lib/listKeys.ts @@ -0,0 +1,31 @@ +import type { KeyboardEvent } from 'react'; + +/** Shared keyboard-navigation handler for `role="listbox"` containers whose children are `role="option"` rows. */ +export function onListKeyDown(e: KeyboardEvent): void { + const row = (e.target as HTMLElement).closest('[role="option"]'); + if (!row) return; + const options = Array.from(e.currentTarget.querySelectorAll('[role="option"]')); + const index = options.indexOf(row); + if (index < 0) return; + + if (e.key === 'ArrowDown') { + e.preventDefault(); + options[Math.min(index + 1, options.length - 1)].focus(); + } else if (e.key === 'ArrowUp') { + e.preventDefault(); + options[Math.max(index - 1, 0)].focus(); + } else if (e.key === 'Home') { + e.preventDefault(); + options[0].focus(); + } else if (e.key === 'End') { + e.preventDefault(); + options[options.length - 1].focus(); + } else if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + row.click(); + } else if (e.key === 'ContextMenu' || (e.key === 'F10' && e.shiftKey)) { + e.preventDefault(); + const rect = row.getBoundingClientRect(); + row.dispatchEvent(new MouseEvent('contextmenu', { bubbles: true, cancelable: true, clientX: rect.left + rect.width / 2, clientY: rect.top + rect.height / 2 })); + } +} diff --git a/src/renderer/src/state/actions.ts b/src/renderer/src/state/actions.ts index 4dd3666..335d4be 100644 --- a/src/renderer/src/state/actions.ts +++ b/src/renderer/src/state/actions.ts @@ -869,7 +869,7 @@ export function selectCommitFile(path: string): void { export async function loadCommitDetails(sha: string): Promise { const repo = store.get().currentRepo; if (!repo) return; - patchHistory({ detailsLoading: true }); + patchHistory({ detailsLoading: true, detailsError: null }); try { const details = await invoke('repo.commit.details', repo.path, sha); const h = store.get().history; @@ -895,10 +895,10 @@ export async function loadCommitDetails(sha: string): Promise { const pathAtCommit = h.path ? h.pathHistory?.find((e) => e.sha === sha)?.path ?? h.path : null; const preferred = pathAtCommit && visibleFiles.some((f) => f.path === pathAtCommit) ? pathAtCommit : null; const selectedFile = preferred ?? (h.selectedFile && visibleFiles.some((f) => f.path === h.selectedFile) ? h.selectedFile : visibleFiles[0]?.path ?? null); - patchHistory({ details, detailsLoading: false, selectedFile, matchingFiles }); + patchHistory({ details, detailsLoading: false, selectedFile, matchingFiles, detailsError: null }); void loadDiff(); } catch (err) { - patchHistory({ detailsLoading: false }); + patchHistory({ detailsLoading: false, detailsError: errorMessage(err) }); showToast({ kind: 'error', title: 'Could not load commit', message: errorMessage(err) }); } } @@ -927,7 +927,7 @@ export async function loadHistory(reset: boolean): Promise { const h = store.get().history; if (h.loading && !reset) return; if (slowSearchTimer) clearTimeout(slowSearchTimer); - patchHistory({ loading: true, slowSearch: false }); + patchHistory({ loading: true, slowSearch: false, ...(reset ? { error: null } : {}) }); slowSearchTimer = setTimeout(() => patchHistory({ slowSearch: true }), 5000); try { const skip = reset ? 0 : h.commits.length; @@ -935,13 +935,13 @@ export async function loadHistory(reset: boolean): Promise { if (store.get().currentRepo?.path !== repo.path) return; const commits = reset ? page.commits : [...h.commits, ...page.commits]; const stillSelected = store.get().history.selectedShas.filter((sha) => commits.some((c) => c.sha === sha)); - patchHistory({ commits, hasMore: page.hasMore, loading: false, slowSearch: false, selectedShas: stillSelected, details: stillSelected.length === 1 ? store.get().history.details : null }); + patchHistory({ commits, hasMore: page.hasMore, loading: false, slowSearch: false, error: null, selectedShas: stillSelected, details: stillSelected.length === 1 ? store.get().history.details : null }); if (store.get().view === 'history' && stillSelected.length === 0 && commits.length) selectCommit(commits[0].sha); else if (stillSelected.length === 1 && reset) void loadCommitDetails(stillSelected[0]); } catch (err) { // A newer history request superseded this one (the main process aborts the older git run); the newer call owns the loading state. if (err instanceof ApiError && err.code === 'cancelled') return; - patchHistory({ loading: false, slowSearch: false }); + patchHistory({ loading: false, slowSearch: false, error: errorMessage(err) }); showToast({ kind: 'error', title: 'Could not load history', message: errorMessage(err) }); } finally { if (slowSearchTimer) { @@ -1717,9 +1717,18 @@ export async function abortOperation(): Promise { if (!repo || !s.status) return; const kind = s.status.operation.kind; const method = kind === 'rebase' ? 'git.rebase.abort' : kind === 'cherry-pick' ? 'git.cherryPick.abort' : kind === 'revert' ? 'git.revert.abort' : 'git.merge.abort'; - await runOperation(`Abort ${kind}`, () => invoke(method, repo.path)); - closeAllDialogs(); - await loadHistory(true); + openDialog({ + kind: 'confirm', + title: `Abort ${kind}?`, + message: `Conflict resolutions made during this ${kind} will be discarded and the branch returns to its pre-${kind} state.`, + confirmLabel: `Abort ${kind}`, + danger: true, + onConfirm: async () => { + await runOperation(`Abort ${kind}`, () => invoke(method, repo.path)); + closeAllDialogs(); + await loadHistory(true); + }, + }); } export async function skipRebaseCommit(): Promise { @@ -1851,13 +1860,25 @@ export async function undoResolutions(results: ConflictResolutionResult[]): Prom await loadDiff(true); } +async function undoUseSide(repoPath: string, path: string, original: string): Promise { + try { + await invoke('git.conflict.unresolve', repoPath, path, original); + } catch (err) { + showToast({ kind: 'error', title: `Could not undo ${path}`, message: errorMessage(err) }); + } + await refreshStatus(); + await loadDiff(true); +} + export async function useSide(path: string, side: 'ours' | 'theirs'): Promise { const repo = store.get().currentRepo; if (!repo) return; + const original = await invoke('repo.readFile', repo.path, path).catch(() => null); try { await invoke('git.conflict.useSide', repo.path, path, side); await refreshStatus(); await loadDiff(true); + showToast({ kind: 'success', title: `Took ${side} for ${path}`, action: original !== null ? { label: 'Undo', onClick: () => void undoUseSide(repo.path, path, original) } : undefined }); } catch (err) { showError('Could not resolve conflict', err); } @@ -2596,9 +2617,9 @@ export function openHealth(): void { export async function loadWork(): Promise { try { const work = await invoke('repos.work'); - store.set({ work }); - } catch { - /* best-effort */ + store.set({ work, workError: null }); + } catch (err) { + store.set({ workError: errorMessage(err) }); } } diff --git a/src/renderer/src/state/store.ts b/src/renderer/src/state/store.ts index 135fd82..c365874 100644 --- a/src/renderer/src/state/store.ts +++ b/src/renderer/src/state/store.ts @@ -127,6 +127,10 @@ export interface HistoryState { path: string | null; /** sha -> the tracked file's path at that commit, for `path` (from `repo.pathHistory`); null while loading or inactive. */ pathHistory: PathHistoryEntry[] | null; + /** Set when the most recent list load failed; cleared on the next successful load and when a reset load starts. */ + error: string | null; + /** Set when the most recent commit-details load failed; cleared on the next successful load and when a new load starts. */ + detailsError: string | null; } export interface ReviewState { @@ -453,6 +457,8 @@ export interface AppState { submoduleBannerDismissed: boolean; /** Unpushed work across every repository on disk; loaded on demand for the Welcome screen and, opt-in, the repository list's warning dot. */ work: RepoWork[]; + /** Set when the most recent `loadWork` fetch failed; previous `work` data is left in place. */ + workError: string | null; /** Bumped whenever signing config is saved from Options → Git, so the commit form's signing indicator re-fetches without needing a repo switch. */ signingConfigVersion: number; /** Bumped after a settings-sync action (enable/upload/download/disconnect) completes, so the sync card re-fetches status even though its confirmation dialog remounted it mid-action. */ @@ -478,7 +484,7 @@ export const initialChanges: ChangesState = { export const initialStashesView: StashesViewState = { loading: false, selectedSha: null, files: [], filesLoading: false, selectedFile: null }; -export const initialHistory: HistoryState = { commits: [], hasMore: false, loading: false, search: '', query: EMPTY_HISTORY_QUERY, freeText: '', queryError: null, slowSearch: false, selectedShas: [], details: null, detailsLoading: false, selectedFile: null, matchingFiles: null, dragging: null, path: null, pathHistory: null }; +export const initialHistory: HistoryState = { commits: [], hasMore: false, loading: false, search: '', query: EMPTY_HISTORY_QUERY, freeText: '', queryError: null, slowSearch: false, selectedShas: [], details: null, detailsLoading: false, selectedFile: null, matchingFiles: null, dragging: null, path: null, pathHistory: null, error: null, detailsError: null }; export const initialDiff: DiffState = { key: null, diff: null, loading: false, error: null, selectedLines: null, blameOn: false, blame: null, blameLoading: false, activeBlameId: null, highlightTerm: null }; @@ -534,6 +540,7 @@ const initialState: AppState = { lfsStatus: null, submoduleBannerDismissed: false, work: [], + workError: null, signingConfigVersion: 0, settingsSyncVersion: 0, inbox: initialInbox, diff --git a/src/renderer/src/styles/components.css b/src/renderer/src/styles/components.css index ecf32b8..5e3c379 100644 --- a/src/renderer/src/styles/components.css +++ b/src/renderer/src/styles/components.css @@ -341,6 +341,12 @@ text-align: center; color: var(--fg-muted); } +.commit-row:focus-visible, +.file-row:focus-visible, +.list-row:focus-visible { + outline: none; + box-shadow: inset 0 0 0 2px var(--accent); +} .context-menu { position: fixed; @@ -527,7 +533,8 @@ min-height: 0; } .stashes-files { - width: 280px; + width: min(280px, 32%); + min-width: 160px; flex: 0 0 auto; border-right: 1px solid var(--border); display: flex; @@ -572,6 +579,7 @@ } .commit-form .form-actions { display: flex; + flex-wrap: wrap; align-items: center; gap: 8px; justify-content: space-between; @@ -801,7 +809,8 @@ min-height: 0; } .commit-files { - width: 280px; + width: min(280px, 32%); + min-width: 160px; flex: 0 0 auto; border-right: 1px solid var(--border); display: flex; diff --git a/src/renderer/src/styles/global.css b/src/renderer/src/styles/global.css index 49446b9..9012a3e 100644 --- a/src/renderer/src/styles/global.css +++ b/src/renderer/src/styles/global.css @@ -15,7 +15,7 @@ --border-muted: #d8dee4; --fg: #1f2328; --fg-muted: #656d76; - --fg-subtle: #8c959f; + --fg-subtle: #6e7781; --fg-on-emphasis: #ffffff; --accent: #0969da; --accent-emphasis: #0969da; @@ -78,7 +78,7 @@ --border-muted: #21262d; --fg: #e6edf3; --fg-muted: #8d96a0; - --fg-subtle: #6e7681; + --fg-subtle: #848d97; --fg-on-emphasis: #ffffff; --accent: #4493f8; --accent-emphasis: #1f6feb; @@ -86,7 +86,7 @@ --accent-subtle: rgba(56, 139, 253, 0.15); --success: #3fb950; --success-emphasis: #238636; - --success-hover: #2ea043; + --success-hover: #207b32; --success-subtle: rgba(46, 160, 67, 0.15); --attention: #d29922; --attention-emphasis: #9e6a03; @@ -314,3 +314,19 @@ kbd { .hljs-deletion { color: var(--hl-deletion); } + +@media (prefers-reduced-motion: reduce) { + .dialog-backdrop, + .dialog, + .toast, + table.diff tr.flash-highlight td { + animation: none !important; + } + .toolbar-progress, + .progress-bar > div { + transition: none !important; + } + .spinner { + animation-duration: 1.6s !important; + } +} From 8c4179cd0477230881b95838813943d0872424df Mon Sep 17 00:00:00 2001 From: Erwin Wee Date: Sat, 19 Sep 2026 03:41:53 +0800 Subject: [PATCH 2/2] Address review: keep checkbox Space native, restore dialog/menu focus, cover ReviewView list - listKeys: Enter/Space only activate when the row itself is focused - Dialog: capture opener before autoFocus, prefer live outside-dialog focus, restore even from context-menu-opened dialogs; Tab with no focusables stays put - ContextMenuHost: focus first item on open, ArrowUp/Down cycle, restore opener on close - ReviewView file list gets role=listbox + key handler - selectCommit clears detailsError; LargeFilesCard resets blobs per repository --- src/renderer/src/components/HealthView.tsx | 1 + .../src/components/review/ReviewView.tsx | 3 +- src/renderer/src/components/ui.tsx | 41 +++++++++++++++---- src/renderer/src/lib/listKeys.ts | 2 +- src/renderer/src/state/actions.ts | 2 +- 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/renderer/src/components/HealthView.tsx b/src/renderer/src/components/HealthView.tsx index 3f113ce..d16441d 100644 --- a/src/renderer/src/components/HealthView.tsx +++ b/src/renderer/src/components/HealthView.tsx @@ -102,6 +102,7 @@ function LargeFilesCard(): React.JSX.Element { }; useEffect(() => { + setBlobs([]); void load(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [repo?.path]); diff --git a/src/renderer/src/components/review/ReviewView.tsx b/src/renderer/src/components/review/ReviewView.tsx index bd6f365..18b8e1c 100644 --- a/src/renderer/src/components/review/ReviewView.tsx +++ b/src/renderer/src/components/review/ReviewView.tsx @@ -3,6 +3,7 @@ import type { ReviewFinding, ReviewRun, ReviewSeverity } from '@shared/types'; import { isMac } from '../../api'; import * as actions from '../../state/actions'; import { useAppStore, type PrReviewRun } from '../../state/store'; +import { onListKeyDown } from '../../lib/listKeys'; import { CommitFileRow } from '../ChangesTab'; import { Badge, Button, Icon, PathLabel, Spinner, openContextMenu, type IconName } from '../ui'; @@ -67,7 +68,7 @@ export function ReviewView(): React.JSX.Element { {targetLabel(run)}
-
+
{reviewedFiles.map((entry) => { const f = entry.file; const c = countByPath.get(f.path); diff --git a/src/renderer/src/components/ui.tsx b/src/renderer/src/components/ui.tsx index 6d678c8..b6dc17d 100644 --- a/src/renderer/src/components/ui.tsx +++ b/src/renderer/src/components/ui.tsx @@ -241,11 +241,13 @@ interface ContextMenuState { } let menuState: ContextMenuState | null = null; +let menuOpener: HTMLElement | null = null; const menuListeners = new Set<() => void>(); export function openContextMenu(e: { clientX: number; clientY: number; preventDefault(): void; stopPropagation(): void }, items: MenuItem[]): void { e.preventDefault(); e.stopPropagation(); + menuOpener = document.activeElement as HTMLElement | null; menuState = { x: e.clientX, y: e.clientY, items }; for (const l of menuListeners) l(); } @@ -268,11 +270,19 @@ export function ContextMenuHost(): React.JSX.Element | null { }, []); useEffect(() => { if (!menuState) return; + const opener = menuOpener; + ref.current?.querySelector('button:not(:disabled)')?.focus(); const onDown = (ev: MouseEvent) => { if (ref.current && !ref.current.contains(ev.target as Node)) closeContextMenu(); }; const onKey = (ev: KeyboardEvent) => { if (ev.key === 'Escape') closeContextMenu(); + if (ev.key !== 'ArrowDown' && ev.key !== 'ArrowUp') return; + ev.preventDefault(); + const items = [...(ref.current?.querySelectorAll('button:not(:disabled)') ?? [])]; + if (!items.length) return; + const i = items.indexOf(document.activeElement as HTMLElement); + items[(i + (ev.key === 'ArrowDown' ? 1 : items.length - 1)) % items.length].focus(); }; window.addEventListener('mousedown', onDown, true); window.addEventListener('keydown', onKey, true); @@ -281,8 +291,9 @@ export function ContextMenuHost(): React.JSX.Element | null { window.removeEventListener('mousedown', onDown, true); window.removeEventListener('keydown', onKey, true); window.removeEventListener('blur', closeContextMenu); + if (opener?.isConnected && (document.activeElement === document.body || ref.current?.contains(document.activeElement))) opener.focus(); }; - }); + }, [menuState]); useLayoutEffect(() => { const el = ref.current; if (!el || !menuState) return; @@ -331,8 +342,22 @@ export function ContextMenuHost(): React.JSX.Element | null { export function Dialog({ title, onClose, children, footer, width, icon, dismissible = true, className }: { title: ReactNode; onClose: () => void; children: ReactNode; footer?: ReactNode; width?: 'default' | 'wide' | 'xwide'; icon?: IconName; dismissible?: boolean; className?: string }): React.JSX.Element { const ref = useRef(null); const titleId = useId(); + const opener = useRef(null); + if (opener.current === null) { + const active = document.activeElement as HTMLElement | null; + opener.current = active?.closest('[role="menu"]') ? menuOpener : active; + } + useEffect(() => { + const active = document.activeElement as HTMLElement | null; + if (!ref.current?.contains(active)) { + if (active && active !== document.body) opener.current = active; + ref.current?.querySelector('.dialog-body input:not([type=checkbox]), .dialog-body textarea, .dialog-body select, .dialog-footer button.primary, button')?.focus(); + } + return () => { + if (opener.current?.isConnected) opener.current.focus(); + }; + }, []); useEffect(() => { - const previouslyFocused = document.activeElement as HTMLElement | null; const onKey = (e: KeyboardEvent) => { if (e.key === 'Escape' && dismissible) { e.stopPropagation(); @@ -342,7 +367,10 @@ export function Dialog({ title, onClose, children, footer, width, icon, dismissi if (e.key === 'Tab') { if (!ref.current || ref.current.closest('.dialog-backdrop') !== [...document.querySelectorAll('.dialog-backdrop')].at(-1)) return; const focusables = [...ref.current.querySelectorAll('a[href],button:not(:disabled),input:not(:disabled),select:not(:disabled),textarea:not(:disabled),[tabindex]:not([tabindex="-1"])')]; - if (focusables.length === 0) return; + if (focusables.length === 0) { + e.preventDefault(); + return; + } const first = focusables[0]; const last = focusables[focusables.length - 1]; const active = document.activeElement; @@ -353,12 +381,7 @@ export function Dialog({ title, onClose, children, footer, width, icon, dismissi } }; window.addEventListener('keydown', onKey); - const first = ref.current?.querySelector('input:not([type=checkbox]), textarea, select, button.primary, button'); - first?.focus(); - return () => { - window.removeEventListener('keydown', onKey); - if (previouslyFocused?.isConnected) previouslyFocused.focus(); - }; + return () => window.removeEventListener('keydown', onKey); }, [dismissible, onClose]); return (
e.target === e.currentTarget && dismissible && onClose()}> diff --git a/src/renderer/src/lib/listKeys.ts b/src/renderer/src/lib/listKeys.ts index 8b28d97..55b2523 100644 --- a/src/renderer/src/lib/listKeys.ts +++ b/src/renderer/src/lib/listKeys.ts @@ -20,7 +20,7 @@ export function onListKeyDown(e: KeyboardEvent): void { } else if (e.key === 'End') { e.preventDefault(); options[options.length - 1].focus(); - } else if (e.key === 'Enter' || e.key === ' ') { + } else if ((e.key === 'Enter' || e.key === ' ') && e.target === row) { e.preventDefault(); row.click(); } else if (e.key === 'ContextMenu' || (e.key === 'F10' && e.shiftKey)) { diff --git a/src/renderer/src/state/actions.ts b/src/renderer/src/state/actions.ts index 335d4be..8a8af49 100644 --- a/src/renderer/src/state/actions.ts +++ b/src/renderer/src/state/actions.ts @@ -856,7 +856,7 @@ export function selectCommit(sha: string, opts: { toggle?: boolean; range?: bool } else { selected = [sha]; } - patchHistory({ selectedShas: selected, details: selected.length === 1 && s.history.details?.commit.sha === selected[0] ? s.history.details : null, selectedFile: selected.length === 1 && s.history.details?.commit.sha === selected[0] ? s.history.selectedFile : null }); + patchHistory({ selectedShas: selected, detailsError: null, details: selected.length === 1 && s.history.details?.commit.sha === selected[0] ? s.history.details : null, selectedFile: selected.length === 1 && s.history.details?.commit.sha === selected[0] ? s.history.selectedFile : null }); if (selected.length === 1) void loadCommitDetails(selected[0]); else void loadDiff(); }