mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 22:50:51 +00:00
fix(insights): cap the ECG teaser, never the ECG page
The five-strip cap belonged on the overview teaser — a glance on /insights — not on the surface a user opens to read their history. It was living in the shared EcgSection, which the dedicated /insights/ecg page reuses, so an account with many recordings could only ever reach the newest five there and the older strips became unreachable with no pagination behind them. The cap is a prop now. The teaser passes ECG_OVERVIEW_LIMIT; the page passes nothing and lists every recording. The constant lives in the React-free classification module so the overview page can pass it without pulling the dynamically-loaded ECG component into its bundle.
This commit is contained in:
1 parent
65a3d91b6e
commit
8f3e4b6387
5 files changed
+87
-22
No files matched your search
@@ -76,6 +76,26 @@ describe("/insights/ecg page (H1)", () => {
|
||||
expect(html).not.toContain("<h2");
|
||||
});
|
||||
|
||||
it("renders every recording — the full page passes no cap", () => {
|
||||
// The teaser on `/insights` caps at five; this dedicated page must not.
|
||||
// An account with many strips reaches all of them here, or the older ones
|
||||
// become unreachable with no pagination behind them. Eight in, eight out.
|
||||
const recordings = Array.from({ length: 8 }, (_, index) => ({
|
||||
...seededRecording(),
|
||||
id: `rec-${index}`,
|
||||
recordedAt: new Date(Date.UTC(2026, 6, 10 - index, 8, 0)).toISOString(),
|
||||
}));
|
||||
const html = render({ recordings, hasRecordings: true });
|
||||
|
||||
const rows = (html.match(/data-slot="ecg-row"/g) ?? []).length;
|
||||
expect(rows).toBe(8);
|
||||
for (const rec of recordings) {
|
||||
expect(html, `${rec.id} must be reachable on the full page`).toContain(
|
||||
`/insights/ecg/${rec.id}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("carries no disclaimer paragraph above the list", () => {
|
||||
// Two muted lines used to lead this page — one from the shell's own
|
||||
// explainer slot and one hand-rolled beneath it, both saying HealthLog
|
||||
|
||||
@@ -9,6 +9,9 @@ import { SlidersHorizontal, TrendingUp } from "lucide-react";
|
||||
import { useAuth } from "@/hooks/use-auth";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { apiGet } from "@/lib/api/api-fetch";
|
||||
// Pure constant — importing it here does NOT pull the dynamically-loaded ECG
|
||||
// component into this bundle, which is exactly why it lives in the lib module.
|
||||
import { ECG_OVERVIEW_LIMIT } from "@/lib/insights/ecg-classification";
|
||||
import { useFeatureFlags } from "@/hooks/use-feature-flags";
|
||||
import { useInsightsLayoutQuery } from "@/hooks/use-insights-layout";
|
||||
import { usePullToRefresh } from "@/hooks/use-pull-to-refresh";
|
||||
@@ -527,7 +530,7 @@ export default function InsightsPageClient() {
|
||||
"health-status": <HealthStatusCard enabled={isAuthenticated} />,
|
||||
breathing: <BreathingScreeningCard enabled={isAuthenticated} />,
|
||||
"labs-changes": <LabsChangesCard enabled={isAuthenticated} />,
|
||||
ecg: <EcgSection enabled={isAuthenticated} />,
|
||||
ecg: <EcgSection enabled={isAuthenticated} limit={ECG_OVERVIEW_LIMIT} />,
|
||||
};
|
||||
|
||||
const orderedSectionIds = orderedVisibleSectionIds(layout);
|
||||
|
||||
@@ -28,7 +28,7 @@ vi.mock("@tanstack/react-query", () => ({
|
||||
useQuery: (opts: unknown) => useQueryMock(opts),
|
||||
}));
|
||||
|
||||
const { EcgSection, ECG_LIST_LIMIT } = await import("../ecg-section");
|
||||
const { EcgSection, ECG_OVERVIEW_LIMIT } = await import("../ecg-section");
|
||||
type EcgRecordingListItem = import("../ecg-section").EcgRecordingListItem;
|
||||
|
||||
const IRREGULAR_REC: EcgRecordingListItem = {
|
||||
@@ -53,11 +53,12 @@ const NORMAL_REC: EcgRecordingListItem = {
|
||||
function renderSection(
|
||||
data:
|
||||
{ recordings: EcgRecordingListItem[]; hasRecordings: boolean } | undefined,
|
||||
props: { limit?: number } = {},
|
||||
) {
|
||||
useQueryMock.mockReturnValue({ data, isLoading: false });
|
||||
return renderToStaticMarkup(
|
||||
<I18nProvider initialLocale="en">
|
||||
<EcgSection />
|
||||
<EcgSection {...props} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
}
|
||||
@@ -99,30 +100,52 @@ describe("<EcgSection>", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("caps the list at the most recent handful", () => {
|
||||
// Eight strips in, five out — and the five are the ones the route
|
||||
// handed back first, which it orders `recordedAt desc`.
|
||||
const recordings = Array.from({ length: 8 }, (_, index) => ({
|
||||
const eightRecordings = () =>
|
||||
Array.from({ length: 8 }, (_, index) => ({
|
||||
...NORMAL_REC,
|
||||
id: `ecg_${index}`,
|
||||
recordedAt: new Date(Date.UTC(2026, 5, 20 - index, 9, 15)).toISOString(),
|
||||
}));
|
||||
const html = renderSection({ recordings, hasRecordings: true });
|
||||
|
||||
expect(ECG_LIST_LIMIT).toBe(5);
|
||||
expect(countRows(html)).toBe(ECG_LIST_LIMIT);
|
||||
for (const kept of recordings.slice(0, ECG_LIST_LIMIT)) {
|
||||
it("caps the OVERVIEW teaser at the most recent handful", () => {
|
||||
// The teaser passes `limit`. Eight strips in, five out — and the five are
|
||||
// the ones the route handed back first, which it orders `recordedAt desc`.
|
||||
const recordings = eightRecordings();
|
||||
const html = renderSection(
|
||||
{ recordings, hasRecordings: true },
|
||||
{ limit: ECG_OVERVIEW_LIMIT },
|
||||
);
|
||||
|
||||
expect(ECG_OVERVIEW_LIMIT).toBe(5);
|
||||
expect(countRows(html)).toBe(ECG_OVERVIEW_LIMIT);
|
||||
for (const kept of recordings.slice(0, ECG_OVERVIEW_LIMIT)) {
|
||||
expect(html, `${kept.id} should be listed`).toContain(
|
||||
`/insights/ecg/${kept.id}`,
|
||||
);
|
||||
}
|
||||
for (const dropped of recordings.slice(ECG_LIST_LIMIT)) {
|
||||
expect(html, `${dropped.id} is past the cap`).not.toContain(
|
||||
for (const dropped of recordings.slice(ECG_OVERVIEW_LIMIT)) {
|
||||
expect(html, `${dropped.id} is past the teaser cap`).not.toContain(
|
||||
`/insights/ecg/${dropped.id}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("renders every recording when NO limit is given (the full page)", () => {
|
||||
// The dedicated `/insights/ecg` page passes no `limit`, so an account with
|
||||
// many strips reaches all of them — the cap is a teaser affordance, never
|
||||
// a wall on the full surface. Eight seeded, eight rendered, every one
|
||||
// reachable by its own address.
|
||||
const recordings = eightRecordings();
|
||||
const html = renderSection({ recordings, hasRecordings: true });
|
||||
|
||||
expect(countRows(html)).toBe(8);
|
||||
for (const rec of recordings) {
|
||||
expect(html, `${rec.id} must be reachable on the full page`).toContain(
|
||||
`/insights/ecg/${rec.id}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("opens the recording's own address when it has a waveform", () => {
|
||||
const html = renderSection({
|
||||
recordings: [IRREGULAR_REC],
|
||||
|
||||
@@ -12,10 +12,16 @@ import { cn } from "@/lib/utils";
|
||||
import { SectionHeading } from "@/components/ui/section-heading";
|
||||
import {
|
||||
ecgResultLabel,
|
||||
ECG_OVERVIEW_LIMIT,
|
||||
type EcgClassification,
|
||||
} from "@/lib/insights/ecg-classification";
|
||||
import { InsightSectionCard } from "./insight-section-card";
|
||||
|
||||
// Re-exported so the tests (and any teaser call site) can keep importing the
|
||||
// overview cap from the component they are exercising; the value itself lives
|
||||
// in the React-free lib module.
|
||||
export { ECG_OVERVIEW_LIMIT };
|
||||
|
||||
/**
|
||||
* v1.28.50 — ECG recording list.
|
||||
*
|
||||
@@ -38,12 +44,6 @@ import { InsightSectionCard } from "./insight-section-card";
|
||||
* when the user has no recordings — never an empty / alarming card.
|
||||
*/
|
||||
|
||||
/**
|
||||
* How many strips the list paints. The API already caps its own read at 200;
|
||||
* this is the reading limit, not the fetch limit.
|
||||
*/
|
||||
export const ECG_LIST_LIMIT = 5;
|
||||
|
||||
export interface EcgRecordingListItem {
|
||||
id: string;
|
||||
recordedAt: string;
|
||||
@@ -72,12 +72,20 @@ interface EcgSectionProps {
|
||||
* teaser keeps its own heading unchanged.
|
||||
*/
|
||||
hideHeading?: boolean;
|
||||
/**
|
||||
* Cap the number of strips shown. The overview teaser passes
|
||||
* `ECG_OVERVIEW_LIMIT`; the dedicated `/insights/ecg` page passes nothing,
|
||||
* so it renders the full list. Omitted = no cap: a full surface must never
|
||||
* silently hide a recording behind a limit with no way to reach it.
|
||||
*/
|
||||
limit?: number;
|
||||
}
|
||||
|
||||
export function EcgSection({
|
||||
enabled = true,
|
||||
className,
|
||||
hideHeading = false,
|
||||
limit,
|
||||
}: EcgSectionProps) {
|
||||
const { isAuthenticated } = useAuth();
|
||||
const { t } = useTranslations();
|
||||
@@ -98,9 +106,11 @@ export function EcgSection({
|
||||
// Data-availability gate — never paint an empty card.
|
||||
if (isLoading || !data || !data.hasRecordings) return null;
|
||||
|
||||
// The route hands them back most-recent-first; the slice is the reading
|
||||
// limit on top of that, so the newest strips are the ones on screen.
|
||||
const recordings = data.recordings.slice(0, ECG_LIST_LIMIT);
|
||||
// The route hands them back most-recent-first. A `limit` (the overview
|
||||
// teaser) keeps the newest few; no limit (the dedicated page) shows them
|
||||
// all, so an older strip is never made unreachable.
|
||||
const recordings =
|
||||
limit != null ? data.recordings.slice(0, limit) : data.recordings;
|
||||
|
||||
return (
|
||||
<section
|
||||
|
||||
@@ -13,6 +13,15 @@
|
||||
export type EcgClassification =
|
||||
"IRREGULAR" | "NOT_DETECTED" | "INCONCLUSIVE" | null;
|
||||
|
||||
/**
|
||||
* How many strips the OVERVIEW teaser paints. It is a glance, not a history;
|
||||
* the dedicated `/insights/ecg` page passes no limit and shows every
|
||||
* recording. The constant lives here — a pure value with no React import — so
|
||||
* the `/insights` page can pass it without pulling the ECG component eagerly
|
||||
* across its dynamic boundary.
|
||||
*/
|
||||
export const ECG_OVERVIEW_LIMIT = 5;
|
||||
|
||||
/** i18n key per device verdict, under `insights.ecg.result.*`. */
|
||||
export const ECG_RESULT_LABEL_KEYS: Record<string, string> = {
|
||||
IRREGULAR: "insights.ecg.result.irregular",
|
||||
|
||||
Reference in new issue
Block a user