mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 15:02:04 +00:00
fix(ui): trim document-read copy and de-duplicate recovery headings
Remove the two per-document AI copy blocks in the document detail sheet: the external-egress notice and the "read this document" explainer. The settings-toggle honesty confirm already states the egress trade, so the per-document repeat is redundant. Drop the now-unused `egressExternal` prop and the `documents.ai.egressNotice` + `documents.ai.available` keys across all six locales. Keep the capability `egress` DTO field — the API still reports it. On /insights/recovery, the metric block rendered an outer heading plus an explainer above a tile whose own header repeated the same title. Move the explainer inside the tile as a `description` beneath its `TileHeader` and promote the tile title to a real `<h2>` (new `titleAs` affordance), so each metric shows one heading with the blurb under it and the h1->h2 outline stays nested. Applies to every metric the block renders.
This commit is contained in:
1 parent
1e2b1d2c6a
commit
97f8cb4303
13 files changed
+88
-85
No files matched your search
@@ -34,7 +34,6 @@ function base(
|
||||
aiEnabled: true,
|
||||
indexEnabled: true,
|
||||
unavailableReason: null,
|
||||
egressExternal: false,
|
||||
actionsDisabled: false,
|
||||
onSuggest: noop,
|
||||
suggestPending: false,
|
||||
@@ -143,20 +142,14 @@ describe("<DocumentAiSection>", () => {
|
||||
expect(html).toContain("Not saved");
|
||||
});
|
||||
|
||||
it("shows the vendor-blind egress notice before an external document read", () => {
|
||||
const html = render(base({ aiEnabled: true, egressExternal: true }));
|
||||
expect(html).toContain('data-slot="document-ai-egress-notice"');
|
||||
expect(html).toContain("third-party AI service");
|
||||
// Vendor-blind: never names a specific AI vendor in the document surface.
|
||||
expect(html).not.toContain("OpenAI");
|
||||
expect(html).not.toContain("ChatGPT");
|
||||
// The actions are still offered — the notice informs, it does not block.
|
||||
expect(html).toContain('data-slot="assist-suggest"');
|
||||
});
|
||||
|
||||
it("omits the egress notice when the read stays on the local machine", () => {
|
||||
const html = render(base({ aiEnabled: true, egressExternal: false }));
|
||||
it("renders no per-document egress notice — the settings confirm covers it", () => {
|
||||
// The per-document egress notice was retired; the settings-toggle honesty
|
||||
// confirm is now the single place the egress trade is stated.
|
||||
const html = render(base({ aiEnabled: true }));
|
||||
expect(html).not.toContain('data-slot="document-ai-egress-notice"');
|
||||
// The actions are still offered.
|
||||
expect(html).toContain('data-slot="assist-suggest"');
|
||||
expect(html).toContain('data-slot="document-read-ai"');
|
||||
});
|
||||
|
||||
it("hides the Read-with-AI action when indexing is unavailable but keeps suggest", () => {
|
||||
|
||||
@@ -22,7 +22,7 @@
|
||||
* status pill stays, honestly reflecting any local auto-index, and the manual
|
||||
* form below stays fully usable.
|
||||
*/
|
||||
import { FileText, ScanText, ShieldAlert, WandSparkles } from "lucide-react";
|
||||
import { FileText, ScanText, WandSparkles } from "lucide-react";
|
||||
import type { ReactNode } from "react";
|
||||
|
||||
import { Button } from "@/components/ui/button";
|
||||
@@ -47,7 +47,6 @@ export function DocumentAiSection({
|
||||
aiEnabled,
|
||||
indexEnabled,
|
||||
unavailableReason,
|
||||
egressExternal,
|
||||
actionsDisabled,
|
||||
onSuggest,
|
||||
suggestPending,
|
||||
@@ -75,12 +74,6 @@ export function DocumentAiSection({
|
||||
aiEnabled: boolean;
|
||||
indexEnabled: boolean;
|
||||
unavailableReason: "no-provider" | "enable-local-ocr" | null;
|
||||
/**
|
||||
* True when a document read will egress to a third-party AI service (any
|
||||
* provider other than a self-hosted local model). Drives the vendor-blind
|
||||
* "this leaves your machine" notice shown before the AI actions.
|
||||
*/
|
||||
egressExternal: boolean;
|
||||
/** Capability still resolving — the transport mode isn't known yet. */
|
||||
actionsDisabled: boolean;
|
||||
onSuggest: () => void;
|
||||
@@ -143,23 +136,8 @@ export function DocumentAiSection({
|
||||
/>
|
||||
</div>
|
||||
|
||||
{egressExternal ? (
|
||||
<div
|
||||
data-slot="document-ai-egress-notice"
|
||||
role="note"
|
||||
className="border-border text-muted-foreground flex items-start gap-2 rounded-lg border border-dashed px-3 py-2.5 text-xs"
|
||||
>
|
||||
<ShieldAlert className="mt-0.5 size-3.5 shrink-0" aria-hidden />
|
||||
<p className="min-w-0">{t("documents.ai.egressNotice")}</p>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{aiEnabled ? (
|
||||
<>
|
||||
<p className="text-muted-foreground text-xs">
|
||||
{t("documents.ai.available")}
|
||||
</p>
|
||||
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
{indexEnabled ? (
|
||||
<Button
|
||||
|
||||
@@ -506,7 +506,6 @@ export function DocumentDetailSheet({
|
||||
aiEnabled={aiEnabled}
|
||||
indexEnabled={indexEnabled}
|
||||
unavailableReason={capability.data?.reason ?? null}
|
||||
egressExternal={capability.data?.egress === "external"}
|
||||
actionsDisabled={capability.isPending}
|
||||
onSuggest={runSuggest}
|
||||
suggestPending={suggest.isPending}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import type { ComponentType } from "react";
|
||||
import type { ComponentType, ReactNode } from "react";
|
||||
import { Sigma } from "lucide-react";
|
||||
|
||||
import { useFormatters, useTranslations } from "@/lib/i18n/context";
|
||||
@@ -102,6 +102,21 @@ export interface MetricStatSeries {
|
||||
* all-time central value. Omit it to keep the generic label.
|
||||
*/
|
||||
medianLabel?: string;
|
||||
/**
|
||||
* v1.28.4 — optional descriptive line rendered INSIDE the tile, directly
|
||||
* under the `<TileHeader>` and above the four-stat grid. Lets a metric block
|
||||
* own its heading + blurb in a single tile rather than duplicating the
|
||||
* heading in an outer section above it. Muted supporting copy; omit it for
|
||||
* the numbers-only header the other subpages lead with.
|
||||
*/
|
||||
description?: ReactNode;
|
||||
/**
|
||||
* Render the series' `<TileHeader>` title as a real heading element (e.g.
|
||||
* "h2") rather than the presentational `CardTitle` div. Used where the tile
|
||||
* carries the ONLY heading on its subpage branch (the recovery blocks) so the
|
||||
* `h1 → h2` outline under `SubPageShell` stays nested with no skipped level.
|
||||
*/
|
||||
titleAs?: "h2";
|
||||
}
|
||||
|
||||
type MetricStatStripProps = Partial<MetricStatSeries> & {
|
||||
@@ -140,6 +155,8 @@ function SeriesBlock({
|
||||
windowStats,
|
||||
dataKey,
|
||||
medianLabel,
|
||||
description,
|
||||
titleAs,
|
||||
}: MetricStatSeries) {
|
||||
const { t } = useTranslations();
|
||||
const fmt = useFormatters();
|
||||
@@ -183,7 +200,16 @@ function SeriesBlock({
|
||||
className="space-y-2"
|
||||
>
|
||||
{seriesLabel ? (
|
||||
<TileHeader icon={icon ?? Sigma} title={seriesLabel} />
|
||||
<TileHeader
|
||||
icon={icon ?? Sigma}
|
||||
title={seriesLabel}
|
||||
titleAs={titleAs}
|
||||
/>
|
||||
) : null}
|
||||
{description ? (
|
||||
<p className="text-muted-foreground text-sm leading-relaxed">
|
||||
{description}
|
||||
</p>
|
||||
) : null}
|
||||
<div className="grid grid-cols-2 gap-x-3 gap-y-2 sm:grid-cols-4 [&>div]:min-h-[44px]">
|
||||
{cells.map((cell) => (
|
||||
@@ -215,6 +241,8 @@ export function MetricStatStrip({
|
||||
windowStats,
|
||||
dataKey,
|
||||
medianLabel,
|
||||
description,
|
||||
titleAs,
|
||||
series,
|
||||
groupLabel,
|
||||
}: MetricStatStripProps) {
|
||||
@@ -277,6 +305,8 @@ export function MetricStatStrip({
|
||||
windowStats={windowStats}
|
||||
dataKey={dataKey}
|
||||
medianLabel={medianLabel}
|
||||
description={description}
|
||||
titleAs={titleAs}
|
||||
/>
|
||||
</CardContent>
|
||||
</Card>
|
||||
|
||||
@@ -71,28 +71,20 @@ export function RecoveryMetricBlock({
|
||||
|
||||
return (
|
||||
<section data-slot={`recovery-block-${type}`} className="space-y-3">
|
||||
<div className="space-y-1">
|
||||
<div className="flex items-center gap-2">
|
||||
<Icon
|
||||
className="text-muted-foreground size-4 shrink-0"
|
||||
aria-hidden="true"
|
||||
/>
|
||||
{/* v1.18.6 — block title is an `<h2>` (at `text-base`): the per-group
|
||||
sub-headings were removed, so each block now sits directly under
|
||||
the page `<h1>` from `SubPageShell`. Promoting it from `<h3>` keeps
|
||||
the recovery page's heading outline strictly nested (h1 → h2)
|
||||
with no skipped level. */}
|
||||
<h2 className="text-base font-semibold">{title}</h2>
|
||||
</div>
|
||||
<p className="text-foreground text-sm leading-relaxed">{explainer}</p>
|
||||
</div>
|
||||
|
||||
{/* v1.28.4 — the block no longer wraps the tile in an outer `<h2>` +
|
||||
explainer that the stat-strip's own `TileHeader` then repeated. The
|
||||
tile owns the single heading: `seriesLabel` becomes the block's `<h2>`
|
||||
(via `titleAs`, preserving the h1 → h2 nesting under `SubPageShell`)
|
||||
and the explainer renders inside the tile beneath it as `description`.
|
||||
The chart tile keeps its own canonical `TileHeader` naming itself. */}
|
||||
<MetricStatStrip
|
||||
summary={summary}
|
||||
unit={unit}
|
||||
fractionDigits={fractionDigits}
|
||||
seriesLabel={title}
|
||||
icon={Icon}
|
||||
titleAs="h2"
|
||||
description={explainer}
|
||||
windowStats={statsByType?.[type] ?? null}
|
||||
/>
|
||||
|
||||
|
||||
@@ -51,6 +51,14 @@ interface TileHeaderProps {
|
||||
titleClassName?: string;
|
||||
/** Optional id on the `CardTitle`, e.g. for an `aria-labelledby` link. */
|
||||
id?: string;
|
||||
/**
|
||||
* Render the title as a real heading element (e.g. `"h2"`) instead of the
|
||||
* default presentational `CardTitle` div, keeping the identical CardTitle
|
||||
* styling. Use only where the tile owns the ONLY heading on its subpage
|
||||
* branch (the recovery blocks) so the `h1 → h2` outline stays nested with no
|
||||
* skipped level. Default (unset) keeps every other tile a plain CardTitle.
|
||||
*/
|
||||
titleAs?: "h2";
|
||||
}
|
||||
|
||||
export function TileHeader({
|
||||
@@ -61,7 +69,12 @@ export function TileHeader({
|
||||
className,
|
||||
titleClassName,
|
||||
id,
|
||||
titleAs,
|
||||
}: TileHeaderProps) {
|
||||
const titleClassNames = cn(
|
||||
size === "sm" ? "text-sm" : "text-base",
|
||||
titleClassName,
|
||||
);
|
||||
return (
|
||||
<div
|
||||
data-slot="tile-header"
|
||||
@@ -78,12 +91,21 @@ export function TileHeader({
|
||||
aria-hidden="true"
|
||||
/>
|
||||
) : null}
|
||||
<CardTitle
|
||||
id={id}
|
||||
className={cn(size === "sm" ? "text-sm" : "text-base", titleClassName)}
|
||||
>
|
||||
{title}
|
||||
</CardTitle>
|
||||
{titleAs === "h2" ? (
|
||||
// Heading semantics with the exact CardTitle visual treatment so the
|
||||
// tile can carry a real `<h2>` where it owns the subpage's heading.
|
||||
<h2
|
||||
id={id}
|
||||
data-slot="card-title"
|
||||
className={cn("leading-none font-semibold", titleClassNames)}
|
||||
>
|
||||
{title}
|
||||
</h2>
|
||||
) : (
|
||||
<CardTitle id={id} className={titleClassNames}>
|
||||
{title}
|
||||
</CardTitle>
|
||||
)}
|
||||
{right ? <div className="ml-auto flex items-center">{right}</div> : null}
|
||||
</div>
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user