fix(quickfiler): stop folder-search keystrokes from stealing focus (#438)

[drmoisan]

Suggested title

fix(quickfiler): stop folder-search keystrokes from stealing focus (#438)

Summary

• Typing in the QuickFiler folder-search textbox lost keyboard focus after one to two characters, making any multi-character folder search unusable. The handler now issues a single presentation intent instead of a four-call composition that opened, closed, and re-opened the drop-down on every keystroke.
• Research found two independent focus-steal mechanisms, not the one described in the issue. A fix addressing only the reported mechanism would not have worked.
• A third defect in the same handler is also fixed: the per-keystroke highlight mutated the committed selection, leaving a stale cached folder after Escape.
• Both contract changes are strictly additive — one new IItemViewer member and one new IBreadcrumbDropDownHost overload. No existing signature is removed or altered, and every existing caller keeps its exact semantics.
• Focus-on-open is preserved for explicit gestures (mouse toggle, Down arrow, JumpToFolderDropDown); only search-driven opens become non-focusing. This is a deliberate, documented qualification of an issue Bug: quickfiler-folder-selector-dropdown #400 acceptance criterion — see Backward Compatibility.
• 67 new tests; full toolchain green; 6350/6350 tests pass; repo coverage 85.86% line / 79.29% branch.

Why

Folder search is the primary way to file an item to a folder that is not among the suggestions. Because focus left the textbox after one to two characters, the search box was effectively unusable for any multi-character query, and the view jumped to whichever folder matched the truncated string.

Root cause, confirmed against source rather than assumed:

1. Open-side steal. BreadcrumbDropDownOpenLifetime.FocusCurrentSurface calls _host.FocusPending() on a fresh open, and BreadcrumbDropDownHost.OpenAsync schedules _focusPending on a re-issued open. Opening the drop-down focuses the popup by design.
2. Close-side steal. The per-keystroke ClearFolderItems() cancelled the open selector session; BreadcrumbDropDownHost.FinishClose unconditionally invokes _focusAnchor. This mechanism was not identified in the original issue, and any fix suppressing only _focusPending would still have lost focus here.
3. Committed-selection mutation. SetFolderSelectedIndex(1) on each keystroke mutated the model selection (what the collapsed surface and GetSelectedFolder() report) and raised SelectionChanged, so the controller cached a mid-search folder. Because CancelSelector raises no SelectionChanged, Escape left that stale value behind.

The defect was the composition of "refresh on every keystroke" with "opening the drop-down takes focus" — neither part is wrong on its own.

What Changed

Core fix

TextBoxSearch_TextChanged reduces to FindFolder plus one call:

var folders = _folderHandler.FindFolder(/* ... */);
_itemViewer.PresentFolderSearchResults(folders);

Sequencing moves into the coordinator layer that owns the posted FIFO operation queue, where it can be ordered deterministically.

• New additive IItemViewer.PresentFolderSearchResults(string[]) — the single controller-facing search intent.
• BreadcrumbBridgeCoordinator.PresentSearchResults — composite: replace rows preserving the session, open the selector if closed, then apply a pending-only highlight. Reuses the existing ReconcileRowsReplaced primitive.
• Session-preserving row replacement (FolderBreadcrumbBridgeRouter.SearchPresentation.cs) — a refresh no longer closes and re-opens the popup, which removes the close-side _focusAnchor steal and the per-keystroke popup churn.
• Pending-only HighlightRow (BreadcrumbSelectionSession.Highlight.cs) — highlights without committing, so Escape restores the identity committed before the search began.
• takeFocus intent through the open pipeline — additive IBreadcrumbDropDownHost.OpenAsync(anchor, workingArea, size, bool takeFocus). The existing 3-parameter overload delegates with takeFocus: true, and default opens continue to route through it, so every existing caller and test is untouched.

Tests

• New: QfcItemController.SearchFocusRegressionTests.cs, BreadcrumbDropDownSearchIntegrationTests.cs (+.Part2), BreadcrumbDropDownOpenCoordinatorTests.Part3.cs, BreadcrumbSelectionSessionHighlightTests.cs, FolderBreadcrumbBridgeRouterReplaceItemsTests.cs.
• Exactly one existing test method was rewritten: TextBoxSearch_TextChanged_UsesInjectedFolderSearchHandler_PopulatesAndSelectsFolder, which pinned the defective composition. Its durable protections are re-asserted against the new intent, with added negative assertions.
• Three existing test files gained a purely additive member on a private test fake (required, because they hand-implement IBreadcrumbDropDownHost); one gained a partial keyword. No test method was added, removed, weakened, or altered by those edits.

Docs and evidence

Feature folder under docs/features/active/2026-08-07-quickfiler-search-keystroke-focus-steal-438/: spec, research, atomic plan, remediation plan, two review cycles, a human-verification runbook, and the full evidence tree.

Architecture / How It Fits Together

TextBoxSearch_TextChanged
  └─ IItemViewer.PresentFolderSearchResults(items)          [new, additive]
       └─ BreadcrumbBridgeCoordinator.PresentSearchResults
            ├─ router.ReplaceItemsPreservingSession(items)   → rows swap, session survives
            ├─ OpenSelector() if closed                      → latches "next open takes no focus"
            └─ session.HighlightRow(...)                     → pending only, no commit
                 └─ BreadcrumbDropDownOpenCoordinator
                      └─ IBreadcrumbDropDownHost.OpenAsync(..., takeFocus: false)   [new overload]

The latch is deterministic because SetDroppedDown-posted work and HandleSelectorOpenStateChanged-posted work execute FIFO on the same BreadcrumbPopupUiOperations queue.

Explicit-gesture paths are unchanged and still call the 3-parameter OpenAsync, which defaults to takeFocus: true.

Verification

Completed

• Fail-before / pass-after: six tests failed against the pre-fix tree with captured Moq output, then passed after. Evidence under evidence/regression-testing/.
• Full toolchain, final uninterrupted pass, every stage exit 0: CSharpier 1.2.6 format then check (1501 files, 0 violations); .NET analyzers (0 errors); nullable with TreatWarningsAsErrors (0 errors).
• Tests: 6350/6350 pass.
• Coverage: repo 85.862% line / 79.286% branch, both above the 85%/75% floors and both improved against a same-session baseline. New/changed members at or above 95.24%.
• Two review cycles. Cycle 1 raised one blocking finding (50% branch coverage on a new file); cycle 2 verified it resolved at 100% and returned zero blocking findings.
• Scope guards: the EfcViewer search path has a zero diff; BreadcrumbDropDownIntegrationTests.cs is byte-unmodified; no file exceeds the 500-line ceiling.

Recommended

• pwsh -NoProfile -Command "& ./.dotnet-sdk/dotnet.exe tool run csharpier check . ; exit $LASTEXITCODE"
• pwsh -NoProfile -Command "& msbuild TaskMaster.sln /t:Build /p:Configuration=Debug /p:Platform='Any CPU' /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true ; exit $LASTEXITCODE"
• pwsh -NoProfile -Command "& msbuild TaskMaster.sln /t:Build /p:Configuration=Debug /p:Platform='Any CPU' /p:Nullable=enable /p:TreatWarningsAsErrors=true ; exit $LASTEXITCODE"
• pwsh -NoProfile -Command "& ./scripts/vscode/Invoke-MSTestWithCoverage.ps1 -SearchRoot . ; exit $LASTEXITCODE"
• Manual, post-merge, not a merge gate: docs/features/active/2026-08-07-quickfiler-search-keystroke-focus-steal-438/runbooks/verify-search-focus-retention.runbook.md

Backward Compatibility / Migration Notes

• No breaking changes. Both interface changes are additive; no signature is removed or altered. All implementers are in-repo.
• Deliberate qualification of issue Bug: quickfiler-folder-selector-dropdown #400 AC-13. That criterion states focus enters the pending option on open. Search-driven opens are now non-focusing; explicit-gesture opens are unchanged. This is a documented, gesture-scoped refinement recorded in spec.md, not an undisclosed regression — every other mapped Bug: quickfiler-folder-selector-dropdown #400 criterion is preserved and its suites pass unmodified.
• Acceptance criterion AC-11 was amended mid-flight. As originally written it required that exactly one existing test file change, which became unsatisfiable once the new interface member forced three hand-written test fakes to implement it. It was restated to govern test methods and to enumerate the sanctioned structural edits. It still forbids weakening any test; the amendment was reviewed and judged to tighten rather than weaken the criterion.

Risks and Mitigations

• Native focus behavior is outside the managed seam. CoreWebView2 popup creation may take Win32 focus independently of managed code, and ToolStripDropDown.AutoClose behavior while typing is not unit-observable. Mitigation: a human-verification runbook covers the residual check; it is explicitly not a merge gate. All managed focus transfers are delegate invocations and are asserted at existing harnesses.
• Popup height is fixed at open time, so a later keystroke that changes the row count does not resize an already-open popup. Accepted and documented.
• Rollback: revert the merge commit. The change is additive, so reverting restores prior behavior without leaving dangling callers.

Review Guide

1. QuickFiler/Controllers/QfcItemController.EventHandlers.cs — the actual fix, ~16 lines.
2. QuickFiler/Viewers/BreadcrumbBridgeCoordinator.Search.cs — the composite and its ordering.
3. UtilitiesCS/.../FolderBreadcrumbBridgeRouter.SearchPresentation.cs and BreadcrumbSelectionSession.Highlight.cs — the host-neutral transitions.
4. QuickFiler/Viewers/BreadcrumbDropDownOpenCoordinator.cs and IBreadcrumbDropDownHost.cs — the takeFocus latch and overload; confirm default opens still use the 3-parameter path.
5. QuickFiler.Test/Controllers/QfcItemController.SearchFocusRegressionTests.cs — the primary regression.
6. Mechanical/low-signal: new partial-class files exist to respect the 500-line ceiling; the bulk of the diff is Markdown evidence and two checked-in Cobertura XML files.

Follow-ups

Filed during this work, all out of scope here:

• Bug: csharpier-documented-command-incompatible-with-pinned-version #509 — CLAUDE.md and .claude/rules/csharp.md document a CSharpier command (csharpier .) that cannot work against the pinned 1.2.6, which requires format/check subcommands.
• Bug: winformspumphost-tests-load-flaky-visible-window #511 — WinFormsPumpHost tests run a real message pump: load-flaky, show a window, and make repo-wide coverage measurement non-reproducible across runs.
• Bug: collect-pr-context-misclassifies-csharp-as-documentation #513 — the PR-context collector classifies changed C# files as documentation, which can cause the C# coverage gate to be silently skipped.
• Bug: utilitiescs-test-cs2002-duplicate-compile-entry #394 (pre-existing) — duplicate PercentageFormatterTests.cs compile entry; re-confirmed with current evidence.

Deferred, not addressed here:

• QfcItemController.EventHandlers.cs remains below the per-file coverage floor. This is pre-existing: every changed line is covered and the uncovered-line set is identical before and after. Awaiting a maintainer disposition — either a follow-up issue for the untested WinForms theme/menu handlers, or a recorded exemption.
• Two remediation-plan tasks are deliberately left unchecked and disclosed rather than checked off: a repo-wide coverage floor that compares against a cross-session constant the unmodified tree cannot reproduce, and a commit step that belonged to the orchestrator.
• Evidence timestamp labels in this feature folder were estimated rather than clock-read. Technical content and relative ordering are unaffected and reproducible; absolute times are not. Disclosed in evidence/other/timestamp-and-coverage-floor-correction.2026-08-08T19-25Z.md.

GitHub Auto-close

• Closes Bug: quickfiler-search-keystroke-focus-steal #438