Issues list: Milestone and Labels filters should be dropdowns fed from the repo caches #416

Closed
opened 2026-09-12 20:59:50 +00:00 by crueber · 3 comments
Owner

Issues-list filter bar: convert Milestone and Labels filters to dropdowns fed from the repo caches

What's requested

On the issues list filter bar (web/src/pages/Issues.jsx), replace the two free-text inputs with dropdown pickers:

  • Milestone — single-select dropdown. Options come from the existing milestones:{o}/{r} cache the page already fetches (useData at Issues.jsx:65, ctx.repoClient.milestones.list(), TTL.milestones). Include an explicit "No milestone" option mapping to the none filter value and an unfiltered/clear option.
  • Labels — multi-select dropdown. Options come from the existing labels:{o}/{r} cache (useData at Issues.jsx:56, ctx.repoClient.labels.list(), TTL.labels). Selection maps to the same comma-separated name list the labels query param accepts today (current placeholder "labels (a,b)").

No backend change: the list endpoint's filter params (labels, milestone) are unchanged — this is pure client wiring on data the page already caches.

Evidence

  • Both inputs are bare text fields in the filter form: Labels (placeholder="labels (a,b)", Issues.jsx ~113–121) and Milestone (placeholder="milestone or none", ~131–139). Users must know label names and the none sentinel by heart.
  • The page already holds both option sets: getLabelSet (Issues.jsx:56) and getMilestoneSet (Issues.jsx:65) — shared cache entries with the Labels/Milestones pages and the thread sidebar.
  • Filter value semantics: ?milestone=<id> is the canonical per-milestone href used elsewhere (docs/features/02_issues.md, #148/#119 linked-title pattern); labels is a comma-separated name list.

Architecture notes

  • Copy the in-repo dropdown pattern, don't invent one. web/src/components/LabelPicker.jsx (08 §2, #107) is the reference: <details>/signal panel with rows, Esc closes and restores trigger focus, document-level outside-click close removed in onCleanup (watch the toggle-fight trap — the outside handler must not close-and-reopen on the trigger click), w-80 panel within the #278 viewport bound, rows as a grid so multi-word names don't wrap. Reuse LabelChip/labelColor for label row dots if it fits.
  • Milestone single-select can be a styled native <select> if it matches the design language — the State field is already a native select; consistency argues native for single-select, popover pattern for multi-select labels. Implementer's call; note it.
  • Pending/empty caches: rows render before the option caches settle (see the milestoneDisplay pending precedent at Issues.jsx:60–64) — the dropdown should either disable while pending or render after settle, never flash bare ids.
  • URL-driven state: the filter bar hydrates from search params (deep links like ?milestone=<id> from the linked-title pattern must still hydrate the dropdown correctly, including none and unknown/deleted milestone ids — fall back to showing the raw value rather than silently dropping the filter).
  • Both caches are shared, SWR-style entries — a mutation elsewhere (label created on Labels page) shows up on next TTL settle; no new invalidation contract is introduced by this change.
  • No cached-API response shape changes, so no ETag/Cache-Control concern.

Acceptance criteria

  • Milestone filter is a dropdown fed from milestones:{o}/{r}; single-select; includes "No milestone" (→ none) and clear/unfiltered.
  • Labels filter is a multi-select dropdown fed from labels:{o}/{r}; selection produces the same comma-separated labels param value the endpoint accepts today.
  • Deep links (?milestone=<id>, ?labels=a,b, ?milestone=none) hydrate the dropdowns correctly; unknown ids render as the raw value, not a dropped filter.
  • Dropdown closes on outside click and Escape (LabelPicker pattern); focus restored to trigger.
  • Works at phone widths (panel within viewport bound, #278).
  • No backend/API changes; existing filter unit tests updated/extended.
# Issues-list filter bar: convert Milestone and Labels filters to dropdowns fed from the repo caches ## What's requested On the issues list filter bar (`web/src/pages/Issues.jsx`), replace the two free-text inputs with dropdown pickers: - **Milestone** — single-select dropdown. Options come from the existing `milestones:{o}/{r}` cache the page already fetches (`useData` at Issues.jsx:65, `ctx.repoClient.milestones.list()`, TTL.milestones). Include an explicit "No milestone" option mapping to the `none` filter value and an unfiltered/clear option. - **Labels** — multi-select dropdown. Options come from the existing `labels:{o}/{r}` cache (`useData` at Issues.jsx:56, `ctx.repoClient.labels.list()`, TTL.labels). Selection maps to the same comma-separated name list the `labels` query param accepts today (current placeholder "labels (a,b)"). No backend change: the list endpoint's filter params (`labels`, `milestone`) are unchanged — this is pure client wiring on data the page already caches. ## Evidence - Both inputs are bare text fields in the filter form: Labels (`placeholder="labels (a,b)"`, Issues.jsx ~113–121) and Milestone (`placeholder="milestone or none"`, ~131–139). Users must know label names and the `none` sentinel by heart. - The page already holds both option sets: `getLabelSet` (Issues.jsx:56) and `getMilestoneSet` (Issues.jsx:65) — shared cache entries with the Labels/Milestones pages and the thread sidebar. - Filter value semantics: `?milestone=<id>` is the canonical per-milestone href used elsewhere (`docs/features/02_issues.md`, #148/#119 linked-title pattern); `labels` is a comma-separated name list. ## Architecture notes - **Copy the in-repo dropdown pattern, don't invent one.** `web/src/components/LabelPicker.jsx` (08 §2, #107) is the reference: `<details>`/signal panel with rows, Esc closes and restores trigger focus, document-level outside-click close removed in `onCleanup` (watch the toggle-fight trap — the outside handler must not close-and-reopen on the trigger click), `w-80` panel within the #278 viewport bound, rows as a grid so multi-word names don't wrap. Reuse `LabelChip`/`labelColor` for label row dots if it fits. - **Milestone single-select** can be a styled native `<select>` if it matches the design language — the State field is already a native select; consistency argues native for single-select, popover pattern for multi-select labels. Implementer's call; note it. - **Pending/empty caches**: rows render before the option caches settle (see the milestoneDisplay pending precedent at Issues.jsx:60–64) — the dropdown should either disable while pending or render after settle, never flash bare ids. - **URL-driven state**: the filter bar hydrates from `search` params (deep links like `?milestone=<id>` from the linked-title pattern must still hydrate the dropdown correctly, including `none` and unknown/deleted milestone ids — fall back to showing the raw value rather than silently dropping the filter). - Both caches are shared, SWR-style entries — a mutation elsewhere (label created on Labels page) shows up on next TTL settle; no new invalidation contract is introduced by this change. - No cached-API response shape changes, so no ETag/Cache-Control concern. ## Acceptance criteria - [ ] Milestone filter is a dropdown fed from `milestones:{o}/{r}`; single-select; includes "No milestone" (→ `none`) and clear/unfiltered. - [ ] Labels filter is a multi-select dropdown fed from `labels:{o}/{r}`; selection produces the same comma-separated `labels` param value the endpoint accepts today. - [ ] Deep links (`?milestone=<id>`, `?labels=a,b`, `?milestone=none`) hydrate the dropdowns correctly; unknown ids render as the raw value, not a dropped filter. - [ ] Dropdown closes on outside click and Escape (LabelPicker pattern); focus restored to trigger. - [ ] Works at phone widths (panel within viewport bound, #278). - [ ] No backend/API changes; existing filter unit tests updated/extended.
crueber added this to the v1 milestone 2026-09-12 21:00:04 +00:00
Author
Owner

Fixed by #418 (branch fix/issue-416): Milestone is now a native single-select from milestones:{o}/{r} (All milestones + No milestone → none); Labels is a LabelPicker-idiom multi-select popover from labels:{o}/{r} mapping to the same comma-separated param. Deep links hydrate with raw-value fallbacks; pending caches disable; no backend change. Tests: 21 new headless cases green, #415 order tests untouched-green, vite build green.

Fixed by #418 (branch fix/issue-416): Milestone is now a native single-select from milestones:{o}/{r} (All milestones + No milestone → none); Labels is a LabelPicker-idiom multi-select popover from labels:{o}/{r} mapping to the same comma-separated param. Deep links hydrate with raw-value fallbacks; pending caches disable; no backend change. Tests: 21 new headless cases green, #415 order tests untouched-green, vite build green.
Author
Owner

Review: PR #418 (fix/issue-416) — filter dropdowns

Reviewed diff main...origin/fix/issue-416 (4 files) against all 6 acceptance criteria in a scratch worktree. No browser used (per review instructions — node tests + reasoning only); no live instance touched; no backend exists in this change to verify beyond the diff.

Acceptance criteria

  1. Milestone dropdown from milestones:{o}/{r} — PASS. Native single-select (web/src/pages/Issues.jsx:286-300), options from getMilestoneSet()?.milestones (:296), "All milestones" clear (:294) + "No milestone" → none (:295). Pending disables (:289) — never a bare-id flash. Unknown/deleted ids render as a raw-value option (:297-299) with a "no longer in this repo" title (:292). Native-select choice matches the implementer's-call note (State-field consistency).
  2. Labels multi-select from labels:{o}/{r} — PASS. LabelsFilter (Issues.jsx:46-169) in the LabelPicker idiom: document-level outside-click close (:51-53) + Esc (:54-59), focus restored to trigger (:57), both listeners removed in onCleanup (:62-65). No toggle-fight — trigger lives inside root, so its clicks never read as outside (:81 + comment :36-39). w-80 panel (:100) with the shared label-drop hook, grid rows (:147), color dots, Clear row (:104-118), "All labels" unfiltered summary (:76). Selection → same comma-separated labels param via toggleLabel(parseLabelsParam(...)) + serializeLabelsParam (:224-225).
  3. Deep-link hydration — PASS. web/src/lib/issueFilters.js: parseLabelsParam keeps unknown names (never dropped), resolveMilestoneFilter binds ""/none/known verbatim and falls back to the raw value for unknown ids, pending while unsettled. Unknown labels render as removable bare rows (:119-136) marked "(no longer in this repo)".
  4. Outside-click/Esc + focus restore — PASS (see #2; test pins exactly 2 setOpen(false) sites so row toggles stay open for multi-add).
  5. Phone widths — PASS by reasoning (no browser per instructions): panel is absolute left-0 (:100); w-80 = 320px ≤ 390−16 = 374px #278 cap. Left-anchoring is correct here — this cell sits mid-grid, so right-0 would hang past the viewport's left edge. Comment (:40-43) says exactly this.
  6. No backend/API changes + tests updated/extended — PASS. No Go/SDK files in diff; query() and its params unchanged (:182-189). New web/test/unit/issues-filter-dropdowns.test.js (21 tests) covers lib mapping, hydration, close behavior, order, deps, and phone bound.

Invariants

  • #415 order intact: State/Assignee/Labels/Milestone/Refresh + grid template unchanged (Issues.jsx:249); issues-filter-order.test.js 3/3 green.
  • Shared caches, no new invalidation: same labels:{o}/{r} (:199) / milestones:{o}/{r} (:208) useData entries; invalidate only on the existing issues-key reload.
  • No new deps: imports are solid-js, @solidjs/router, relative lib only (pinned by test). AGENTS.md laws 1/8/12 hold (no new modules; no upward imports — issueFilters.js is dependency-free/pure, Issues.jsx only adds relative-lib imports; docs decision appended in docs/features/02_issues.md in the same change, accurately noting vite green + browser proof open).
  • Nit (non-blocking, no fix): parseLabelsParam dedups exact-case only while toggleLabel matches case-insensitively (labels.js:15-21) — ?labels=Bug,bug shows both spellings selected but one toggle clears both. Harmless (server sees the same label twice = AND self), documented "first spelling wins" behavior in test. Left as is.

Verification (scratch worktree /tmp/pr418, removed afterward)

  • New tests: 21/21 pass. Related (issues-filter-order, label-picker-rows, labels, milestones, issues-row-milestone, issue-state): 38/38 pass.
  • Full node --test web/test/unit/*.test.js: 847/849 — the 2 failures are smoke.test.js server-dependent tests that fail identically on main (they need a live server on 127.0.0.1:8080; something else answers 401 there; untouched per workspace rules). Pre-existing/environmental, unrelated to this PR.
  • vite build (web): green.
  • No push needed — no changes required. Main worktree left clean/read-only.

Recommendation

Ready to merge. All 6 acceptance criteria met, #415 order tests green, no backend change, docs accurate. Only open item is the live-browser proof the PR's own doc decision already declares open.

# Review: PR #418 (fix/issue-416) — filter dropdowns Reviewed diff main...origin/fix/issue-416 (4 files) against all 6 acceptance criteria in a scratch worktree. No browser used (per review instructions — node tests + reasoning only); no live instance touched; no backend exists in this change to verify beyond the diff. ## Acceptance criteria 1. **Milestone dropdown from milestones:{o}/{r} — PASS.** Native single-select (web/src/pages/Issues.jsx:286-300), options from `getMilestoneSet()?.milestones` (:296), "All milestones" clear (:294) + "No milestone" → `none` (:295). Pending disables (:289) — never a bare-id flash. Unknown/deleted ids render as a raw-value option (:297-299) with a "no longer in this repo" title (:292). Native-select choice matches the implementer's-call note (State-field consistency). 2. **Labels multi-select from labels:{o}/{r} — PASS.** `LabelsFilter` (Issues.jsx:46-169) in the LabelPicker idiom: document-level outside-click close (:51-53) + Esc (:54-59), focus restored to trigger (:57), both listeners removed in `onCleanup` (:62-65). No toggle-fight — trigger lives inside `root`, so its clicks never read as outside (:81 + comment :36-39). w-80 panel (:100) with the shared `label-drop` hook, grid rows (:147), color dots, Clear row (:104-118), "All labels" unfiltered summary (:76). Selection → same comma-separated `labels` param via `toggleLabel(parseLabelsParam(...))` + `serializeLabelsParam` (:224-225). 3. **Deep-link hydration — PASS.** `web/src/lib/issueFilters.js`: `parseLabelsParam` keeps unknown names (never dropped), `resolveMilestoneFilter` binds `""`/`none`/known verbatim and falls back to the raw value for unknown ids, `pending` while unsettled. Unknown labels render as removable bare rows (:119-136) marked "(no longer in this repo)". 4. **Outside-click/Esc + focus restore — PASS** (see #2; test pins exactly 2 `setOpen(false)` sites so row toggles stay open for multi-add). 5. **Phone widths — PASS by reasoning** (no browser per instructions): panel is `absolute left-0` (:100); w-80 = 320px ≤ 390−16 = 374px #278 cap. Left-anchoring is correct here — this cell sits mid-grid, so right-0 would hang past the viewport's left edge. Comment (:40-43) says exactly this. 6. **No backend/API changes + tests updated/extended — PASS.** No Go/SDK files in diff; `query()` and its params unchanged (:182-189). New `web/test/unit/issues-filter-dropdowns.test.js` (21 tests) covers lib mapping, hydration, close behavior, order, deps, and phone bound. ## Invariants - **#415 order intact:** State/Assignee/Labels/Milestone/Refresh + grid template unchanged (Issues.jsx:249); `issues-filter-order.test.js` 3/3 green. - **Shared caches, no new invalidation:** same `labels:{o}/{r}` (:199) / `milestones:{o}/{r}` (:208) `useData` entries; `invalidate` only on the existing issues-key reload. - **No new deps:** imports are `solid-js`, `@solidjs/router`, relative lib only (pinned by test). AGENTS.md laws 1/8/12 hold (no new modules; no upward imports — `issueFilters.js` is dependency-free/pure, `Issues.jsx` only adds relative-lib imports; docs decision appended in `docs/features/02_issues.md` in the same change, accurately noting vite green + browser proof open). - **Nit (non-blocking, no fix):** `parseLabelsParam` dedups exact-case only while `toggleLabel` matches case-insensitively (`labels.js:15-21`) — `?labels=Bug,bug` shows both spellings selected but one toggle clears both. Harmless (server sees the same label twice = AND self), documented "first spelling wins" behavior in test. Left as is. ## Verification (scratch worktree `/tmp/pr418`, removed afterward) - New tests: 21/21 pass. Related (`issues-filter-order`, `label-picker-rows`, `labels`, `milestones`, `issues-row-milestone`, `issue-state`): 38/38 pass. - Full `node --test web/test/unit/*.test.js`: 847/849 — the 2 failures are `smoke.test.js` server-dependent tests that fail identically on main (they need a live server on 127.0.0.1:8080; something else answers 401 there; untouched per workspace rules). Pre-existing/environmental, unrelated to this PR. - `vite build` (web): green. - No push needed — no changes required. Main worktree left clean/read-only. ## Recommendation **Ready to merge.** All 6 acceptance criteria met, #415 order tests green, no backend change, docs accurate. Only open item is the live-browser proof the PR's own doc decision already declares open.
Author
Owner

Fixed by PR #418 (review clean — all 8 checks pass, hydration + #278 + #415 order verified), merged. Closing.

Fixed by PR #418 (review clean — all 8 checks pass, hydration + #278 + #415 order verified), merged. Closing.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub#416
No description provided.