Issues list: Milestone and Labels filters should be dropdowns fed from the repo caches #416
Labels
No labels
actions
bug
cli
duplicate
enhancement
fork
forum
git storage
help wanted
insights
invalid
issues
moderation
oidc
ownership transfer
packages
pr/merge protection rules
projects
pull requests
question
releases
sponsorships
tags
webhooks
wiki
wontfix
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
crueber/walhub#416
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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:milestones:{o}/{r}cache the page already fetches (useDataat Issues.jsx:65,ctx.repoClient.milestones.list(), TTL.milestones). Include an explicit "No milestone" option mapping to thenonefilter value and an unfiltered/clear option.labels:{o}/{r}cache (useDataat Issues.jsx:56,ctx.repoClient.labels.list(), TTL.labels). Selection maps to the same comma-separated name list thelabelsquery 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
placeholder="labels (a,b)", Issues.jsx ~113–121) and Milestone (placeholder="milestone or none", ~131–139). Users must know label names and thenonesentinel by heart.getLabelSet(Issues.jsx:56) andgetMilestoneSet(Issues.jsx:65) — shared cache entries with the Labels/Milestones pages and the thread sidebar.?milestone=<id>is the canonical per-milestone href used elsewhere (docs/features/02_issues.md, #148/#119 linked-title pattern);labelsis a comma-separated name list.Architecture notes
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 inonCleanup(watch the toggle-fight trap — the outside handler must not close-and-reopen on the trigger click),w-80panel within the #278 viewport bound, rows as a grid so multi-word names don't wrap. ReuseLabelChip/labelColorfor label row dots if it fits.<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.searchparams (deep links like?milestone=<id>from the linked-title pattern must still hydrate the dropdown correctly, includingnoneand unknown/deleted milestone ids — fall back to showing the raw value rather than silently dropping the filter).Acceptance criteria
milestones:{o}/{r}; single-select; includes "No milestone" (→none) and clear/unfiltered.labels:{o}/{r}; selection produces the same comma-separatedlabelsparam value the endpoint accepts today.?milestone=<id>,?labels=a,b,?milestone=none) hydrate the dropdowns correctly; unknown ids render as the raw value, not a dropped filter.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.
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
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).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 inonCleanup(:62-65). No toggle-fight — trigger lives insideroot, so its clicks never read as outside (:81 + comment :36-39). w-80 panel (:100) with the sharedlabel-drophook, grid rows (:147), color dots, Clear row (:104-118), "All labels" unfiltered summary (:76). Selection → same comma-separatedlabelsparam viatoggleLabel(parseLabelsParam(...))+serializeLabelsParam(:224-225).web/src/lib/issueFilters.js:parseLabelsParamkeeps unknown names (never dropped),resolveMilestoneFilterbinds""/none/known verbatim and falls back to the raw value for unknown ids,pendingwhile unsettled. Unknown labels render as removable bare rows (:119-136) marked "(no longer in this repo)".setOpen(false)sites so row toggles stay open for multi-add).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.query()and its params unchanged (:182-189). Newweb/test/unit/issues-filter-dropdowns.test.js(21 tests) covers lib mapping, hydration, close behavior, order, deps, and phone bound.Invariants
issues-filter-order.test.js3/3 green.labels:{o}/{r}(:199) /milestones:{o}/{r}(:208)useDataentries;invalidateonly on the existing issues-key reload.solid-js,@solidjs/router, relative lib only (pinned by test). AGENTS.md laws 1/8/12 hold (no new modules; no upward imports —issueFilters.jsis dependency-free/pure,Issues.jsxonly adds relative-lib imports; docs decision appended indocs/features/02_issues.mdin the same change, accurately noting vite green + browser proof open).parseLabelsParamdedups exact-case only whiletoggleLabelmatches case-insensitively (labels.js:15-21) —?labels=Bug,bugshows 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)issues-filter-order,label-picker-rows,labels,milestones,issues-row-milestone,issue-state): 38/38 pass.node --test web/test/unit/*.test.js: 847/849 — the 2 failures aresmoke.test.jsserver-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.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.
Fixed by PR #418 (review clean — all 8 checks pass, hydration + #278 + #415 order verified), merged. Closing.