Repository navigation
fix(keyboard): preserve focus across dialogs, tabs, and drags - #1900
Conversation
The backdrop click handler tested the click against the responsive bin around the dialog, which fills the window, so every click counted as inside the dialog and none closed Settings. Test against the dialog panel, whose margin is the area the backdrop shows.
Reuse modal focus restoration for deletion and cross-volume prompts, keep hidden tabs and background completions from taking focus, and guard Settings against window-level dialogs. Share heading-drag cancellation between browser and chooser. Preserve initial and renamed-row focus while cancelling obsolete keyboard refocus work. Extend existing behavioral coverage, retain chooser selection on Escape, and make the near-end route playback check observe actual progress instead of a transient playing frame. Refs #1859
Implementation and validationMerged main at The remaining changes share one focus eligibility policy and one heading-drag finalizer across the browser/chooser. No new polling or per-row input handlers; no measured performance improvement claimed. Regressions exercise input routing, hidden tabs, startup, filtering, modal ownership, dismissal routes, file safety, and selection after drag cancellation—not layout assertions. Final affected validation after merging mainAll GUI checks used the pinned container with isolated Xvfb/D-Bus; no desktop session was used. Scoped Rust commands used the ignored helper
Scope rationale: shared focus/view switching also affects initial mapping, chooser startup, filters, renamed rows, and background completions. The selected tests cover those callers plus every reported dismissal/reorder case. The merge did not change the test image, dependencies, or harness; Cargo's incoming change is the application's version bump. Earlier full sweeps and intentional omissionsFull Per the scoped-rerun policy, full suites were not repeated after those fixes or the main merge. No clean final full-suite run is claimed. The final evidence is 38 selected Rust tests and 21 selected QA runs, plus formatting and Clippy. Required full CI remains authoritative before merge; this PR stays draft, so the workflow intentionally skips its build/test jobs until the owner marks it ready. Local videosThe owner's requested captures remain ignored and are not uploaded or committed. |
Raise the direct minimum and regenerate the lockfile for zstd 0.14.3, zstd-safe 8.2.0, and zstd-sys 2.1.2. Resolve RUSTSEC-2026-0391, RUSTSEC-2026-0398, RUSTSEC-2026-0403, and RUSTSEC-2026-0410 without weakening the dependency policy. Refs #1859
The metadata regression must not assume a scrollable context-menu item fits inside the screen. Invoke the same menu action directly; retain its dialog and preview assertions. Pointer input remains covered by the context-menu scenarios. Refs #1859
CI fixes
Main was already current at ValidationDependency changes justified one full pinned Rust sweep and one full pinned QA sweep. Subsequent failures were isolated rather than repeating full suites.
Exact isolated Rust commands — each selected one test and passedThe ignored helper uses the verified pinned image and existing quality cache and invokes bash target/issue-1859-rust.sh ui::media::tests::reopening_resumes_where_the_preview_closed -- --exact --test-threads=1
bash target/issue-1859-rust.sh ui::missing_tools::tests::missing_tools_dialog_shows_caller_content_and_closes -- --exact --test-threads=1
bash target/issue-1859-rust.sh ui::progress_dock::tests::already_hovered_notifications_start_paused_and_pinning_is_independent -- --exact --test-threads=1
bash target/issue-1859-rust.sh ui::window::composition::tabs::tests::restored_tabs_open_once_a_worker_checks_their_folders -- --exact --test-threads=1
bash target/issue-1859-rust.sh ui::window::drive_dialogs::tests::format_dialog_reads_drive_facts_off_the_ui_thread_and_continue_waits_for_them -- --exact --test-threads=1
bash target/issue-1859-rust.sh ui::window::tests::keyboard_dispatch::filter_focus::space_acts_on_a_selected_hit_only_from_the_filter -- --exact --test-threads=1
bash target/issue-1859-rust.sh ui::window::tests::keyboard_dispatch::overlay_focus::closing_settings_returns_focus_to_a_focused_filter_field -- --exact --test-threads=1
bash target/issue-1859-rust.sh ui::window::tests::keyboard_dispatch::pane_focus::empty_directory_focus_is_a_named_mapped_surface_and_tabs_out -- --exact --test-threads=1
bash target/issue-1859-rust.sh ui::window::tests::keyboard_dispatch::video_clips::enter_on_a_video_preview_opens_it_where_it_stopped -- --exact --test-threads=1CI: all required checks green at The CI QA job remains intentionally skipped because it runs only on manual dispatch. No clean final full local QA or unsharded Rust run is claimed; the initial full results and passing scoped reruns are recorded above. No merge or readiness change was made; the owner's PR description and video attachments were preserved. |
Review and cleanupMerged Review against the description and #1859. I checked each open item in the issue against the code, and each one has a test that fails without its fix:
There were no review threads or inline comments to reconcile. Only the author's two status comments were on the PR. Tests and comments audit. The new and changed Rust tests and QA scenarios each test input routing, focus lifecycle or file safety, and none is tautological, so nothing was removed. I kept every added comment: each explains a non-obvious reason (drag-end ordering on reset, why startup focus is allowed before mapping, why the panel is the hit-test target, why the recycled rename-row cursor is restored). Fixes pushed: none. I found no merge blockers. Validation (on the merged tree, in an isolated worktree):
Follow-ups outside this PR
CI: all required checks green at |
# Conflicts: # tests/qa/scenarios/properties_media/media-details-properties-only.jsonl
Merge conflict resolvedMerged Validation (on the merged tree, in an isolated worktree):
CI: all required checks green at |
Description
Keep keyboard focus in the visible file pane after dialogs close and views change, and preserve an open modal's focus when a background operation finishes. Settings now refuses to open underneath Properties.
Reuse the shared modal focus restoration and heading-drag finalizer, including the chooser. Escape cancels heading reordering without clearing file selection or saving a new order; later drags still work. Preserve startup and renamed-row focus while discarding obsolete keyboard refocus work.
Includes the original outside-click fix from #1879 rather than a second implementation. Merges current main and reuses #1890's rename-cursor and playback-test fixes without duplicating them.
Visual evidence
Before:
before-short.mp4
After:
after-short.mp4
How to test
Use disposable files. Do not confirm permanent deletion.
Expected result: arrows navigate the visible pane; hidden tabs and background completions do not steal focus; Settings stays closed behind Properties and closes on an outside click; Escape preserves heading order and file selection without closing the chooser; an open modal retains its focused control.
Related issue
Closes #1859
Reuses #1879 and the fixes already merged in #1890.