diff --git a/src/components/insights/__tests__/metric-target-summary.test.tsx b/src/components/insights/__tests__/metric-target-summary.test.tsx index 9b9aaa980..3ddf78249 100644 --- a/src/components/insights/__tests__/metric-target-summary.test.tsx +++ b/src/components/insights/__tests__/metric-target-summary.test.tsx @@ -193,43 +193,23 @@ describe("", () => { expect(html).toContain("30-day average: 122/76 mmHg"); }); - it("renders the inline adjust-target button instead of a /targets link", () => { + it("keeps the adjust-target affordance out of the card body", () => { + // The "Adjust target range" affordance moved to the page-header gear + // (`` via `TargetAdjustProvider`); the card is now + // a read surface, so neither the old in-card button nor a /targets + // route link appears in its markup. const html = renderWith("weight", WEIGHT_DATA); - // v1.8.6 — the /targets page is deprecated; the editor moved inline, - // so the footer is a button that opens the sheet, not a route link. - expect(html).toContain('data-slot="metric-target-adjust"'); - expect(html).toContain("Adjust target range"); + expect(html).not.toContain('data-slot="metric-target-adjust"'); expect(html).not.toContain('href="/targets"'); }); - it("keeps the inline editor lazy-mounted while the panel is closed", () => { - // The panel mounts alongside itself; - // the sheet body (and its TanStack Query hooks) only instantiate once - // the button flips the open flag, so a closed panel paints no editor. + it("does not mount the target editor inside the card", () => { + // The editor sheet is owned by the provider, opened from the header + // gear — never mounted by the card itself. const html = renderWith("weight", WEIGHT_DATA); expect(html).not.toContain('data-slot="target-edit-sheet"'); }); - it("renders the adjust-target affordance as a button, not a navigation link", () => { - // The migration's load-bearing change: the affordance is a local - // button (opens the inline sheet) rather than an / routing to - // the deprecated /targets page. - const html = renderWith("weight", WEIGHT_DATA); - const button = html.match(/]*data-slot="metric-target-adjust"[^>]*/)?.[0] ?? ""; - expect(button).toContain('type="button"'); - expect(html).not.toContain('href="/targets"'); - }); - - it("threads the diastolic range into the editor for blood pressure", () => { - // The panel passes `bpDiastolic.range` as the sheet's - // `initialDiastolicRange`. Render the panel for BP and confirm the - // adjust button is present so the BP editor is reachable inline (the - // sheet body itself portals, so it isn't part of the static markup). - const html = renderWith("blood-pressure", BP_DATA); - expect(html).toContain('data-slot="metric-target-adjust"'); - expect(html).not.toContain('href="/targets"'); - }); - it("suppresses the share + strip when the route flags insufficient data", () => { const html = renderWith("weight", { ...WEIGHT_DATA, diff --git a/src/components/insights/metric-target-summary.tsx b/src/components/insights/metric-target-summary.tsx index 71792ac8d..da6709f4a 100644 --- a/src/components/insights/metric-target-summary.tsx +++ b/src/components/insights/metric-target-summary.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useEffect } from "react"; import { useQuery } from "@tanstack/react-query"; import { ExternalLink, Target } from "lucide-react"; @@ -11,7 +11,7 @@ import { convertGlucose, resolveGlucoseUnit } from "@/lib/glucose"; import { RangeBar } from "@/components/targets/range-bar"; import { ConsistencyStrip } from "@/components/targets/consistency-strip"; import { TargetStatusPill } from "@/components/targets/target-status-pill"; -import { TargetEditSheet } from "@/components/targets/target-edit-sheet"; +import { useTargetAdjust } from "@/lib/insights/target-adjust-context"; import { getTargetSourceLink } from "@/lib/targets/source-link"; /** @@ -35,9 +35,13 @@ import { getTargetSourceLink } from "@/lib/targets/source-link"; * (the spatial answer). BP renders the second diastolic bar. * • Row 3: in-target share % + the 30-day average. * • Row 4: `` — the last seven days at a glance. - * • Footer: an "Adjust target range" button that opens the - * `` inline — this panel is the home of - * target editing. + * • Footer: the guideline source link. + * + * The panel is a read surface: editing the target range moved to a gear + * button in the page header (``). Each panel registers its + * editable target with the `TargetAdjustProvider` (see + * `target-adjust-context.tsx`); the header gear opens the per-metric + * `` the provider owns. * * Each insights slug maps to one target `type`, except blood glucose * which maps to up to four per-context cards (fasting / postprandial / @@ -153,7 +157,9 @@ export function MetricTargetSummary({ slug }: MetricTargetSummaryProps) { const panels = GLUCOSE_TARGET_TYPES.map((type) => data.targets.find((entry) => entry.type === type), ) - .filter((entry): entry is TargetItem => entry != null && entry.range != null) + .filter( + (entry): entry is TargetItem => entry != null && entry.range != null, + ) .map((entry) => ({ ...entry, label: t(entry.label), @@ -171,8 +177,9 @@ export function MetricTargetSummary({ slug }: MetricTargetSummaryProps) { if (panels.length === 0) return null; // Glucose fans out to up to four per-context panels. Each context - // maps to its own editable threshold, so every panel carries its own - // inline edit button rather than a single shared one. + // maps to its own editable threshold and registers with the + // `TargetAdjustProvider`; the header gear opens the primary + // (first-registered) context's editor. return (
{panels.map((panel) => ( @@ -209,9 +216,16 @@ interface TargetReferencePanelProps { /** * One compact reference panel: range + status pill + source, a range * bar (plus a diastolic bar for BP), the in-target share + 30-day - * average, the 7-day consistency strip, and the adjust-target link. + * average, the 7-day consistency strip, and the guideline source. * All fields come from the `/api/insights/targets` payload — nothing is * recomputed here. + * + * Editing the range is a header action: the panel registers its + * editable target with the `TargetAdjustProvider` and the header gear + * opens the per-metric `` (seeded with this metric's + * type / unit / range, plus the diastolic range for BP). The sheet + * writes `PUT /api/user/thresholds` and invalidates the + * `insightsTargets()` cache on save, so the panel repaints in place. */ function TargetReferencePanel({ target, @@ -219,15 +233,43 @@ function TargetReferencePanel({ heading, }: TargetReferencePanelProps) { const { t } = useTranslations(); - - // The per-metric editor lives here. The button below opens a - // self-contained ``, seeded with this metric's - // type / unit / range (and the diastolic range for BP). The sheet - // writes `PUT /api/user/thresholds` and invalidates the - // `insightsTargets()` cache on save, so the panel repaints in place. - const [editOpen, setEditOpen] = useState(false); + const adjust = useTargetAdjust(); const { range, unit } = target; + + // Register this metric as the header gear's edit target. The effect + // re-runs when the seeded range / unit / label changes (e.g. a glucose + // unit-preference swap), keeping the sheet seed current, and the + // cleanup unregisters when the panel unmounts so a metric without a + // band never leaves a dead gear behind. + const registerAdjust = adjust?.register; + const adjustType = target.type; + const adjustLabel = heading ?? target.label; + const rangeMin = range?.min ?? null; + const rangeMax = range?.max ?? null; + const diaMin = bpDiastolic?.range?.min ?? null; + const diaMax = bpDiastolic?.range?.max ?? null; + useEffect(() => { + if (!registerAdjust || rangeMin == null || rangeMax == null) return; + return registerAdjust({ + type: adjustType, + label: adjustLabel, + unit, + range: { min: rangeMin, max: rangeMax }, + diastolicRange: + diaMin != null && diaMax != null ? { min: diaMin, max: diaMax } : null, + }); + }, [ + registerAdjust, + adjustType, + adjustLabel, + unit, + rangeMin, + rangeMax, + diaMin, + diaMax, + ]); + if (!range) return null; const isBp = target.type === "BLOOD_PRESSURE"; @@ -362,8 +404,10 @@ function TargetReferencePanel({ /> ) : null} - {/* Footer: source link + inline adjust-target button. */} -
+ {/* Footer: guideline source link. The adjust-target affordance + moved to the header gear (see `TargetAdjustProvider`); this + panel registers itself as that gear's edit target above. */} + - - {/* v1.8.6 — inline target editor, mounted alongside the panel and - portalled by the sheet primitive. The body only instantiates its - TanStack Query hooks once `editOpen` is true (lazy mount inside - the sheet), so closed panels stay cheap. */} -
); } diff --git a/src/components/insights/sub-page-shell.tsx b/src/components/insights/sub-page-shell.tsx index 3c7cb2841..7eefc2203 100644 --- a/src/components/insights/sub-page-shell.tsx +++ b/src/components/insights/sub-page-shell.tsx @@ -8,6 +8,8 @@ import { ListOrdered } from "lucide-react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { CoachLaunchButton } from "@/components/insights/coach-launch-button"; +import { TargetAdjustButton } from "@/components/insights/target-adjust-button"; +import { TargetAdjustProvider } from "@/lib/insights/target-adjust-context"; import { useScrollResetOnRoute } from "@/hooks/use-scroll-reset-on-route"; import { useTranslations } from "@/lib/i18n/context"; import { cn } from "@/lib/utils"; @@ -157,11 +159,17 @@ export function SubPageShell({ }, [focusOnMount]); return ( -
- {/* v1.8.7.1 — back-nav leads the page, above the heading. */} - {backLink} -
- {/* v1.8.6 — the heading row pins the Coach icon top-right while the + // The target-adjust provider bridges the header gear (rendered just + // below) to the per-metric `` opened from the + // `` card in the page body. It owns the sheet + // state + renders the sheet, so the gear and the card don't have to + // share a parent beyond this shell. + +
+ {/* v1.8.7.1 — back-nav leads the page, above the heading. */} + {backLink} +
+ {/* v1.8.6 — the heading row pins the Coach icon top-right while the title / nudge / badge wrap together on the left. Design M2 fix: the Coach icon is a `shrink-0` sibling of the @@ -176,100 +184,107 @@ export function SubPageShell({ with and without a nudge now align. `items-start` keeps the Coach icon at the top of the row when the title wraps to two lines. */} -
-
-

- {title} -

- {/* v1.8.6 — the diversity nudge rides the heading as a +
+
+

+ {title} +

+ {/* v1.8.6 — the diversity nudge rides the heading as a `Lightbulb` glyph (the node self-gates to nothing when the spread is healthy), replacing the old inline block. */} - {diversityNudge} - {badge ? ( - - {badge} - - ) : null} + {diversityNudge} + {badge ? ( + + {badge} + + ) : null} +
+ {/* Header action cluster, pinned top-right at heading height by + the row's `justify-between` so it aligns with the title + regardless of badge / glyph presence or a wrapped title. + + The target-adjust gear sits to the LEFT of the Coach icon. + It self-gates on a registered editable target (a metric with + no numeric band registers nothing, so the gear stays hidden), + and the Coach icon self-gates on the Coach flag + per-user + opt-out — so the cluster collapses cleanly to one, both, or + neither control. */} +
+ + {coachLaunch ? : null} +
- {/* v1.8.6 — Coach launch sits top-right at heading height as an - icon, pinned by the row's `justify-between` so it aligns with - the title regardless of badge / glyph presence or a wrapped - title. The button self-gates on the Coach flag + per-user - opt-out. */} - {coachLaunch ? ( - + {explainerMetric ? ( + // v1.8.4 — surface the metric's static definition inline, + // directly beneath the heading. + // + // v1.8.5 W4a — render the explainer body as the *single* caption + // under the heading. Pre-v1.8.5 both this paragraph and the + // `description` below it stacked, so every metric page opened + // with two near-duplicative muted captions before any data — the + // root of the "airy / static under the heading" feel. The + // explainer body is the definition, so it wins; the description + // only renders when no explainer is set (the mother page and any + // future explainer-less sub-page). + // + // v1.8.6 — this caption is now the only surface for the + // definition; the round `?` popover that used to read the same + // string was removed. +

+ {t(`insights.subPage.explainer.${explainerMetric}Body`)} +

) : null} -
- {explainerMetric ? ( - // v1.8.4 — surface the metric's static definition inline, - // directly beneath the heading. - // - // v1.8.5 W4a — render the explainer body as the *single* caption - // under the heading. Pre-v1.8.5 both this paragraph and the - // `description` below it stacked, so every metric page opened - // with two near-duplicative muted captions before any data — the - // root of the "airy / static under the heading" feel. The - // explainer body is the definition, so it wins; the description - // only renders when no explainer is set (the mother page and any - // future explainer-less sub-page). - // - // v1.8.6 — this caption is now the only surface for the - // definition; the round `?` popover that used to read the same - // string was removed. -

- {t(`insights.subPage.explainer.${explainerMetric}Body`)} -

- ) : null} - {description && !explainerMetric ? ( -

{description}

- ) : null} -
- {/* v1.8.5 W4b — numbers-first stat strip sits between the header + {description && !explainerMetric ? ( +

{description}

+ ) : null} +
+ {/* v1.8.5 W4b — numbers-first stat strip sits between the header and the chart so the page leads with data, mirroring the Apple Health / Withings detail layout. Self-gating (renders nothing without data), so the spacing rhythm holds on the empty-state and brand-new-metric paths. v1.8.6 — the diversity nudge moved up to the heading row. */} - {statStrip} - {children} - {/* v1.8.5 W4b — "show all readings" entry at the foot, linking to + {statStrip} + {children} + {/* v1.8.5 W4b — "show all readings" entry at the foot, linking to the dedicated per-metric values subpage. v1.8.6 — normalised to the `h-10` secondary-button height so it reads as a consistent control now that the foot-of-page Coach button (which set the old visual baseline) has moved to the header. */} - {showAllValuesType ? ( - - ) : null} -
+ +
+ ); } diff --git a/src/components/insights/target-adjust-button.tsx b/src/components/insights/target-adjust-button.tsx new file mode 100644 index 000000000..b0a80d3ad --- /dev/null +++ b/src/components/insights/target-adjust-button.tsx @@ -0,0 +1,61 @@ +"use client"; + +import { Settings2 } from "lucide-react"; + +import { Button } from "@/components/ui/button"; +import { cn } from "@/lib/utils"; +import { useTranslations } from "@/lib/i18n/context"; +import { useTargetAdjust } from "@/lib/insights/target-adjust-context"; + +/** + * Header gear that opens the per-metric target editor. + * + * The "Adjust target range" affordance used to sit inside the target + * reference card; it now lives in the Insights category-page header + * (``), to the left of the Coach launch icon, so the card + * stays a read surface. The button is a thin wrapper around + * `useTargetAdjust()`: + * + * - renders nothing when no provider is mounted or no editable target + * has registered for the page (e.g. a metric without a numeric + * target band, where the card paints nothing), so the header never + * shows a dead control; + * - opens the primary registered target's `` on click. + * + * Sized to match `` so the gear + the + * Coach sparkle read as a consistent icon cluster top-right of the + * heading. + */ +export function TargetAdjustButton({ className }: { className?: string }) { + const { t } = useTranslations(); + const adjust = useTargetAdjust(); + + // No provider, or no editable target registered for this page. + if (!adjust || !adjust.canAdjust) return null; + + // Reuse the action's existing copy ("Adjust target range" / + // "Zielbereich anpassen") — the gear performs exactly what the old + // in-card link did, so the same string is the right accessible label. + const label = t("insights.subPage.target.adjustLink"); + + return ( + + ); +} diff --git a/src/lib/insights/__tests__/target-adjust-context.test.tsx b/src/lib/insights/__tests__/target-adjust-context.test.tsx new file mode 100644 index 000000000..e92469705 --- /dev/null +++ b/src/lib/insights/__tests__/target-adjust-context.test.tsx @@ -0,0 +1,71 @@ +import { describe, it, expect } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { + TargetAdjustProvider, + useTargetAdjust, +} from "../target-adjust-context"; + +/** + * Target-adjust context smoke contract. + * + * The project's test convention is `renderToStaticMarkup` (no + * `@testing-library/react` dependency), so we assert the SSR-observable + * surface — interaction (register / requestAdjust opening the sheet) + * runs in effects + state and is covered by the component-level + * behaviour. Here we pin: + * 1. The hook returns `null` outside the provider, so the header gear + * degrades to rendering nothing rather than crashing. + * 2. The hook exposes the full shape beneath the provider, with + * `canAdjust` starting false (no target registered yet). + * 3. The provider renders its children and mounts no editor sheet at + * rest (nothing is open). + */ + +function Probe({ output }: { output: string[] }) { + const adjust = useTargetAdjust(); + if (!adjust) { + output.push("null"); + return null; + } + output.push( + JSON.stringify({ + hasRegister: typeof adjust.register, + hasRequestAdjust: typeof adjust.requestAdjust, + canAdjust: adjust.canAdjust, + }), + ); + return {output.join(",")}; +} + +describe("TargetAdjustProvider", () => { + it("returns null when consumed outside the provider", () => { + const output: string[] = []; + renderToStaticMarkup(); + expect(output).toEqual(["null"]); + }); + + it("exposes the full context shape with canAdjust false at rest", () => { + const output: string[] = []; + renderToStaticMarkup( + + + , + ); + expect(output).toHaveLength(1); + const parsed = JSON.parse(output[0]); + expect(parsed.hasRegister).toBe("function"); + expect(parsed.hasRequestAdjust).toBe("function"); + expect(parsed.canAdjust).toBe(false); + }); + + it("renders children and mounts no editor sheet at rest", () => { + const html = renderToStaticMarkup( + +
child
+ , + ); + expect(html).toContain('data-slot="child-mount"'); + expect(html).not.toContain('data-slot="target-edit-sheet"'); + }); +}); diff --git a/src/lib/insights/target-adjust-context.tsx b/src/lib/insights/target-adjust-context.tsx new file mode 100644 index 000000000..0768d7f10 --- /dev/null +++ b/src/lib/insights/target-adjust-context.tsx @@ -0,0 +1,142 @@ +"use client"; + +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useRef, + useState, + type ReactNode, +} from "react"; + +import { TargetEditSheet } from "@/components/targets/target-edit-sheet"; + +/** + * Insights target-adjust context. + * + * The per-metric target editor (``) used to be opened + * by an "Adjust target range" link sitting inside the target reference + * card on each Insights category page. The trigger moved up to a gear + * button in the page header (``), beside the Coach launch + * icon, so the card stays a read surface and the action reads as a + * header control. + * + * Because the gear lives in the header while the editable target's + * type / range still come from the card in the page body, this context + * bridges the two: + * + * - `` is mounted by ``, owns the + * sheet's open state + which registered target is being edited, and + * renders the `` once a target is open. + * - `` registers each editable target it paints + * (one per category page, except blood glucose which fans out to up + * to four per-context targets) via `register()`. + * - The header gear reads `canAdjust` to gate its own visibility and + * calls `requestAdjust()` to open the editor. + * + * Single-target pages open that target's sheet directly. Blood glucose + * registers several contexts; there is no standalone targets-management + * surface (target editing lives only on these category pages), so the + * gear opens the first registered context — every context's threshold + * is reachable from its own card, and the header gear is the quick path + * into the primary one. + */ + +export interface AdjustableTarget { + /** Target type literal, e.g. `"WEIGHT"` or `"BLOOD_GLUCOSE_FASTING"`. */ + type: string; + /** Display label for the sheet heading. */ + label: string; + /** Display unit, already converted (e.g. mmol/L for a glucose preference). */ + unit: string; + /** Current range seed for the editor, in the display unit. */ + range: { min: number; max: number }; + /** Diastolic range seed for blood pressure; null otherwise. */ + diastolicRange?: { min: number; max: number } | null; +} + +interface TargetAdjustValue { + /** Whether at least one editable target is registered on this page. */ + canAdjust: boolean; + /** Register (or update) an editable target. Returns an unregister fn. */ + register: (target: AdjustableTarget) => () => void; + /** Open the editor for the primary (first-registered) target. */ + requestAdjust: () => void; +} + +const TargetAdjustContext = createContext(null); + +export function TargetAdjustProvider({ children }: { children: ReactNode }) { + // Insertion-ordered registry: a Map keyed by target type so the same + // card can update its seed on a repaint without duplicating, and the + // first-registered entry stays the primary one the gear opens. + const [targets, setTargets] = useState>([]); + // The type currently open in the sheet (null when closed). + const [openType, setOpenType] = useState(null); + + // Keep a ref of the latest registry so `requestAdjust` reads the + // current primary without being recreated on every registration. + // Synced in an effect (never during render) so the callback identity + // stays stable across registrations. + const targetsRef = useRef(targets); + useEffect(() => { + targetsRef.current = targets; + }, [targets]); + + const register = useCallback((target: AdjustableTarget) => { + setTargets((prev) => { + const next = prev.filter((entry) => entry.type !== target.type); + next.push(target); + return next; + }); + return () => { + setTargets((prev) => prev.filter((entry) => entry.type !== target.type)); + }; + }, []); + + const requestAdjust = useCallback(() => { + const primary = targetsRef.current[0]; + if (primary) setOpenType(primary.type); + }, []); + + const value = useMemo( + () => ({ canAdjust: targets.length > 0, register, requestAdjust }), + [targets.length, register, requestAdjust], + ); + + const openTarget = targets.find((entry) => entry.type === openType) ?? null; + + return ( + + {children} + {/* The sheet body lazy-mounts on `open`, so a closed page pays + nothing. Keyed by type so switching the open target remounts a + clean editor seeded with the right range. */} + {openTarget ? ( + { + if (!next) setOpenType(null); + }} + /> + ) : null} + + ); +} + +/** + * Returns the target-adjust context, or `null` when no provider is + * mounted (so dropping a consumer outside an Insights sub-page is a + * no-op rather than a crash, mirroring `useCoachLaunch()`). + */ +export function useTargetAdjust(): TargetAdjustValue | null { + return useContext(TargetAdjustContext); +}