Skip to content

Keep Code browser filters after selecting files#824

Draft
srid wants to merge 3 commits intomasterfrom
fix/code-browser-search-preserve-on-click
Draft

Keep Code browser filters after selecting files#824
srid wants to merge 3 commits intomasterfrom
fix/code-browser-search-preserve-on-click

Conversation

@srid
Copy link
Copy Markdown
Member

@srid srid commented May 4, 2026

The Code browser now keeps its active filename filter after file selection, including repeated clicks on the already selected filtered result. Pierre closes its internal search after row clicks, so the Solid adapter restores Kolu's host-owned searchQuery after Pierre finishes handling the click.

The adapter keeps that workaround inside @kolu/solid-pierre: CodeTab still owns the search signal, while FileTree centralizes search forwarding through the same onError path used by the other Pierre setters. The regression scenario covers the original filtered click and the repeated-click case that does not emit a selection change.

Verification

  • just check
  • just fmt
  • just test-quick features/code-tab.feature:64 (223 scenarios, 1544 steps; Cucumber merged the configured feature glob with the line filter)

Closes #817

Generated by /do on Codex (model gpt-5).

srid added 3 commits May 4, 2026 18:44
Restore host-controlled search from the FileTree click boundary so repeated clicks on the selected row cannot leave Pierre unfiltered. Centralize search forwarding through the wrapper error path.
Use waitForFunction for the Code tab filter assertion so async UI state is polled instead of sampled once.
@srid
Copy link
Copy Markdown
Member Author

srid commented May 4, 2026

Hickey/Lowy Analysis

# Lens Finding Disposition
1 Hickey Search restoration is temporal No-op
2 Hickey Microtask bypasses onError Fixed in this PR
3 Hickey Search normalization is duplicated Fixed in this PR
4 Lowy Search errors escape adapter Fixed in this PR

Hickey rationale

The change separates host-owned Code tab filter state from Pierre-owned internal search state without adding a new public abstraction. The only unavoidable coupling is temporal: Pierre 1.0.0-beta.3 closes search after row clicks, and the adapter must restore the host query after that close because Pierre does not expose a close-search opt-out.

Two refinements landed from the review. Search forwarding now uses one wrapper helper for normalization, and the asynchronous reapply path routes through the same try/catch -> onError boundary as the existing Pierre setter calls.

Lowy rationale

The volatility belongs at the @kolu/solid-pierre boundary: Pierre's row-click/search behavior can change independently from CodeTab's host-owned filter state. Keeping the workaround in the wrapper prevents Pierre's internal close behavior from leaking into CodeTab or becoming a new prop contract.

Lowy flagged that the queued setSearch call originally bypassed the adapter's error surface. The follow-up commit fixed that by centralizing search application, so all search pushes share the wrapper's stable error interface.

srid added a commit that referenced this pull request May 5, 2026
Swaps the #817 fix from `onSelectionChange` to a DOM `click` listener on
the FileTree wrapper's container, matching @srid's parallel PR #824. The
original `onSelectionChange` hook missed the re-click case: Pierre's
`#applySelection` short-circuits its `selectionVersion` increment when
the new selection equals the current one, so the callback never fires
on re-click of an already-selected row — but `controller.closeSearch()`
still runs on every row click, so the filter would be wiped with no
recovery path. The DOM-event hook fires regardless of selection-version
state.

Three calls to Pierre's `setSearch` now route through a single
`applySearchQuery` helper that owns normalization (`""` → `null`,
Pierre's contract for "no filter") and `onError` routing.

Also folds in reviewer feedback (hickey + lowy on the post-implement
diff):

- `aria-hidden` on the right-panel root (collapsed) and on each tab
  wrapper (inactive) — makes the keep-mounted contract legible to both
  future readers and assistive tech.
- Top-of-block invariant comment in CodeTab naming the
  selection-stability invariant the three guards collectively defend
  ("preserve selection across non-genuine transitions"); each guard
  documents a different churn source.
- `resetKey` separator note: `::` is collision-safe given current
  `view()` enum + `repoPath()` semantics.
- Strengthened comment on the row-click handler explicitly naming the
  two Pierre invariants the microtask-after-click pattern relies on
  (synchronous handler; `data-item-path` attribute on rows).
- Re-click assertion added to `Filter survives clicking a filtered
  result` — the case the previous implementation missed.

Approach credit: PR #824 by @srid (Codex). The DOM-listener structure,
`applySearchQuery` helper, and re-click test scenario are adapted from
that PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clicking resets search results in Code browser

1 participant