diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx index 1ad62ed6..350876ed 100644 --- a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx @@ -482,6 +482,7 @@ export function AgentsPage() { rows={rows} rowKey={(a) => a.name} emptyMessage="no agents yet — the swarm-wide identity store has no agents registered on any hive" + storageKey="swarm-ui:agents:table-filters" /> ) : null} r.name} + storageKey="swarm-ui:components-demo:table-filters" /> - r.name} /> +
r.name} + storageKey="swarm-ui:components-demo:table-filters-empty" + /> diff --git a/frontend/packages/swarm-ui/src/pages/HivesPage.tsx b/frontend/packages/swarm-ui/src/pages/HivesPage.tsx index 1ee81b32..4d0deda4 100644 --- a/frontend/packages/swarm-ui/src/pages/HivesPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/HivesPage.tsx @@ -144,7 +144,12 @@ export function HivesPage() { ) : null} {!error && hives === null ?

loading…

: null} {hives ? ( -
h.name} /> +
h.name} + storageKey="swarm-ui:hives:table-filters" + /> ) : null} ); diff --git a/frontend/packages/swarm-ui/src/pages/IssueReportPage.css b/frontend/packages/swarm-ui/src/pages/IssueReportPage.css index 1419727d..b69e7b20 100644 --- a/frontend/packages/swarm-ui/src/pages/IssueReportPage.css +++ b/frontend/packages/swarm-ui/src/pages/IssueReportPage.css @@ -1,11 +1,13 @@ /* — the repo-picker + hide-blocked toggle sit in one row (mirrors CreateAgentForm's row/wrap pattern: flex-wrap so narrow - viewports stack instead of overflowing), the label chips get their - own row below since the set is open-ended and shouldn't fight the - controls row for width. Sort buttons live inside the table's own - ` + + + ) : ( + visibleRows.map((row) => ( + + {columns.map((c) => ( + + ))} + + )) + )} + +
` cells (Table.tsx's `header` now accepts real markup for - exactly this) so they inherit the table's header styling for free — - only the pointer cursor + no-underline reset is this page's to own. */ + viewports stack instead of overflowing). The label filter used to + have its own chip row here; folded into the "labels" column's own + `Table` filter (`ui-table-filter-multiselect` in Table.css) instead, + so there's nothing page-specific left to style for it. Sort buttons + live inside the table's own `` cells (Table.tsx's `header` now + accepts real markup for exactly this) so they inherit the table's + header styling for free — only the pointer cursor + no-underline + reset is this page's to own. */ .issue-report-controls { display: flex; flex-wrap: wrap; @@ -19,19 +21,6 @@ gap: 0.4em; cursor: pointer; } -.issue-report-labels { - display: flex; - flex-wrap: wrap; - gap: 0.6em; - margin-bottom: 1em; -} -.issue-report-label-chip { - display: flex; - align-items: center; - gap: 0.35em; - cursor: pointer; - color: var(--muted); -} .issue-report-sort-btn { background: none; border: none; diff --git a/frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx b/frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx index 75045841..1c95de8d 100644 --- a/frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx @@ -9,8 +9,8 @@ // `transitively_blocks_count` all arrive pre-resolved per row, so // nothing here does its own dependency-graph walk. // -// Sorting and the label/hide-blocked filters are client-side over -// whatever's currently loaded — only the repo selection re-fetches. +// Sorting and the hide-blocked/label/etc. filters are all client-side +// over whatever's currently loaded — only the repo selection re-fetches. // Default sort is `depended_on_by_count` descending: mara's own framing // ("i would also like to rank them by how many issues depend on them") // reads as the report's headline ordering, not just one more column. @@ -21,11 +21,12 @@ // be instant), so it fetches once per repo-selection change instead of // polling. // -// Sort/filter state persists via the shared `useLocalSetting` hook (same +// Sort state persists via the shared `useLocalSetting` hook (same // plumbing the theme/motion overrides use) — mara: "local storage" over -// URL params. `labelFilter` is `string[]`, not `Set`: a `Set` -// serializes to `"{}"` through `JSON.stringify` and silently loses its -// contents, which is exactly what this hook round-trips through. +// URL params. The label filter used to be this page's own bespoke +// checkbox row — folded into the "labels" column's own `Table` filter +// (`filterMode: "multiselect"`) per mara's "fold filters into table" +// request, one mechanism instead of two. import { useEffect, useMemo, useState } from "preact/hooks"; import { ApiErrorPanel } from "@hive/shared/api-error-panel.js"; import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js"; @@ -71,9 +72,12 @@ const ALL_REPOS = ""; // page — collides with these by accident. const REPO_FILTER_KEY = "swarm-ui:issue-report:repo-filter"; const HIDE_BLOCKED_KEY = "swarm-ui:issue-report:hide-blocked"; -const LABEL_FILTER_KEY = "swarm-ui:issue-report:label-filter"; const SORT_KEY_KEY = "swarm-ui:issue-report:sort-key"; const SORT_DIR_KEY = "swarm-ui:issue-report:sort-dir"; +// `Table`'s own filter state (repo/labels/etc. are handled separately +// above/below — this key is just for the generic per-column filters, +// e.g. title/assignees/blocked). +const TABLE_FILTERS_KEY = "swarm-ui:issue-report:table-filters"; function splitRepo(repo: string): { org: string; name: string } | null { const i = repo.indexOf("/"); @@ -145,10 +149,6 @@ export function IssueReportPage() { HIDE_BLOCKED_KEY, false, ); - const [labelFilter, setLabelFilter] = useLocalSetting( - LABEL_FILTER_KEY, - [], - ); const [sortKey, setSortKey] = useLocalSetting( SORT_KEY_KEY, "depended_on_by_count", @@ -180,7 +180,8 @@ export function IssueReportPage() { }, []); // The one fetch this page re-runs on state change — everything else - // (sort, hide-blocked, label filter) works over what's already loaded. + // (sort, hide-blocked, the table's own per-column filters) works over + // what's already loaded. useEffect(() => { let cancelled = false; setLoading(true); @@ -210,26 +211,14 @@ export function IssueReportPage() { }; }, [repoFilter]); - // Label choices are derived from whatever's currently loaded, not a - // separate endpoint — naturally scoped to "all repos" or one repo - // depending on the current selection, and never asks for a label that - // isn't actually present in the report. - const allLabels = useMemo(() => { - const s = new Set(); - for (const row of rows ?? []) for (const l of row.labels) s.add(l); - return [...s].sort(); - }, [rows]); - const visibleRows = useMemo(() => { let out = rows ?? []; if (hideBlocked) out = out.filter((r) => !r.blocked); - if (labelFilter.length > 0) - out = out.filter((r) => r.labels.some((l) => labelFilter.includes(l))); return [...out].sort((a, b) => { const c = compareRows(a, b, sortKey); return sortDir === "asc" ? c : -c; }); - }, [rows, hideBlocked, labelFilter, sortKey, sortDir]); + }, [rows, hideBlocked, sortKey, sortDir]); function onSort(key: SortKey) { if (key === sortKey) { @@ -240,14 +229,6 @@ export function IssueReportPage() { } } - function toggleLabel(label: string) { - setLabelFilter( - labelFilter.includes(label) - ? labelFilter.filter((l) => l !== label) - : [...labelFilter, label], - ); - } - // `aria-sort` for a sortable column's `` — 'none' while sortable // but not the active column, the real direction while it is. Screen // readers announce this; the ▲/▼ glyph in `SortHeader` is `aria-hidden` @@ -315,6 +296,8 @@ export function IssueReportPage() { key: "labels", header: "labels", render: (r) => (r.labels.length ? r.labels.join(", ") : "—"), + filterValues: (r) => r.labels, + filterMode: "multiselect", }, { key: "assignees", @@ -414,20 +397,6 @@ export function IssueReportPage() { hide blocked (open dependency) - {allLabels.length ? ( -
- {allLabels.map((l) => ( - - ))} -
- ) : null} {error ? ( `${r.repo}#${r.number}`} emptyMessage="no issues match the current filters" + storageKey={TABLE_FILTERS_KEY} /> ) : null} diff --git a/frontend/packages/swarm-ui/src/ui/table/Table.css b/frontend/packages/swarm-ui/src/ui/table/Table.css index 9f434e8c..6a0a8d1b 100644 --- a/frontend/packages/swarm-ui/src/ui/table/Table.css +++ b/frontend/packages/swarm-ui/src/ui/table/Table.css @@ -141,3 +141,60 @@ font: inherit; font-size: 0.9em; } + +/* `filterMode: "multiselect"` — a scrollable checkbox list rather than + a `` — * today's only behavior, unaffected if you don't set this. @@ -78,8 +87,38 @@ export interface TableColumn { * live rows rather than a hardcoded enum means the list never offers * a choice that would match zero rows. Ignored unless `filterValue` * is also set. + * `"multiselect"`: a checkbox list, same live-rows-derived option + * source as `"select"` but over `filterValues` and matching on any + * overlap rather than one exact value — for a column whose row value + * is itself a set (e.g. labels). Ignored unless `filterValues` is set. */ - filterMode?: "text" | "select"; + filterMode?: "text" | "select" | "multiselect"; +} + +// One filter's full state: `value` for `"text"`/`"select"`, `values` for +// `"multiselect"` (the other is left at its default and ignored), plus a +// `negate` flag consulted regardless of mode (mara: "you should be able +// to change if the filter is negated or not — search for this text vs +// exclude it"). A column with no entry here behaves identically to the +// all-defaults entry below — `getFilter` returns this same shape either +// way, so callers never need an `undefined` branch. +interface ColumnFilterState { + value: string; + values: string[]; + negate: boolean; +} +const EMPTY_FILTER: ColumnFilterState = { + value: "", + values: [], + negate: false, +}; + +function isFilterActive(f: ColumnFilterState | undefined): boolean { + return f !== undefined && (f.value !== "" || f.values.length > 0); +} + +function isFilterable(c: TableColumn): boolean { + return c.filterValue !== undefined || c.filterValues !== undefined; } type SortDir = "asc" | "desc"; @@ -97,6 +136,7 @@ export function Table({ rows, rowKey, emptyMessage, + storageKey, }: { columns: TableColumn[]; rows: T[]; @@ -107,14 +147,27 @@ export function Table({ // table entirely) has nothing useful to add here, so this stays // opt-in rather than every table growing a mandatory default string. emptyMessage?: ComponentChildren; + // Namespaces this table's filter state in `localStorage` (mara: + // "tables should remember their filters in general") — required, not + // optional, so every table gets persistence for free and no caller can + // forget it; a stray collision between two tables sharing a key is a + // caller bug worth a distinct, unique string, same as any other + // storage key in this codebase (see `IssueReportPage`'s own + // `swarm-ui:issue-report:…` keys). Sort intentionally stays + // `useState` below, not persisted — only asked for filters. + storageKey: string; }) { // One active sort column at a time, same as any spreadsheet — a // multi-column sort is real complexity (tie-break order, a UI to // express it) nothing here has asked for yet. const [sort, setSort] = useState<{ key: string; dir: SortDir } | null>(null); - // Filter text per filterable column key, only populated for a column - // whose input the operator has actually typed into. - const [filters, setFilters] = useState>({}); + // Filter state per filterable column key, only populated for a column + // the operator has actually touched. Persisted (see `storageKey` + // above), same `useLocalSetting` plumbing `IssueReportPage` already + // used for its own now-folded-in label filter. + const [filters, setFilters] = useLocalSetting< + Record + >(storageKey, {}); // Which column's filter popover is open, if any — at most one at a // time (opening a second closes the first) so the header row never // shows more than one panel at once. @@ -173,7 +226,33 @@ export function Table({ }; }, [openFilterKey]); - const activeFilters = Object.entries(filters).filter(([, v]) => v !== ""); + const activeFilters = Object.entries(filters).filter(([, f]) => + isFilterActive(f), + ); + + function getFilter(key: string): ColumnFilterState { + return filters[key] ?? EMPTY_FILTER; + } + function updateFilter(key: string, patch: Partial) { + setFilters({ + ...filters, + [key]: { ...getFilter(key), ...patch }, + }); + } + function toggleFilterValue(key: string, value: string) { + const current = getFilter(key).values; + updateFilter(key, { + values: current.includes(value) + ? current.filter((v) => v !== value) + : [...current, value], + }); + } + // mara: "have a small reset filters btn" — one button clears every + // column's filter at once rather than hunting down each popover + // individually. + function resetFilters() { + setFilters({}); + } // Close on outside click or Escape — same contract `Dropdown` gives // its own popover (../.../shared/src/dropdown/Dropdown.tsx), but @@ -205,11 +284,20 @@ export function Table({ let out = rows; if (activeFilters.length > 0) { out = out.filter((row) => - activeFilters.every(([key, query]) => { + activeFilters.every(([key, f]) => { const col = columns.find((c) => c.key === key); - const value = col?.filterValue?.(row) ?? ""; - if (col?.filterMode === "select") return value === query; - return value.toLowerCase().includes(query.toLowerCase()); + let matches: boolean; + if (col?.filterMode === "multiselect") { + const rowValues = col.filterValues?.(row) ?? []; + matches = f.values.some((v) => rowValues.includes(v)); + } else if (col?.filterMode === "select") { + const value = col.filterValue?.(row) ?? ""; + matches = value === f.value; + } else { + const value = col?.filterValue?.(row) ?? ""; + matches = value.toLowerCase().includes(f.value.toLowerCase()); + } + return f.negate ? !matches : matches; }), ); } @@ -260,6 +348,17 @@ export function Table({ ); } + // Same reasoning as `selectOptionsFor` above, over `filterValues` + // (a row contributes every one of its own values, not just one). + function multiselectOptionsFor(c: TableColumn): string[] { + if (!c.filterValues) return []; + const values = new Set(); + for (const row of rows) for (const v of c.filterValues(row)) values.add(v); + return Array.from(values).sort((a, b) => + a.localeCompare(b, undefined, { sensitivity: "base", numeric: true }), + ); + } + // Only one popover is ever open at a time, so one ref (rather than a // per-column ref map) is enough to focus whichever control just // mounted — typing immediately after the click that opened it, @@ -282,138 +381,200 @@ export function Table({ return `filter by ${typeof c.header === "string" ? c.header : c.key}`; } - function renderFilterControl(c: TableColumn) { - if (c.filterMode === "select") { - return ( - - ); - } + // mara: "you should be able to change if the filter is negated or not + // (eg search for this text vs exclude it)" — one toggle, consulted the + // same way regardless of which control above it produced the match. + function renderNegateToggle(c: TableColumn) { + const f = getFilter(c.key); return ( - { - const v = (e.target as HTMLInputElement).value; - setFilters((prev) => ({ ...prev, [c.key]: v })); - }} - /> + ); } + function renderFilterControl(c: TableColumn) { + if (c.filterMode === "multiselect") { + const f = getFilter(c.key); + return ( + <> +
+ {multiselectOptionsFor(c).map((v) => ( + + ))} +
+ {renderNegateToggle(c)} + + ); + } + if (c.filterMode === "select") { + return ( + <> + + {renderNegateToggle(c)} + + ); + } + return ( + <> + { + const v = (e.target as HTMLInputElement).value; + updateFilter(c.key, { value: v }); + }} + /> + {renderNegateToggle(c)} + + ); + } + + const anyActiveFilter = activeFilters.length > 0; + return ( -
- - - - {columns.map((c) => { - const hasFilterValue = (filters[c.key] ?? "") !== ""; - const filterOpen = openFilterKey === c.key; - const headerContent = c.sortBy ? ( - - ) : ( - c.header - ); - return ( - - ); - })} - - - - {rows.length === 0 && emptyMessage ? ( + <> + {anyActiveFilter ? ( + + ) : null} +
+
{ - if (!c.filterValue) return; - if (el) thRefs.current.set(c.key, el); - else thRefs.current.delete(c.key); - }} - > - {headerContent} - {c.filterValue ? ( - - ) : null} -
+ - + {columns.map((c) => { + const hasFilterValue = isFilterActive(filters[c.key]); + const filterOpen = openFilterKey === c.key; + const headerContent = c.sortBy ? ( + + ) : ( + c.header + ); + return ( + + ); + })} - ) : rows.length > 0 && visibleRows.length === 0 ? ( - // Distinct from `emptyMessage` — real rows exist, the active - // filter(s) just matched none of them, not "there's nothing - // here at all". - - - - ) : ( - visibleRows.map((row) => ( - - {columns.map((c) => ( - - ))} + + + {rows.length === 0 && emptyMessage ? ( + + - )) - )} - -
- {emptyMessage} - { + if (!isFilterable(c)) return; + if (el) thRefs.current.set(c.key, el); + else thRefs.current.delete(c.key); + }} + > + {headerContent} + {isFilterable(c) ? ( + + ) : null} +
- no rows match the current filter -
- {c.render(row)} -
+ {emptyMessage} +
+ ) : rows.length > 0 && visibleRows.length === 0 ? ( + // Distinct from `emptyMessage` — real rows exist, the active + // filter(s) just matched none of them, not "there's nothing + // here at all". +
+ no rows match the current filter +
+ {c.render(row)} +
+ {openFilterKey !== null && popoverPos ? createPortal( (() => { @@ -437,6 +598,6 @@ export function Table({ document.body, ) : null} - + ); }