mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 21:32:18 +00:00
style(charts): grey out comparison-overlay toggle when no prior-period data exists
Marc directive 2026-05-14 — when a chart has a comparisonBaseline
selected but the prior period (lastMonth / lastYear) carries zero
overlay-able rows, the chart settings cog now shows the comparison
buttons in a visibly disabled state (opacity-50, aria-disabled, native
title tooltip, and a sub-label hint) instead of silently rendering
nothing. The 'none' button always stays enabled — it's the escape
hatch.
Plumbing:
- chart-overlay-controls.tsx: new `hasComparisonData` prop (defaults
to true so legacy callers keep their behaviour). The popover body is
extracted as `<ChartOverlayControlsBody>` so unit tests can render
it without Radix's portalled popover stripping it out of the SSR
output.
- health-chart.tsx + mood-chart.tsx: thread the already-computed
`hasComparisonData` memo (line 768 / 457) into the controls.
- medication-compliance-chart.tsx: passes `hasComparisonData={false}`
unconditionally because the compliance heatmap never paints a
prior-period overlay (already documented in the existing N/A
caption block).
Tests: 5 new cases under chart-overlay-controls.test.tsx cover the
normal / greyed / none-baseline / locale / default-prop branches.
The pre-W3f assertion suite (3 tests) keeps passing unchanged.
i18n: appended chart.overlay.controls.comparisonUnavailable to both
locales — no key restructure.
This commit is contained in:
1 parent
6148a179ca
commit
7b6b9161c3
7 files changed
+300
-83
No files matched your search
@@ -731,6 +731,7 @@
|
||||
"trendArrow": "Trend-Pfeil",
|
||||
"targetRange": "Zielbereich",
|
||||
"comparisonBaseline": "Vergleich",
|
||||
"comparisonUnavailable": "Kein Vorzeitraum verfügbar",
|
||||
"tooltip": {
|
||||
"openSettings": "Chart-Overlay-Einstellungen"
|
||||
}
|
||||
|
||||
@@ -731,6 +731,7 @@
|
||||
"trendArrow": "Trend arrow",
|
||||
"targetRange": "Target range",
|
||||
"comparisonBaseline": "Comparison",
|
||||
"comparisonUnavailable": "No prior-period data yet",
|
||||
"tooltip": {
|
||||
"openSettings": "Chart overlay settings"
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import { I18nProvider } from "@/lib/i18n/context";
|
||||
|
||||
import {
|
||||
ChartOverlayControls,
|
||||
ChartOverlayControlsBody,
|
||||
DEFAULT_CHART_OVERLAY_PREFS,
|
||||
type ChartOverlayPrefs,
|
||||
} from "../chart-overlay-controls";
|
||||
@@ -95,4 +96,135 @@ describe("<ChartOverlayControls>", () => {
|
||||
|
||||
expect(html).toContain('aria-label="Chart-Overlay-Einstellungen"');
|
||||
});
|
||||
|
||||
/**
|
||||
* v1.4.25 W3f — comparison-baseline grey-out when the parent chart
|
||||
* reports `hasComparisonData=false`. The disabled state communicates
|
||||
* "you selected a comparison but there's no prior-period series to
|
||||
* paint" instead of silently hiding the overlay.
|
||||
*
|
||||
* The popover body is portalled by Radix at runtime — it's not in
|
||||
* SSR markup. The tests target the extracted `<ChartOverlayControlsBody>`
|
||||
* so the JSX inside the popover is renderable in `renderToStaticMarkup`.
|
||||
*/
|
||||
describe("comparison-baseline grey-out (v1.4.25 W3f)", () => {
|
||||
it("renders comparison buttons in normal state when prior data exists", () => {
|
||||
const prefs: ChartOverlayPrefs = {
|
||||
...DEFAULT_CHART_OVERLAY_PREFS,
|
||||
comparisonBaseline: "lastMonth",
|
||||
};
|
||||
const html = renderToStaticMarkup(
|
||||
withProvider(
|
||||
<ChartOverlayControlsBody
|
||||
prefs={prefs}
|
||||
onChange={vi.fn()}
|
||||
hasComparisonData
|
||||
/>,
|
||||
),
|
||||
);
|
||||
|
||||
expect(html).not.toContain('data-comparison-disabled="true"');
|
||||
expect(html).not.toContain('data-comparison-greyed="true"');
|
||||
expect(html).not.toContain(
|
||||
'data-slot="chart-overlay-comparison-unavailable-hint"',
|
||||
);
|
||||
// The lastMonth button has no aria-disabled set when data is
|
||||
// available.
|
||||
expect(html).not.toMatch(
|
||||
/data-slot="chart-overlay-comparison-lastMonth"[^>]*aria-disabled="true"/,
|
||||
);
|
||||
});
|
||||
|
||||
it("greys out lastMonth + lastYear when prior period is empty AND a baseline is selected", () => {
|
||||
const prefs: ChartOverlayPrefs = {
|
||||
...DEFAULT_CHART_OVERLAY_PREFS,
|
||||
comparisonBaseline: "lastMonth",
|
||||
};
|
||||
const html = renderToStaticMarkup(
|
||||
withProvider(
|
||||
<ChartOverlayControlsBody
|
||||
prefs={prefs}
|
||||
onChange={vi.fn()}
|
||||
hasComparisonData={false}
|
||||
/>,
|
||||
),
|
||||
);
|
||||
|
||||
expect(html).toContain('data-comparison-disabled="true"');
|
||||
// The lastMonth + lastYear buttons grey out, "none" does not.
|
||||
expect(html).toMatch(
|
||||
/data-slot="chart-overlay-comparison-lastMonth"[^>]*data-comparison-greyed="true"/,
|
||||
);
|
||||
expect(html).toMatch(
|
||||
/data-slot="chart-overlay-comparison-lastYear"[^>]*data-comparison-greyed="true"/,
|
||||
);
|
||||
// The "none" button stays enabled — it's the escape hatch.
|
||||
expect(html).not.toMatch(
|
||||
/data-slot="chart-overlay-comparison-none"[^>]*data-comparison-greyed="true"/,
|
||||
);
|
||||
// aria-disabled + opacity-50 + tooltip surface.
|
||||
expect(html).toMatch(
|
||||
/data-slot="chart-overlay-comparison-lastMonth"[^>]*aria-disabled="true"/,
|
||||
);
|
||||
expect(html).toContain("opacity-50");
|
||||
expect(html).toContain("No prior-period data yet");
|
||||
// The footer hint paragraph is also rendered so screen readers
|
||||
// hear the explanation.
|
||||
expect(html).toContain(
|
||||
'data-slot="chart-overlay-comparison-unavailable-hint"',
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps all buttons enabled when the baseline is 'none' even with no prior data", () => {
|
||||
// The "none" baseline is the safe default — the grey-out only
|
||||
// triggers once a user has opted in to comparing.
|
||||
const html = renderToStaticMarkup(
|
||||
withProvider(
|
||||
<ChartOverlayControlsBody
|
||||
prefs={DEFAULT_CHART_OVERLAY_PREFS}
|
||||
onChange={vi.fn()}
|
||||
hasComparisonData={false}
|
||||
/>,
|
||||
),
|
||||
);
|
||||
|
||||
expect(html).not.toContain('data-comparison-disabled="true"');
|
||||
expect(html).not.toContain('data-comparison-greyed="true"');
|
||||
expect(html).not.toContain(
|
||||
'data-slot="chart-overlay-comparison-unavailable-hint"',
|
||||
);
|
||||
});
|
||||
|
||||
it("emits the localised tooltip + hint copy under de locale", () => {
|
||||
const prefs: ChartOverlayPrefs = {
|
||||
...DEFAULT_CHART_OVERLAY_PREFS,
|
||||
comparisonBaseline: "lastYear",
|
||||
};
|
||||
const html = renderToStaticMarkup(
|
||||
<I18nProvider initialLocale="de">
|
||||
<ChartOverlayControlsBody
|
||||
prefs={prefs}
|
||||
onChange={vi.fn()}
|
||||
hasComparisonData={false}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
expect(html).toContain("Kein Vorzeitraum verfügbar");
|
||||
});
|
||||
|
||||
it("defaults hasComparisonData to true when the prop is omitted", () => {
|
||||
// Charts that haven't been updated to thread the flag keep the
|
||||
// pre-W3f behaviour — every button stays enabled.
|
||||
const prefs: ChartOverlayPrefs = {
|
||||
...DEFAULT_CHART_OVERLAY_PREFS,
|
||||
comparisonBaseline: "lastMonth",
|
||||
};
|
||||
const html = renderToStaticMarkup(
|
||||
withProvider(
|
||||
<ChartOverlayControlsBody prefs={prefs} onChange={vi.fn()} />,
|
||||
),
|
||||
);
|
||||
expect(html).not.toContain('data-comparison-disabled="true"');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -70,28 +70,27 @@ export interface ChartOverlayControlsProps {
|
||||
* wrapper align the cog with its title row.
|
||||
*/
|
||||
triggerClassName?: string;
|
||||
/**
|
||||
* v1.4.25 W3f — when the parent chart already knows the prior period
|
||||
* carries no overlay-able rows, it threads `hasComparisonData=false`
|
||||
* here so the comparison-baseline buttons render in a visibly
|
||||
* disabled state (reduced opacity + `aria-disabled` + tooltip)
|
||||
* instead of letting the user pick a value that paints nothing on
|
||||
* the chart.
|
||||
*
|
||||
* Default `true` (i.e. enabled) so charts that never compute the
|
||||
* flag (or compute it asynchronously) keep their existing behaviour.
|
||||
*/
|
||||
hasComparisonData?: boolean;
|
||||
}
|
||||
|
||||
export function ChartOverlayControls({
|
||||
prefs,
|
||||
onChange,
|
||||
triggerClassName,
|
||||
hasComparisonData = true,
|
||||
}: ChartOverlayControlsProps): ReactElement {
|
||||
const { t } = useTranslations();
|
||||
const trendIndicatorId = useId();
|
||||
const trendArrowId = useId();
|
||||
const targetRangeId = useId();
|
||||
|
||||
const setToggle = (
|
||||
key: "showTrendIndicator" | "showTrendArrow" | "showTargetRange",
|
||||
value: boolean,
|
||||
): void => {
|
||||
onChange({ ...prefs, [key]: value });
|
||||
};
|
||||
|
||||
const setComparisonBaseline = (value: ComparisonBaseline): void => {
|
||||
onChange({ ...prefs, comparisonBaseline: value });
|
||||
};
|
||||
|
||||
return (
|
||||
<DropdownMenu>
|
||||
@@ -113,70 +112,135 @@ export function ChartOverlayControls({
|
||||
className="w-[240px] p-3"
|
||||
data-slot="chart-overlay-controls-content"
|
||||
>
|
||||
<DropdownMenuLabel className="text-xs font-medium tracking-wide uppercase">
|
||||
{t("chart.overlay.controls.title")}
|
||||
</DropdownMenuLabel>
|
||||
<DropdownMenuSeparator />
|
||||
<div className="flex flex-col gap-2.5 pt-1">
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<Label
|
||||
htmlFor={trendIndicatorId}
|
||||
className="cursor-pointer text-xs font-normal"
|
||||
>
|
||||
{t("chart.overlay.controls.trendIndicator")}
|
||||
</Label>
|
||||
<Switch
|
||||
id={trendIndicatorId}
|
||||
checked={prefs.showTrendIndicator}
|
||||
onCheckedChange={(value) =>
|
||||
setToggle("showTrendIndicator", Boolean(value))
|
||||
}
|
||||
data-slot="chart-overlay-toggle-trend-indicator"
|
||||
/>
|
||||
</div>
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<Label
|
||||
htmlFor={trendArrowId}
|
||||
className="cursor-pointer text-xs font-normal"
|
||||
>
|
||||
{t("chart.overlay.controls.trendArrow")}
|
||||
</Label>
|
||||
<Switch
|
||||
id={trendArrowId}
|
||||
checked={prefs.showTrendArrow}
|
||||
onCheckedChange={(value) =>
|
||||
setToggle("showTrendArrow", Boolean(value))
|
||||
}
|
||||
data-slot="chart-overlay-toggle-trend-arrow"
|
||||
/>
|
||||
</div>
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<Label
|
||||
htmlFor={targetRangeId}
|
||||
className="cursor-pointer text-xs font-normal"
|
||||
>
|
||||
{t("chart.overlay.controls.targetRange")}
|
||||
</Label>
|
||||
<Switch
|
||||
id={targetRangeId}
|
||||
checked={prefs.showTargetRange}
|
||||
onCheckedChange={(value) =>
|
||||
setToggle("showTargetRange", Boolean(value))
|
||||
}
|
||||
data-slot="chart-overlay-toggle-target-range"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
<DropdownMenuSeparator />
|
||||
<div className="flex flex-col gap-1.5 pt-1">
|
||||
<Label className="text-muted-foreground text-xs font-medium">
|
||||
{t("chart.overlay.controls.comparisonBaseline")}
|
||||
</Label>
|
||||
<div
|
||||
className="grid grid-cols-3 gap-1"
|
||||
data-slot="chart-overlay-comparison-baseline"
|
||||
<ChartOverlayControlsBody
|
||||
prefs={prefs}
|
||||
onChange={onChange}
|
||||
hasComparisonData={hasComparisonData}
|
||||
/>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.4.25 W3f — the dropdown body is extracted so unit tests can render
|
||||
* it without spinning up Radix's portalled popover (which is invisible
|
||||
* to `renderToStaticMarkup`). Production callers always go through
|
||||
* `<ChartOverlayControls>` above; the body export is purely a test
|
||||
* affordance + a future hook for in-line (non-popover) renders.
|
||||
*/
|
||||
export interface ChartOverlayControlsBodyProps {
|
||||
prefs: ChartOverlayPrefs;
|
||||
onChange: (next: ChartOverlayPrefs) => void;
|
||||
hasComparisonData?: boolean;
|
||||
}
|
||||
|
||||
export function ChartOverlayControlsBody({
|
||||
prefs,
|
||||
onChange,
|
||||
hasComparisonData = true,
|
||||
}: ChartOverlayControlsBodyProps): ReactElement {
|
||||
const { t } = useTranslations();
|
||||
const trendIndicatorId = useId();
|
||||
const trendArrowId = useId();
|
||||
const targetRangeId = useId();
|
||||
|
||||
// v1.4.25 W3f — when the underlying prior-period series is empty for
|
||||
// BOTH lastMonth and lastYear (we only know about the currently
|
||||
// selected baseline), grey out the non-"none" buttons. We can only
|
||||
// verify the active baseline truthfully, so the disabled state is
|
||||
// gated on `comparisonBaseline !== "none" && !hasComparisonData`:
|
||||
// the user has already opted in to comparing and we discovered the
|
||||
// prior period is empty. From the "none" baseline we keep all
|
||||
// buttons enabled so the user can still toggle in for the chart to
|
||||
// re-evaluate.
|
||||
const comparisonGreyOut =
|
||||
prefs.comparisonBaseline !== "none" && !hasComparisonData;
|
||||
const disabledTooltip = t("chart.overlay.controls.comparisonUnavailable");
|
||||
|
||||
const setToggle = (
|
||||
key: "showTrendIndicator" | "showTrendArrow" | "showTargetRange",
|
||||
value: boolean,
|
||||
): void => {
|
||||
onChange({ ...prefs, [key]: value });
|
||||
};
|
||||
|
||||
const setComparisonBaseline = (value: ComparisonBaseline): void => {
|
||||
onChange({ ...prefs, comparisonBaseline: value });
|
||||
};
|
||||
|
||||
return (
|
||||
<>
|
||||
<DropdownMenuLabel className="text-xs font-medium tracking-wide uppercase">
|
||||
{t("chart.overlay.controls.title")}
|
||||
</DropdownMenuLabel>
|
||||
<DropdownMenuSeparator />
|
||||
<div className="flex flex-col gap-2.5 pt-1">
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<Label
|
||||
htmlFor={trendIndicatorId}
|
||||
className="cursor-pointer text-xs font-normal"
|
||||
>
|
||||
{COMPARISON_BASELINES.map((value) => (
|
||||
{t("chart.overlay.controls.trendIndicator")}
|
||||
</Label>
|
||||
<Switch
|
||||
id={trendIndicatorId}
|
||||
checked={prefs.showTrendIndicator}
|
||||
onCheckedChange={(value) =>
|
||||
setToggle("showTrendIndicator", Boolean(value))
|
||||
}
|
||||
data-slot="chart-overlay-toggle-trend-indicator"
|
||||
/>
|
||||
</div>
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<Label
|
||||
htmlFor={trendArrowId}
|
||||
className="cursor-pointer text-xs font-normal"
|
||||
>
|
||||
{t("chart.overlay.controls.trendArrow")}
|
||||
</Label>
|
||||
<Switch
|
||||
id={trendArrowId}
|
||||
checked={prefs.showTrendArrow}
|
||||
onCheckedChange={(value) =>
|
||||
setToggle("showTrendArrow", Boolean(value))
|
||||
}
|
||||
data-slot="chart-overlay-toggle-trend-arrow"
|
||||
/>
|
||||
</div>
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<Label
|
||||
htmlFor={targetRangeId}
|
||||
className="cursor-pointer text-xs font-normal"
|
||||
>
|
||||
{t("chart.overlay.controls.targetRange")}
|
||||
</Label>
|
||||
<Switch
|
||||
id={targetRangeId}
|
||||
checked={prefs.showTargetRange}
|
||||
onCheckedChange={(value) =>
|
||||
setToggle("showTargetRange", Boolean(value))
|
||||
}
|
||||
data-slot="chart-overlay-toggle-target-range"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
<DropdownMenuSeparator />
|
||||
<div className="flex flex-col gap-1.5 pt-1">
|
||||
<Label className="text-muted-foreground text-xs font-medium">
|
||||
{t("chart.overlay.controls.comparisonBaseline")}
|
||||
</Label>
|
||||
<div
|
||||
className="grid grid-cols-3 gap-1"
|
||||
data-slot="chart-overlay-comparison-baseline"
|
||||
data-comparison-disabled={comparisonGreyOut ? "true" : undefined}
|
||||
>
|
||||
{COMPARISON_BASELINES.map((value) => {
|
||||
// The "none" choice never greys out — it's the safe
|
||||
// escape hatch when prior data is missing.
|
||||
const isNoneOption = value === "none";
|
||||
const isGreyed = comparisonGreyOut && !isNoneOption;
|
||||
return (
|
||||
<Button
|
||||
key={value}
|
||||
type="button"
|
||||
@@ -184,17 +248,28 @@ export function ChartOverlayControls({
|
||||
prefs.comparisonBaseline === value ? "default" : "outline"
|
||||
}
|
||||
size="sm"
|
||||
className="h-9 px-1 text-[11px]"
|
||||
className={`h-9 px-1 text-[11px] ${isGreyed ? "opacity-50" : ""}`.trim()}
|
||||
onClick={() => setComparisonBaseline(value)}
|
||||
aria-pressed={prefs.comparisonBaseline === value}
|
||||
aria-disabled={isGreyed || undefined}
|
||||
title={isGreyed ? disabledTooltip : undefined}
|
||||
data-slot={`chart-overlay-comparison-${value}`}
|
||||
data-comparison-greyed={isGreyed ? "true" : undefined}
|
||||
>
|
||||
{t(`comparison.baseline.${value}`)}
|
||||
</Button>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
{comparisonGreyOut && (
|
||||
<p
|
||||
className="text-muted-foreground text-[10px] leading-snug"
|
||||
data-slot="chart-overlay-comparison-unavailable-hint"
|
||||
>
|
||||
{disabledTooltip}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -1138,6 +1138,7 @@ export function HealthChart({
|
||||
<ChartOverlayControls
|
||||
prefs={overlayPrefs.prefs}
|
||||
onChange={overlayPrefs.setPrefs}
|
||||
hasComparisonData={hasComparisonData}
|
||||
/>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
@@ -337,10 +337,16 @@ export function MedicationComplianceChart({
|
||||
</Button>
|
||||
))}
|
||||
{/* v1.4.18 — overlay-controls dropdown next to the range
|
||||
tabs. */}
|
||||
tabs.
|
||||
v1.4.25 W3f — the compliance card never paints a
|
||||
prior-period overlay (the heatmap doesn't support it),
|
||||
so the comparison buttons grey out when a baseline is
|
||||
already selected to signal "this metric has no
|
||||
comparison available". */}
|
||||
<ChartOverlayControls
|
||||
prefs={overlayPrefs.prefs}
|
||||
onChange={overlayPrefs.setPrefs}
|
||||
hasComparisonData={false}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -604,6 +604,7 @@ export function MoodChart({
|
||||
<ChartOverlayControls
|
||||
prefs={overlayPrefs.prefs}
|
||||
onChange={overlayPrefs.setPrefs}
|
||||
hasComparisonData={hasComparisonData}
|
||||
/>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
Reference in new issue
Block a user