Code tab: chip + popover picker with inline file-name filter by srid · Pull Request #791 · juspay/kolu

srid · 2026-04-30T11:40:44Z

The Code browser's mode picker had three flat tabs — All, Local, Branch — that read as tabs and wasted horizontal space the file-name filter could have used. Pierre's FileTree rendered its own search header inside the tree, so the panel had two filter affordances stacked on top of each other.

This PR replaces the trio with a single chip + popover on the left and a free-text filter input on the right, all inside one toolbar row. State is unchanged — CodeTabView is still "local" | "branch" | "browse", the persisted preference shape is untouched, and the existing per-mode data-testids still resolve (now inside the popover).

┌──────────────────────────────────────────────┐
│ [⎇ Git: Local ▾]   🔍 filter files…    ×    │
├──────────────────────────────────────────────┤
│ <file tree, filtered live by the input>      │
├──────────────────────────────────────────────┤
│ <selected file's diff or content>            │
└──────────────────────────────────────────────┘

Two presenters, one toolbar

The two filter axes live in two focused components, arranged inline in CodeTab's toolbar:

ModeChipPicker — chip + popover. Pure presenter parameterized by modes: ModeOption[].
FileSearchInput — input + clear button. Just value + onChange.

Visual co-location is preserved by CodeTab's flex row; the components don't share state or know about each other.

How mode metadata flows

The host owns mode identity so adding a new mode (stash, vs-commit, etc.) touches one file:

// CodeTab.tsx
const modeOptions = createMemo<ModeOption[]>(() => {
  const ref = branchRef();
  return [
    { view: "browse", label: "All files", icon: FileBrowseIcon, ... },
    { view: "local",  group: "Git", label: "Local",  icon: GitBranchIcon, ... },
    { view: "branch", group: "Git", label: "Branch",
      hint: ref ? `vs ${ref}` : "Working tree vs branch base", ... },
  ];
});

ModeChipPicker renders this list verbatim — no per-mode knowledge bakes in.

Search ownership change

PierreFileTree accepts a searchQuery?: string prop forwarded to Pierre's setSearch() reactively. CodeTab passes search={false} to disable the built-in tree-header search and renders FileSearchInput itself in the toolbar. Result: one filter input, not two.

Shared popover scaffold

Reaching for a chip + popover surfaced that the same Portal + outside-click + Escape + viewport-clamp scaffold already lived in three other components. This PR extracts it into a single useAnchoredPopover hook (ui/useAnchoredPopover.ts) and migrates all four sites:

const { panelRef, panelStyle } = useAnchoredPopover({
  triggerRef: () => triggerEl,
  open,
  onDismiss: () => setOpen(false),
  anchor: "bottom-start",     // or "bottom-end" for right-anchored
  panelMinWidth: 240,         // viewport clamp pad
});

Migrated: SettingsPopover, RecordPopover, PrUnavailablePopover, ModeChipPicker. ~110 lines of duplicated scaffolding removed.

Refinements during review

What was off	Fix
Mode trio read as tabs	Replaced with chip + popover
Two filter inputs stacked (toolbar + tree header)	Externalized search into `PierreFileTree.searchQuery`
Mode metadata split between picker and host (Lowy)	Moved `OPTIONS` to `CodeTab`; picker parameterized by `modes` prop
Filter chrome conflated mode + search volatilities (Lowy #794)	Split into `ModeChipPicker` + `FileSearchInput`
Popover scaffold duplicated 4 times (#795)	Extracted `useAnchoredPopover`, migrated all 4 sites
Popover slipped off-screen near right edge (Hickey)	Clamped `pos.left` to viewport with 8px pad — now part of the hook
Stringly-typed `iconKind` discriminator	Pass the icon component directly via `Dynamic`
Two `classList` blocks encoding active-state	Collapsed to `group-data-[active=true]` Tailwind variants
Document listeners attached for component lifetime	Gated via reactive `createEventListener` target
`activeMode()!` non-null assertion	`<Show when={…}>{(m) => …}` callback narrowing

Try it locally

nix run github:juspay/kolu/code-tab-toggle-ui

Generated by /do on Claude Code (model claude-opus-4-7).

Restructures the Code browser's mode picker from a flat trio (All / Local / Branch) into a two-level toggle: a primary {All, Git} switch picks between the unfiltered fs view and the git-diff view; when Git is active, a sub-toggle picks the diff base {Local, Branch}. State model is unchanged — still the same three CodeTabView modes — only the UI layout changes. A `lastGitMode` signal remembers which diff base the user last chose so toggling All → Git restores it instead of always landing on "local".

Drops the row-of-buttons toolbar (which read as tabs) for a single filter-chip pattern: one chip on the left shows the active mode ("All files", "Git: Local", "Git: Branch") and opens a popover with the full set of options + their semantic hints. The freed horizontal space is taken by a free-text filter input that drives Pierre's tree filter externally via setSearch(), replacing the built-in tree-header search affordance. PierreFileTree now accepts an external `searchQuery` prop that's forwarded to Pierre's setSearch() reactively, so the host renders the input once at the top of the chrome instead of two separate search bars (one in the toolbar, one in the tree header).

Mode-identity (label, hint, test id, icon, group) lives next to the data-source switch that uses each view, instead of being split between CodeFilterBar and CodeTab. Adding a new mode (e.g. "stash") now touches one file. CodeFilterBar becomes a generic chip-+-popover presenter parameterized by a `modes` array. Subsumes the per-mode `branchRef` prop — context strings are baked into the option's `hint` at the call site (`vs origin/master`), keeping the picker's interface stable as new modes are added. Per Lowy review on the chip + popover diff.

The mode-picker popover positioned itself at the trigger's left/bottom without bounds-checking the viewport. When the right-panel was sized so the chip sat near the screen's right edge, the 240-px popover would slip off-screen. Clamp `left` so the panel stays at least 8px inside both edges. Per Hickey review on the chip + popover diff.

…ssertion The chip label used a `activeMode()!` non-null assertion after a truthy check — exactly the "TS-narrowing-but-not-quite" pattern the project rule warns against. Replace with `<Show when={…}>` callback form so the narrowing is structural, not asserted.

… component Pass the icon component itself in `ModeOption.icon` rather than a `"git" | "file"` string the picker has to dispatch on. Removes the `<Show>` branch, the `activeIsGit()` helper, and the icon imports from the picker — the host owns its own icon registry. Renders via `<Dynamic component={m().icon}>`. Also folds the chip's icon + label into a single `<Show when={activeMode()}>` pass with a callback form, dropping the `props.view` fallback (mode is always present in the modes array).

…-[active] variants The popover items encoded active-ness via twin `classList` blocks (dot color + label text color) plus `data-active` and `aria-checked`. Switch to Tailwind's `group-data-[active=true]:` variants so the active state has a single source — the `data-active` attribute on the button — and the dot/label inherit via `group-data-[active]`. Same pattern as the trigger chip already uses.

…edundant updatePos Two efficiency cleanups in the popover scaffold: * Switch from `makeEventListener` to `createEventListener` with a reactive target — the listener attaches when `open()` is true and detaches when false. The previous code attached two document-level listeners for the component's whole lifetime and short-circuited inside the handler. * Remove the click-time `updatePos()` call. The panel's ref callback already runs `updatePos()` on mount, and the panel only mounts while `open()` is true — so the click-time call was always redone immediately after.

The mode-picker test IDs (`diff-mode-{browse,local,branch}`) now live inside a popover that opens off the new `diff-filter-chip`, so the existing step definitions can no longer locate them synchronously. Two adjustments: * `When I click the Code tab mode {X}` now opens the chip first, then clicks the popover option (which closes the popover). * `Then the Code tab mode should be {X}` reads the chip's new `data-mode` attribute (which mirrors the current `view`) instead of waiting on a hidden popover entry. Adds `data-mode={view}` to the chip — small testability surface, no UX impact. Restores `code-tab.feature` + `right-panel.feature` to fully green (30 scenarios passing).

srid · 2026-04-30T12:19:31Z

Hickey/Lowy Analysis

#	Lens	Finding	Disposition
1	Hickey	Search state controlled-input antipattern	No-op (see rationale)
2	Hickey	Popover viewport clipping unhandled	Fixed in this PR (`8d6c308`)
3	Hickey	Popover position computed in ref callback before Portal mount	No-op (see rationale)
4	Lowy	Mode metadata split between `CodeFilterBar` and `CodeTab`	Fixed in this PR (`2c630db`)
5	Lowy	`branchRef` leaks mode-specific knowledge into picker props	Subsumed by Lowy #4
6	Lowy	`CodeFilterBar` conflates mode + search volatilities	Deferred #794
7	Lowy	`PierreFileTree` dual `search` + `searchQuery` props	No-op (clean seam)
8	Lowy	`searchQuery` reset placement in `CodeTab`	No-op (correct encapsulation)

Hickey rationale

The picker's structural concerns are clean: visibility (open signal), positioning (updatePos), and selection are separable, and after the PR's elegance refinements each lives in one place. Two flagged findings turned out to be false positives in this codebase:

Hickey feat: phase 0 — hello world scaffold #1 (controlled input): the recommendation was to switch to an uncontrolled input that emits via callback only. That would break two existing programmatic-reset paths — the clear-button (onSearchChange("")) and the view-change effect that resets the needle when the file set rotates. SolidJS's fine-grained reactivity also doesn't suffer the React ordering hazard the finding cited; setSearchQuery(x) where x === currentValue is a no-op via Object.is. Controlled is correct here.
Hickey Rust: use 2-space indentation #3 (ref-callback position race): the claim was that pos would be (0, 0) when the panel mounts. In practice the click handler runs updatePos() before setOpen(true), so pos is set before the panel mounts. The post-PR elegance pass actually removed the click-time call (the ref-callback updates pos on mount) and consolidated to one site (f95131a) — the redundancy that did exist is gone, the race never did.

The viewport-clipping finding (#2) was real and got a 4-line clamp on pos.left.

Lowy rationale

Two volatility axes were tangled before this PR: mode-identity (sequence: adding/removing modes) and presentation (chip layout, popover styling). They lived in two files — OPTIONS in the picker, semantics in the host — so adding a "stash" mode meant editing both.

After the fix (Lowy #4 → 2c630db), CodeTab owns the catalog and CodeFilterBar is parameterized over modes: ModeOption[]. Adding a mode is one entry in modeOptions plus a data-source switch — the picker stays untouched. This also subsumed Lowy #5: per-mode context strings now live inside each ModeOption.hint, so branchRef isn't a dedicated picker prop anymore.

The remaining structural debt (Lowy #6: chip + search live in one component despite changing for unrelated reasons) is genuinely optional — visual co-location justifies the grouping, and the seam can be split later when a follow-up touches either concern. Filed as #794 so it isn't lost.

PierreFileTree's dual search props (Lowy #7) are a clean opt-in seam: search: false + searchQuery: … is the "host owns the input" mode; default search: true keeps the built-in header for any future consumer that doesn't want to render its own.

…nput Lowy review on this PR (issue #794) flagged that CodeFilterBar conflated two unrelated volatility axes: * Mode selection — sequence volatility, changes when a mode is added or removed. * Filename filter input — activity volatility, changes when search ownership, algorithm, or placement shifts. They lived in one component because they share chrome space, but the seam was visible in this PR itself: the search-ownership change touched CodeFilterBar even though mode-selection logic didn't. Splits the filter chrome into two focused presenters: * `ModeChipPicker.tsx` — chip + popover, parameterized by `modes` * `FileSearchInput.tsx` — input + clear button, parameterized by `value` + `onChange` `CodeTab` now arranges them inline in a one-line toolbar div, so visual co-location is preserved. The `ModeOption` type re-exports from ModeChipPicker. Closes #794.

The popover scaffold — Portal + outside-click + Escape + viewport-clamped positioning — was duplicated across four sites (Settings, Record, PrUnavailable, ModeChipPicker). Extract into a single `useAnchoredPopover` hook in `ui/useAnchoredPopover.ts` and migrate all four call sites. The hook exposes `panelRef` (ref callback) and `panelStyle` (reactive accessor for `top`/`left`/`right`). Callers wire those into their `<Portal>` panel; the hook owns: * viewport-clamped positioning for `bottom-start` (left-anchored) with optional `panelMinWidth`, * `bottom-end` (right-anchored) for trigger-aligned panels, * document-level `mousedown` and `keydown` listeners attached only while `open()` is true (gated via reactive `createEventListener` target), * a `createEffect` that re-runs `updatePos()` when either the trigger ref or open state changes — covers signal-backed triggers that remount. Net diff: ~110 lines removed across the four call sites, ~95 lines added for the hook. The real win is one place to fix positioning bugs instead of four. Closes #795.

srid · 2026-04-30T12:58:47Z

Evidence

Tested on the code-tab-toggle-ui branch (HEAD bef64470) against the dev server at http://localhost:5173/.

1. Closed state — chip + search bar in `Git: Local` mode

The Code tab toolbar shows the ⎇ Git: Local ▾ chip on the left and a filter files… search input on the right. File tree lists changed files below.

2. Open popover — chip clicked

Clicking the chip opens a popover with three grouped options: All files (Browse the whole repo), Git: Local (Working tree vs HEAD) — active, marked with a colored dot — and Git: Branch (Working tree vs branch base).

3. Search input populated — live filter

Typing cod in the filter input narrows the file tree live to only CodeTab.tsx. An × clear button appears inside the input.

4. Branch mode active

After switching to Git: Branch via the popover, the chip label updates to Git: Branch ▾ and the file tree shows the full set of files changed on this branch vs the merge base — including the new FileSearchInput.tsx, ModeChipPicker.tsx, and the modified RecordPopover.tsx, CodeTab.tsx, SettingsPopover.tsx.

srid · 2026-04-30T12:59:24Z

`/do` results

Step	Status	Duration	Verification
sync	✓	0s	git fetch ok; forge=github; noGit=false
research	✓	0s	completed in prior turns; `--from post-implement`
branch	✓	0s	completed in prior turns; `--from post-implement`
implement	✓	0s	completed in prior turns; `--from post-implement`
check	✓	0s	completed in prior turns; `--from post-implement`
docs	✓	11s	README has no Code-tab section; UI refinement, no doc update needed
fmt	✓	4s	`just fmt` clean
commit	✓	3s	primary feature commits already on branch + pushed
hickey+lowy	✓	10m 12s	5 + 5 findings audited; Lowy #1 + Hickey #2 applied; Lowy #3 deferred to #794 (later applied this PR), Hickey #1/#3 No-op
police	✓	8m 50s	Rules: 1 finding fixed (no-untyped-escape-hatches). Fact-check: clean. Elegance: 3 refactors. #795 filed (later applied this PR)
test	✓	2m 42s	30 scenarios passing; step definitions rewired for chip+popover picker
create-pr	✓	1m 10s	PR title+body updated; hickey/lowy analysis comment posted
ci	✓	34m 37s	linux + darwin clean on `efb50bed`; on `bef64470` linux clean, darwin had one flaky timeout in unrelated `worktree.feature` (logged on #320)
evidence	✓	4m 22s	4 screenshots captured + posted
done	✓	0s	all steps green
Total		64m 36s

Slowest step: ci (34m 37s)

Optimization suggestions

CI dominated by ~50%: this PR ran just ci twice (after the Lowy split commit and after the popover-scaffold extraction commit) plus an extra ci::e2e retry. For follow-up touch-ups on this branch, --from ci-only skips the static gates that already passed and goes straight to the matrix.
Hickey+Lowy + police took ~19m because four review fixes (Lowy feat: phase 0 — hello world scaffold #1, Hickey feat: phase 1 — one terminal in the browser #2, police rules, three elegance commits) landed as separate commits per /do policy. That structure is worth it for reviewability but is the second-largest time bucket — consider --review-model=haiku for routine PRs to trim sub-agent latency.
darwin e2e flakiness re-observed twice — different scenarios each run, both 30s timeouts, both unrelated to PR scope. Tracked on Flaky tests log #320; if this keeps recurring, raising the cucumber default timeout for the worktree-removal and session-restore steps may be cheaper than re-running CI.
Two structural-debt issues filed during the PR were applied within it (Code tab: split CodeFilterBar into ModeChipPicker + FileSearchInput #794, Extract anchored-popover scaffold (Portal + outside-click + escape + viewport clamp) #795) after a follow-up nudge — could shave ~15 minutes by pre-committing to "Fix in this PR" on Lowy/reuse findings up front rather than file-then-flip.

Workflow completed at 2026-04-30T13:23 UTC.

srid changed the title ~~Nest Code tab modes under {All, Git} primary toggle~~ Nest Code tab modes under {All, Git} toggle Apr 30, 2026

srid marked this pull request as draft April 30, 2026 11:49

srid added 4 commits April 30, 2026 07:50

Merge remote-tracking branch 'origin/master' into code-tab-toggle-ui

8ec6821

srid mentioned this pull request Apr 30, 2026

Code tab: split CodeFilterBar into ModeChipPicker + FileSearchInput #794

Closed

srid added 4 commits April 30, 2026 08:08

srid mentioned this pull request Apr 30, 2026

Extract anchored-popover scaffold (Portal + outside-click + escape + viewport clamp) #795

Closed

srid changed the title ~~Nest Code tab modes under {All, Git} toggle~~ Code tab: chip + popover picker with inline file-name filter Apr 30, 2026

srid added 2 commits April 30, 2026 08:40

srid mentioned this pull request Apr 30, 2026

Flaky tests log #320

Open

srid marked this pull request as ready for review April 30, 2026 13:25

srid merged commit 23cd63c into master Apr 30, 2026
14 of 15 checks passed

srid deleted the code-tab-toggle-ui branch April 30, 2026 13:26

srid mentioned this pull request May 1, 2026

Code tab: diff review & file browser #514

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code tab: chip + popover picker with inline file-name filter#791

Code tab: chip + popover picker with inline file-name filter#791
srid merged 12 commits intomasterfrom
code-tab-toggle-ui

srid commented Apr 30, 2026 •

edited

Loading

Uh oh!

srid commented Apr 30, 2026

Uh oh!

srid commented Apr 30, 2026

Uh oh!

srid commented Apr 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

srid commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Two presenters, one toolbar

How mode metadata flows

Search ownership change

Shared popover scaffold

Refinements during review

Try it locally

Uh oh!

srid commented Apr 30, 2026

Hickey/Lowy Analysis

Hickey rationale

Lowy rationale

Uh oh!

srid commented Apr 30, 2026

Evidence

1. Closed state — chip + search bar in Git: Local mode

2. Open popover — chip clicked

3. Search input populated — live filter

4. Branch mode active

Uh oh!

srid commented Apr 30, 2026

/do results

Optimization suggestions

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

srid commented Apr 30, 2026 •

edited

Loading

1. Closed state — chip + search bar in `Git: Local` mode

`/do` results