From bbb90caeca3c7cbbaebcc29892a6e553bf085bc9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 14:09:18 +0000 Subject: [PATCH 1/3] fix(services): fold service-group browse nav into the filter sheet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The standalone "All / Urgent / Public MH / More" chip row above the services results duplicated the separate "Filter services" sheet already on the page. Service categories overlap, so this reuses the multi-select facet plumbing already built (but unwired) in service-core-groups.ts to render group as a proper facet — with candidate-widening counts, applied-filter chips, and clear-all support consistent with every other services filter — instead of a route-driven one-of-N nav. Deletes the now-unused ServiceGroupNav component. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01PeFHfAeXsopKgW4Qay7T4p --- docs/design-system/COMPONENTS.md | 2 +- docs/design-system/adoption-manifest.json | 2 - src/components/services/service-group-nav.tsx | 113 ---------------- .../services/services-navigator-page.tsx | 128 +++++++++++++----- tests/ui-tools.spec.ts | 14 +- 5 files changed, 109 insertions(+), 150 deletions(-) delete mode 100644 src/components/services/service-group-nav.tsx diff --git a/docs/design-system/COMPONENTS.md b/docs/design-system/COMPONENTS.md index 5d22c379bc..f2ae7f7c28 100644 --- a/docs/design-system/COMPONENTS.md +++ b/docs/design-system/COMPONENTS.md @@ -1001,7 +1001,7 @@ This generated snapshot is a local source-derived inventory. It does not assert | `SearchField` | controls | yes | yes | no | yes | no | 0 | | `SegmentedControl` | controls | yes | yes | inherited-global-root | yes | no | 8 | | `Select` | controls | yes | yes | inherited-global-root | yes | no | 2 | -| `Sheet` | layout | yes | yes | inherited-global-root | yes | no | 25 | +| `Sheet` | layout | yes | yes | inherited-global-root | yes | no | 24 | | `Skeleton` | feedback | yes | yes | inherited-global-root | yes | no | 6 | | `SourceDesignationBadge` | source | yes | yes | inherited-global-root | yes | no | 2 | | `SourceProvenance` | source | yes | yes | inherited-global-root | yes | no | 1 | diff --git a/docs/design-system/adoption-manifest.json b/docs/design-system/adoption-manifest.json index d4adc0225f..6490bfc232 100644 --- a/docs/design-system/adoption-manifest.json +++ b/docs/design-system/adoption-manifest.json @@ -1514,7 +1514,6 @@ "src/components/forms/form-detail-page.tsx", "src/components/in-page-nav/in-page-nav-header.tsx", "src/components/mode-nav/mode-nav.tsx", - "src/components/services/service-group-nav.tsx", "src/components/tools/tools-search-results-page.tsx", "src/components/ui/confirm-dialog.tsx" ], @@ -1542,7 +1541,6 @@ "src/components/forms/form-detail-page.tsx", "src/components/in-page-nav/in-page-nav-header.tsx", "src/components/mode-nav/mode-nav.tsx", - "src/components/services/service-group-nav.tsx", "src/components/tools/tools-search-results-page.tsx" ], "designSync": { diff --git a/src/components/services/service-group-nav.tsx b/src/components/services/service-group-nav.tsx deleted file mode 100644 index b0866e99f4..0000000000 --- a/src/components/services/service-group-nav.tsx +++ /dev/null @@ -1,113 +0,0 @@ -"use client"; - -import Link from "next/link"; -import { ChevronDown } from "lucide-react"; -import { useId, useState } from "react"; - -import { Sheet } from "@/components/ui/sheet"; -import { cn } from "@/components/ui-primitives"; -import { serviceCoreGroups, type ServiceCoreGroupId } from "@/lib/service-core-groups"; - -type GroupValue = ServiceCoreGroupId | null; - -export function ServiceGroupNav({ - activeGroup, - hrefForGroup, - counts, - className, -}: { - activeGroup: GroupValue; - hrefForGroup: (group: GroupValue) => string; - counts?: Partial> & { all?: number }; - className?: string; -}) { - const [moreOpen, setMoreOpen] = useState(false); - const panelId = useId(); - const phonePrimary = serviceCoreGroups.slice(0, 2); - const phoneMore = serviceCoreGroups.slice(2); - const moreActive = phoneMore.some((group) => group.id === activeGroup); - - const groupLink = (group: (typeof serviceCoreGroups)[number], phone = false) => ( - setMoreOpen(false)} - className={cn( - "inline-flex min-h-tap min-w-0 items-center justify-center gap-1.5 rounded-lg border px-3 text-xs font-bold transition focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)] sm:min-h-10", - activeGroup === group.id - ? "border-[color:var(--clinical-accent)] bg-[color:var(--clinical-accent-soft)] text-[color:var(--clinical-accent)]" - : "border-[color:var(--border)] bg-[color:var(--surface)] text-[color:var(--text-muted)] hover:border-[color:var(--border-strong)] hover:text-[color:var(--text)]", - )} - > - {phone ? group.shortLabel : group.label} - {typeof counts?.[group.id] === "number" ? ( - {counts[group.id]} - ) : null} - - ); - - return ( - - ); -} diff --git a/src/components/services/services-navigator-page.tsx b/src/components/services/services-navigator-page.tsx index 3e24327226..3120cb071c 100644 --- a/src/components/services/services-navigator-page.tsx +++ b/src/components/services/services-navigator-page.tsx @@ -30,17 +30,17 @@ import { resultFilterFacetGroup, resultFilterGroup, } from "@/components/clinical-dashboard/result-filter-control"; -import { ServiceGroupNav } from "@/components/services/service-group-nav"; import { Chip as DesignChip, type ChipStatusTone } from "@/components/ui/chip"; import { cn } from "@/components/ui-primitives"; import { useResultSort } from "@/components/use-result-sort"; import { compactBestUseTitle } from "@/lib/compact-best-use-title"; import { modeHomeDesktopComposerSlotId } from "@/lib/mode-home-composer"; import { - readServiceCoreGroup, + readServiceCoreGroupSelection, serviceCoreGroupLabel, serviceCoreGroups, - serviceMatchesCoreGroup, + serviceMatchesCoreGroupSelection, + writeServiceCoreGroupSelectionToParams, type ServiceCoreGroupId, } from "@/lib/service-core-groups"; import { @@ -410,7 +410,7 @@ export function ServicesNavigatorPage() { const searchParams = useSearchParams(); const [sortValue, setSortValue] = useResultSort(); const urlQuery = searchParams.get("q")?.trim() || searchParams.get("query")?.trim() || ""; - const activeGroup = readServiceCoreGroup(searchParams.get("group")); + const activeGroupSelection = useMemo(() => readServiceCoreGroupSelection(searchParams.get("group")), [searchParams]); const [localQuery, setLocalQuery] = useState(() => ({ urlQuery, value: urlQuery })); const query = localQuery.urlQuery === urlQuery ? localQuery.value : urlQuery; const deferredQuery = useDeferredValue(query); @@ -426,8 +426,8 @@ export function ServicesNavigatorPage() { return []; }, [deferredQuery, query, searchableRecords]); const groupedMatches = useMemo( - () => rankedMatches.filter((service) => serviceMatchesCoreGroup(service, activeGroup)), - [activeGroup, rankedMatches], + () => rankedMatches.filter((service) => serviceMatchesCoreGroupSelection(service, activeGroupSelection)), + [activeGroupSelection, rankedMatches], ); // Facet selection lives in the URL, alongside `q`/`group`, so a filtered @@ -462,28 +462,47 @@ export function ServicesNavigatorPage() { [searchableRecords, facetSelection, substanceLens], ); const activeFilterCount = - serviceFacetSelectionSize(facetSelection) + (substanceLens === "all" ? 0 : 1) + (resultScope === "all" ? 1 : 0); + serviceFacetSelectionSize(facetSelection) + + (substanceLens === "all" ? 0 : 1) + + (resultScope === "all" ? 1 : 0) + + activeGroupSelection.size; const relevanceRankMap = useMemo(() => { const map = new Map(); rankedMatches.forEach((service, index) => map.set(service.slug, index + 1)); return map; }, [rankedMatches]); + // Group-agnostic base for the group facet's own "how many if I also ticked + // this" counts — `facetBaseMatches` cannot be reused here because it is + // already narrowed by `activeGroupSelection`, which would make every + // unselected group option read as a near-empty intersection with the + // group(s) already active rather than a true widening count. + const coreGroupBaseMatches = resultScope === "all" ? searchableRecords : rankedMatches; + const coreGroupFacetedBase = useMemo( + () => filterServicesByFacets(coreGroupBaseMatches, facetSelection, substanceLens), + [coreGroupBaseMatches, facetSelection, substanceLens], + ); const groupCounts = useMemo( () => Object.fromEntries( - serviceCoreGroups.map((group) => [ - group.id, - rankedMatches.filter((service) => serviceMatchesCoreGroup(service, group.id)).length, - ]), + serviceCoreGroups.map((group) => { + const candidate = activeGroupSelection.has(group.id) + ? activeGroupSelection + : new Set([...activeGroupSelection, group.id]); + return [ + group.id, + coreGroupFacetedBase.filter((service) => serviceMatchesCoreGroupSelection(service, candidate)).length, + ]; + }), ) as Record, - [rankedMatches], + [activeGroupSelection, coreGroupFacetedBase], ); const [selectedSlugs, setSelectedSlugs] = useState([]); const selected = searchableRecords.filter((service) => selectedSlugs.includes(service.slug)); const [showComparison, setShowComparison] = useState(false); const filterPanelId = useId(); const [filterOpen, setFilterOpen] = useState(false); - const heading = query || (activeGroup ? serviceCoreGroupLabel(activeGroup) : "Browse services"); + const activeGroupLabel = activeGroupSelection.size === 1 ? serviceCoreGroupLabel([...activeGroupSelection][0]) : null; + const heading = query || (activeGroupLabel ?? "Browse services"); const accountData = useAccountData(); const [saveNotice, setSaveNotice] = useState(null); // The provider rolls back a failed mutation from its pre-request snapshot. @@ -584,16 +603,17 @@ export function ServicesNavigatorPage() { [updateFilterParams], ); - // Never touches `q`/`query`/`group` — docs/filter-contract.md section 6: - // clearing filters and clearing a search are different intentions. The - // old query-replacing suggestion rail conflated the two (clearing its - // choices also replaced the search). It is gone; this reset now changes - // only scope and narrowing dimensions. + // Never touches `q`/`query` — docs/filter-contract.md section 6: clearing + // filters and clearing a search are different intentions. The old + // query-replacing suggestion rail conflated the two (clearing its choices + // also replaced the search). `group` is now a facet like the rest (folded + // out of the standalone browse nav), so it clears here too. const clearAllFilters = useCallback(() => { updateFilterParams((params) => { for (const dimension of serviceFacetDimensions) params.delete(dimension); params.delete("substance"); params.delete("scope"); + params.delete("group"); }); }, [updateFilterParams]); @@ -607,14 +627,27 @@ export function ServicesNavigatorPage() { }); } - function hrefForGroup(group: ServiceCoreGroupId | null) { + // Only ever called with an empty escape from the zero-results state below — + // individual group values toggle through `toggleCoreGroupValue` instead. + function hrefWithGroupCleared() { const params = new URLSearchParams(searchParams.toString()); params.set("run", "1"); - if (group) params.set("group", group); - else params.delete("group"); + params.delete("group"); return `/services?${params.toString()}`; } + const toggleCoreGroupValue = useCallback( + (value: ServiceCoreGroupId) => { + updateFilterParams((params) => { + const current = readServiceCoreGroupSelection(params.get("group")); + const next = new Set(current); + if (!next.delete(value)) next.add(value); + writeServiceCoreGroupSelectionToParams(params, next); + }); + }, + [updateFilterParams], + ); + // `substance_flags` is an exact partition (measured 2026-08-12: all 219 // services carry exactly one of general/aod) — a lens, not a facet. See // src/lib/service-facets.ts for the full measurement note. @@ -649,6 +682,26 @@ export function ServicesNavigatorPage() { [facetBaseMatches, facetSelection, setSubstanceLensValue, substanceLens, substanceOptionValues], ); + // Service categories overlap (see service-core-groups.ts), so this is a + // facet — many-of-N, OR within the group — rather than the one-of-N lens + // the standalone browse nav it replaces used to be. + const coreGroupFacetGroup = useMemo( + () => + resultFilterFacetGroup({ + id: "core-group", + label: "Service group", + selected: activeGroupSelection, + options: serviceCoreGroups.map((group) => ({ + value: group.id, + label: group.label, + hint: String(groupCounts[group.id]), + disabled: groupCounts[group.id] === 0 && !activeGroupSelection.has(group.id), + })), + onToggle: toggleCoreGroupValue, + }), + [activeGroupSelection, groupCounts, toggleCoreGroupValue], + ); + // Keep the option lists and handlers stable while unrelated page state // changes (for example opening the sheet or updating the shortlist). const facetGroups = useMemo( @@ -707,8 +760,25 @@ export function ServicesNavigatorPage() { }); } } + for (const value of activeGroupSelection) { + chips.push({ + id: `core-group-${value}`, + groupLabel: "Service group", + valueLabel: serviceCoreGroupLabel(value), + onRemove: () => toggleCoreGroupValue(value), + }); + } return chips; - }, [facetSelection, resultScope, setResultScopeValue, setSubstanceLensValue, substanceLens, toggleFacetValue]); + }, [ + activeGroupSelection, + facetSelection, + resultScope, + setResultScopeValue, + setSubstanceLensValue, + substanceLens, + toggleCoreGroupValue, + toggleFacetValue, + ]); return ( -
- -
- setFilterOpen(false)} panelId={filterPanelId} testId="service-filter-panel" title="Filter services" - groups={[substanceGroup, ...facetGroups]} + groups={[substanceGroup, coreGroupFacetGroup, ...facetGroups]} onClearAll={activeFilterCount > 0 ? clearAllFilters : undefined} summary={{ count: displayedMatches.length, noun: displayedMatches.length === 1 ? "service" : "services" }} - chromeResetKey={`${deferredQuery}|${activeGroup ?? ""}|${resultScope}`} + chromeResetKey={`${deferredQuery}|${[...activeGroupSelection].sort().join(",")}|${resultScope}`} scope={ showResultScope ? { @@ -912,7 +974,7 @@ export function ServicesNavigatorPage() {

Show all services diff --git a/tests/ui-tools.spec.ts b/tests/ui-tools.spec.ts index 1513fce715..969d5252ef 100644 --- a/tests/ui-tools.spec.ts +++ b/tests/ui-tools.spec.ts @@ -1416,7 +1416,6 @@ test.describe("Clinical KB tools directory and legacy launcher", () => { const referralProgress = page.getByRole("navigation", { name: "Referral progress" }); await expect(referralProgress).toBeVisible(); await expect(referralProgress.locator('[aria-current="step"]')).toHaveText("Search"); - await expect(page.getByRole("navigation", { name: "Service groups" })).toBeVisible(); await expect(page.getByTestId("services-shortlist-bar")).toHaveCount(0); // The row is compact by contract: the Catchment/Eligibility/Cost strip @@ -1431,6 +1430,19 @@ test.describe("Clinical KB tools directory and legacy launcher", () => { // Exercise a real facet, then clear only that facet while preserving q. await page.getByTestId("service-filter-trigger-desktop").click(); const filterPanel = page.getByTestId("service-filter-panel"); + + // The old standalone "Service groups" browse nav is folded into this + // sheet as a facet (ledger follow-up to #163) rather than a separate + // route-driven row above the results. + await expect(filterPanel.getByRole("button", { name: /^Service group/ })).toBeVisible(); + await filterPanel.getByRole("button", { name: /^Service group/ }).click(); + const urgentGroupFacet = filterPanel.getByRole("button", { name: /^Crisis & urgent/ }); + await expect(urgentGroupFacet).toBeVisible(); + await urgentGroupFacet.click(); + await expect(page).toHaveURL(/group=urgent/); + await urgentGroupFacet.click(); + await expect(page).not.toHaveURL(/group=urgent/); + await filterPanel.getByRole("button", { name: /^Acuity/ }).click(); const crisisFacet = filterPanel.getByRole("button", { name: /^Crisis \/ urgent/ }); await expect(crisisFacet).toBeVisible(); From 9cbf76f89b1ecf4113eb1e4f72820ce459fd2fac Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 14:13:40 +0000 Subject: [PATCH 2/3] chore(ledger): record PR #2040 review (services group facet fold-in) Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01PeFHfAeXsopKgW4Qay7T4p --- ...91df3e8b00a440f4fca3019fc74ac081bec1630d13a1540b0a1.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/10685ba8c4bac91df3e8b00a440f4fca3019fc74ac081bec1630d13a1540b0a1.record.md diff --git a/docs/branch-review-records/10685ba8c4bac91df3e8b00a440f4fca3019fc74ac081bec1630d13a1540b0a1.record.md b/docs/branch-review-records/10685ba8c4bac91df3e8b00a440f4fca3019fc74ac081bec1630d13a1540b0a1.record.md new file mode 100644 index 0000000000..1d8edc73fc --- /dev/null +++ b/docs/branch-review-records/10685ba8c4bac91df3e8b00a440f4fca3019fc74ac081bec1630d13a1540b0a1.record.md @@ -0,0 +1 @@ +| 2026-08-17 | PR-2040 | bbb90caeca3c7cbbaebcc29892a6e553bf085bc9 | src/components/services/services-navigator-page.tsx, src/components/services/service-group-nav.tsx (deleted), tests/ui-tools.spec.ts, docs/design-system/* | OPENED PR #2040: folded the standalone services browse nav (All/Urgent/Public MH/More) into the Filter services sheet as a multi-select facet, reusing previously-unwired plumbing in service-core-groups.ts. Deleted ServiceGroupNav. Focused+broader Vitest (126 tests) green, typecheck/lint/design-system-contract clean, manual Chromium walkthrough confirmed correct rendering and URL toggling. | test:focused (81 passed), targeted vitest sweep (45 passed), typecheck, eslint, check:design-system-contract, manual browser walkthrough | From c8e171f504b6eadcfe99cf724aa03d6f5bca43c8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 01:55:58 +0000 Subject: [PATCH 3/3] fix(services): apply group selection filter in scope=all path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the `scope=all` branch, `facetBaseMatches` was set directly to `searchableRecords`, bypassing `activeGroupSelection`. This meant the group facet appeared active but did not narrow displayed results or the `allScopedCount` when the user switched to "All services". Add `groupedAllRecords` — the full catalogue filtered by `serviceMatchesCoreGroupSelection` — and use it as the base for both `facetBaseMatches` (scope=all) and `allScopedCount`, so the group facet consistently narrows results and counts regardless of scope. `coreGroupBaseMatches` intentionally remains group-agnostic (it is the widening-count base for the facet's own option counts). Fixes the P1 finding from the Copilot code review. Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com> --- src/components/services/services-navigator-page.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/components/services/services-navigator-page.tsx b/src/components/services/services-navigator-page.tsx index 3120cb071c..64ed809869 100644 --- a/src/components/services/services-navigator-page.tsx +++ b/src/components/services/services-navigator-page.tsx @@ -441,8 +441,15 @@ export function ServicesNavigatorPage() { // without discarding the query. Gated on the UNFACETED result set so the // segment does not flicker away as facets are applied — narrowing with // facets only ever makes "catalogue > results" more true, never less. + // Apply the group selection to the full catalogue so that scope=all also + // honours the group facet — without this, switching to "All services" while + // a group is selected would show unfiltered results and an inconsistent count. + const groupedAllRecords = useMemo( + () => searchableRecords.filter((service) => serviceMatchesCoreGroupSelection(service, activeGroupSelection)), + [activeGroupSelection, searchableRecords], + ); const showResultScope = searchableRecords.length > groupedMatches.length; - const facetBaseMatches = resultScope === "all" ? searchableRecords : groupedMatches; + const facetBaseMatches = resultScope === "all" ? groupedAllRecords : groupedMatches; const facetedMatches = useMemo( () => filterServicesByFacets(facetBaseMatches, facetSelection, substanceLens), [facetBaseMatches, facetSelection, substanceLens], @@ -458,8 +465,8 @@ export function ServicesNavigatorPage() { [groupedMatches, facetSelection, substanceLens], ); const allScopedCount = useMemo( - () => filterServicesByFacets(searchableRecords, facetSelection, substanceLens).length, - [searchableRecords, facetSelection, substanceLens], + () => filterServicesByFacets(groupedAllRecords, facetSelection, substanceLens).length, + [groupedAllRecords, facetSelection, substanceLens], ); const activeFilterCount = serviceFacetSelectionSize(facetSelection) +