From a2a3414b2944cbef702b847f071e72e2e633e4e8 Mon Sep 17 00:00:00 2001 From: BernhardBuckel <33808384+BernhardBuckel@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:37:41 +0200 Subject: [PATCH] feat(labs): give the lab-scan review stage the width a large screen offers The review stage is a list of readings to compare and correct, but it sat in the default 448 px dialog, so with several readings it only grew taller. - ResponsiveSheet gains an opt-in 6xl width tier (72rem). Nothing else changes width; the Sheet branch on phones ignores the tier as before. - The review stage uses it. The picking stage stays narrow. - Each row lays out from the width of its container, not the window: stacked in a narrow dialog, and once the dialog is wide enough the reading sits on the left with its fields on the right. - The field grid (value, unit, date) is a container of its own, so it takes three columns when the fields have room for them, whatever the window width. The result-text span follows the same query. - The dialog-overflow e2e spec covers the review stage at 390, 560, 768, 1280, 1920 and 3440 px. 560 is wider than a phone's field grid needs but below the window sm breakpoint, which is what tells a container query from a window one. Refs #1056 --- e2e/dialog-horizontal-overflow.spec.ts | 197 ++++++++ .../labs/__tests__/ocr-row-editor.test.tsx | 33 ++ src/components/labs/ocr-review-dialog.tsx | 4 + src/components/labs/ocr-row-editor.tsx | 459 +++++++++--------- .../ui/__tests__/responsive-sheet.test.tsx | 40 ++ src/components/ui/responsive-sheet.tsx | 3 +- 6 files changed, 516 insertions(+), 220 deletions(-) diff --git a/e2e/dialog-horizontal-overflow.spec.ts b/e2e/dialog-horizontal-overflow.spec.ts index 4fed8626f..6f9ebbe21 100644 --- a/e2e/dialog-horizontal-overflow.spec.ts +++ b/e2e/dialog-horizontal-overflow.spec.ts @@ -1,6 +1,7 @@ import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; +import { aiBlockAvailable, serveAiBlock } from "./setup/ai-capabilities"; /** * An add / edit dialog must not pan sideways, and nothing you have to operate @@ -290,3 +291,199 @@ test.describe("add/edit dialogs do not scroll sideways", () => { ); }); }); + +/** + * The lab-scan review stage is a list of readings to compare and correct, so + * it takes the room a large screen offers while phones keep the bottom sheet. + * Measured at the widths that matter: a phone, a tablet, the shared 1280 px + * frame, full HD and an ultrawide. + * + * The rows lay themselves out from the width of the dialog (a container + * query), so the assertion is about the RESULT: where the reading and its + * fields sit relative to each other, not which class produced it. + */ +const REVIEW_VIEWPORTS = [ + { + label: "phone 390", + width: 390, + height: 844, + wide: false, + threeAcross: false, + }, + // Below the window `sm` breakpoint (640 px) yet with a dialog body well past + // the container `@md` breakpoint (448 px): the fields have room for three + // columns even though the window is "small". This is the width that tells + // "follows the dialog" from "follows the window". + { + label: "small tablet 560", + width: 560, + height: 900, + wide: false, + threeAcross: true, + }, + { + label: "tablet 768", + width: 768, + height: 1024, + wide: false, + threeAcross: true, + }, + { + label: "desktop 1280", + width: 1280, + height: 800, + wide: true, + threeAcross: true, + }, + { + label: "full HD 1920", + width: 1920, + height: 1080, + wide: true, + threeAcross: true, + }, + { + label: "ultrawide 3440", + width: 3440, + height: 1440, + wide: true, + threeAcross: true, + }, +]; + +const REVIEW_ROWS = ["LDL", "HDL", "Glucose", "Creatinine", "ALT", "CRP"].map( + (analyte, index) => ({ + analyte, + value: 1 + index, + valueText: null, + unit: "mmol/L", + referenceLow: 0, + referenceHigh: 5, + referenceText: "0 - 5", + takenAt: index === 0 ? null : "2026-06-10", + confidence: { analyte: 0.9, value: 0.9, unit: 0.9, range: 0.9 }, + biomarkerMatch: "existing", + duplicateOf: null, + }), +); + +// One pixel: enough to be a valid image for the picker, and never decoded. +const PIXEL_PNG = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==", + "base64", +); + +test.describe("lab-scan review dialog uses the width it is given", () => { + test.use({ storageState: STORAGE_STATE_PATH }); + + for (const vp of REVIEW_VIEWPORTS) { + test(`review stage @${vp.label}`, async ({ page }) => { + await page.setViewportSize({ width: vp.width, height: vp.height }); + await serveAiBlock(page, aiBlockAvailable()); + await page.route("**/api/labs/ocr/capability", (route) => + route.fulfill({ + json: { + data: { + available: true, + mode: "vision", + reason: null, + pdfSupported: true, + }, + error: null, + }, + }), + ); + await page.route("**/api/labs/ocr/extract", (route) => + route.fulfill({ + json: { + data: { + reportDate: null, + providerType: "stub", + rows: REVIEW_ROWS, + }, + error: null, + }, + }), + ); + + await page.goto("/labs", { waitUntil: "domcontentloaded" }); + await page.getByRole("button", { name: /^add$/i }).first().click(); + await page.getByRole("menuitem", { name: /scan/i }).click(); + + const surface = page.locator(SURFACE).first(); + await expect(surface).toBeVisible(); + + // The picking stage is a single control and stays narrow. + const pickWidth = (await surface.boundingBox())!.width; + if (!vp.wide) expect(pickWidth).toBeLessThanOrEqual(vp.width); + else expect(pickWidth).toBeLessThan(500); + + await page.locator('input[type="file"]').first().setInputFiles({ + name: "report.png", + mimeType: "image/png", + buffer: PIXEL_PNG, + }); + const rows = surface.locator("input[id$='-val']"); + await expect(rows.first()).toBeVisible(); + await expect(rows).toHaveCount(REVIEW_ROWS.length); + + const label = `scan review @${vp.label}`; + await expectNoSidewaysScroll(page, label); + await expectControlsInsideBox(page, label); + + const box = (await surface.boundingBox())!; + const analyte = (await surface + .getByLabel("Test name") + .first() + .boundingBox())!; + const value = (await rows.first().boundingBox())!; + const unit = (await surface + .locator("input[id$='-unit']") + .first() + .boundingBox())!; + const date = (await surface + .locator('[data-slot="date-field"]') + .first() + .boundingBox())!; + + // Value, unit and date share a line when the fields have room for three + // columns, and the date wraps under them when they do not. + if (vp.threeAcross) { + expect( + Math.abs(unit.y - value.y), + `${label}: unit beside value`, + ).toBeLessThan(8); + expect( + Math.abs(date.y - value.y), + `${label}: date beside value`, + ).toBeLessThan(8); + } else { + expect( + Math.abs(unit.y - value.y), + `${label}: unit beside value`, + ).toBeLessThan(8); + expect(date.y, `${label}: date wraps`).toBeGreaterThan( + value.y + value.height - 1, + ); + } + + if (vp.wide) { + // Room to compare: clearly wider than the old 448 px column, and never + // past the readable cap (72rem = 1152 px). + expect(box.width, `${label}: dialog width`).toBeGreaterThan(700); + expect(box.width, `${label}: dialog width`).toBeLessThanOrEqual(1153); + // The reading and its fields share a line: the value sits to the right + // of the analyte, not underneath it. + expect(value.x, `${label}: value beside analyte`).toBeGreaterThan( + analyte.x + analyte.width - 1, + ); + expect(Math.abs(value.y - analyte.y)).toBeLessThan(80); + } else { + // Narrow: stacked exactly as before. + expect(value.y, `${label}: value under analyte`).toBeGreaterThan( + analyte.y + analyte.height - 1, + ); + } + }); + } +}); diff --git a/src/components/labs/__tests__/ocr-row-editor.test.tsx b/src/components/labs/__tests__/ocr-row-editor.test.tsx index ca85eda5d..88e28812a 100644 --- a/src/components/labs/__tests__/ocr-row-editor.test.tsx +++ b/src/components/labs/__tests__/ocr-row-editor.test.tsx @@ -82,3 +82,36 @@ describe(" required-field marking", () => { expect(render({ date: true }, "de")).toContain("Bitte ein Datum angeben."); }); }); + +describe(" layout", () => { + it("lays out from the width of its container, not the window", () => { + const html = render(undefined); + expect(html).toContain('class="@container"'); + // Two columns only once the container is wide; stacked below that. + expect(html).toContain("@3xl:grid-cols-"); + }); + + it("sizes the field grid from its own container, never from the window", () => { + const numeric = render(undefined); + expect(numeric).toContain("@md:grid-cols-3"); + expect(numeric).not.toMatch(/(? { + const html = render(undefined); + for (const id of ["-val", "-unit", "-date", "-lo", "-hi", "-refText"]) { + expect(html, `field ${id}`).toContain(`${id}"`); + } + }); +}); diff --git a/src/components/labs/ocr-review-dialog.tsx b/src/components/labs/ocr-review-dialog.tsx index 707da5efa..f734509a6 100644 --- a/src/components/labs/ocr-review-dialog.tsx +++ b/src/components/labs/ocr-review-dialog.tsx @@ -271,6 +271,10 @@ export function OcrReviewDialog({ open={open} onOpenChange={handleClose} title={t("labs.ocr.reviewTitle")} + // Picking a file is a single control and stays narrow. Reviewing is a + // list of readings to compare and correct, which wants the room; the + // rows lay themselves out from the width they are given. + contentWidth={stage === "review" ? "6xl" : "md"} description={ stage === "pick" ? t("labs.ocr.uploadHint") : t("labs.ocr.reviewHint") } diff --git a/src/components/labs/ocr-row-editor.tsx b/src/components/labs/ocr-row-editor.tsx index e63388a55..306e5eeb9 100644 --- a/src/components/labs/ocr-row-editor.tsx +++ b/src/components/labs/ocr-row-editor.tsx @@ -97,243 +97,264 @@ export function OcrRowEditor({ const flagRange = isNewNumericMarker && (lowRangeConfidence || !hasBounds); return ( -
-
- {/* The checkbox is the PRIMARY per-row confirm on a touch-first OCR + // `@container` makes the row follow the width of the dialog, not the + // window: in a narrow dialog (or on a phone) it stacks as before; once the + // dialog is wide enough it splits into the reading on the left and its + // fields on the right, so a long report reads as a table, not a column. +
+
+
+ {/* The checkbox is the PRIMARY per-row confirm on a touch-first OCR review screen, so it needs a ≥44px hit area on coarse pointers. The wrapping label supplies the touch target (and forwards the click to the control) without enlarging the 20px glyph or bloating the row on desktop, where the negative margins collapse the padding back. */} - -
- onChange({ ...row, analyte: e.target.value })} - aria-label={t("labs.ocr.analyteLabel")} - {...invalidProps("analyte")} - className="font-medium" - /> - {fieldError("analyte", "labs.ocr.errorAnalyte")} -
- {row.biomarkerMatch === "existing" ? ( - // The chip repeats the analyte, so a long panel name ran past - // the row on a phone; it truncates inside the row instead. - - - - {t("labs.ocr.linksExisting", { name: row.analyte })} - - - ) : ( - - - {t("labs.ocr.newBiomarker")} - - )} - {row.duplicateOf ? ( - - - {t("labs.ocr.duplicateWarning")} - - ) : null} - {lowRowConfidence ? ( - - - {t("labs.ocr.lowConfidenceBadge")} - - ) : null} -
-
-
- -
- {isQualitative ? ( -
- - onChange({ ...row, valueText: e.target.value })} - {...invalidProps("valueText")} + +
+ onChange({ ...row, analyte: e.target.value })} + aria-label={t("labs.ocr.analyteLabel")} + {...invalidProps("analyte")} + className="font-medium" + /> + {fieldError("analyte", "labs.ocr.errorAnalyte")} +
+ {row.biomarkerMatch === "existing" ? ( + // The chip repeats the analyte, so a long panel name ran past + // the row on a phone; it truncates inside the row instead. + + + + {t("labs.ocr.linksExisting", { name: row.analyte })} + + + ) : ( + + + {t("labs.ocr.newBiomarker")} + + )} + {row.duplicateOf ? ( + + + {t("labs.ocr.duplicateWarning")} + + ) : null} + {lowRowConfidence ? ( + + + {t("labs.ocr.lowConfidenceBadge")} + + ) : null} +
- ) : ( - <> -
- - { - const raw = e.target.value.trim(); - const parsed = raw === "" ? null : Number(raw); - onChange({ - ...row, - value: - parsed !== null && Number.isFinite(parsed) - ? parsed - : null, - }); - }} - {...invalidProps("value")} - /> - {fieldError("value", "labs.ocr.errorValue")} -
-
- - onChange({ ...row, unit: e.target.value })} - {...invalidProps("unit")} - /> - {fieldError("unit", "labs.ocr.errorUnit")} -
- - )} -
- - onChange({ ...row, takenAt: value || null })} - {...invalidProps("date")} - // The bordered wrapper is the visible control; the input inside - // it only carries the aria state. - className={errors?.date ? "border-destructive" : undefined} - /> - {fieldError("date", "labs.ocr.errorDate")}
-
- {/* v1.18.10 (#5) — for a NEW numeric biomarker the reference range below + {/* The fields are a container of their own, so how many columns they + get follows the width they actually have: the whole row when + stacked, the right-hand column once the row splits. */} +
+
+ {isQualitative ? ( +
+ + + onChange({ ...row, valueText: e.target.value }) + } + {...invalidProps("valueText")} + /> + {fieldError("valueText", "labs.ocr.errorResult")} +
+ ) : ( + <> +
+ + { + const raw = e.target.value.trim(); + const parsed = raw === "" ? null : Number(raw); + onChange({ + ...row, + value: + parsed !== null && Number.isFinite(parsed) + ? parsed + : null, + }); + }} + {...invalidProps("value")} + /> + {fieldError("value", "labs.ocr.errorValue")} +
+
+ + onChange({ ...row, unit: e.target.value })} + {...invalidProps("unit")} + /> + {fieldError("unit", "labs.ocr.errorUnit")} +
+ + )} +
+ + + onChange({ ...row, takenAt: value || null }) + } + {...invalidProps("date")} + // The bordered wrapper is the visible control; the input inside + // it only carries the aria state. + className={errors?.date ? "border-destructive" : undefined} + /> + {fieldError("date", "labs.ocr.errorDate")} +
+
+ + {/* v1.18.10 (#5) — for a NEW numeric biomarker the reference range below becomes the marker's catalog range. Make that consequence visible, and flag low range-confidence / a missing range so the user verifies it before saving. A calm `secondary` badge, not an alarm. */} - {isNewNumericMarker ? ( -
- - - {flagRange - ? t("labs.ocr.rangeVerify") - : t("labs.ocr.rangeFromScan")} - -
- ) : null} + {isNewNumericMarker ? ( +
+ + + {flagRange + ? t("labs.ocr.rangeVerify") + : t("labs.ocr.rangeFromScan")} + +
+ ) : null} - {!isQualitative ? ( -
-
- - { - const raw = e.target.value.trim(); - const parsed = raw === "" ? null : Number(raw); - onChange({ - ...row, - referenceLow: - parsed !== null && Number.isFinite(parsed) ? parsed : null, - }); - }} - /> -
-
- - { - const raw = e.target.value.trim(); - const parsed = raw === "" ? null : Number(raw); - onChange({ - ...row, - referenceHigh: - parsed !== null && Number.isFinite(parsed) ? parsed : null, - }); - }} - /> -
-
- ) : null} + {!isQualitative ? ( +
+
+ + { + const raw = e.target.value.trim(); + const parsed = raw === "" ? null : Number(raw); + onChange({ + ...row, + referenceLow: + parsed !== null && Number.isFinite(parsed) + ? parsed + : null, + }); + }} + /> +
+
+ + { + const raw = e.target.value.trim(); + const parsed = raw === "" ? null : Number(raw); + onChange({ + ...row, + referenceHigh: + parsed !== null && Number.isFinite(parsed) + ? parsed + : null, + }); + }} + /> +
+
+ ) : null} - {/* The window as the report printed it. Editable, because the human is + {/* The window as the report printed it. Editable, because the human is reviewing a transcription: the two bounds above are the derived reading of this string, and correcting the string is how a range the parser could not read ("bis 5,0", "negativ") still reaches the row instead of being dropped. */} - {!isQualitative ? ( -
- - { - const raw = e.target.value; - onChange({ - ...row, - referenceText: raw.trim() === "" ? null : raw, - }); - }} - /> -
- ) : null} + {!isQualitative ? ( +
+ + { + const raw = e.target.value; + onChange({ + ...row, + referenceText: raw.trim() === "" ? null : raw, + }); + }} + /> +
+ ) : null} - {valueUnreadable ? ( -

- - {t("labs.ocr.valueUnreadable")} -

- ) : lowValueConfidence ? ( -

- - {t("labs.ocr.lowConfidence")} -

- ) : null} + {valueUnreadable ? ( +

+ + {t("labs.ocr.valueUnreadable")} +

+ ) : lowValueConfidence ? ( +

+ + {t("labs.ocr.lowConfidence")} +

+ ) : null} +
+
); } diff --git a/src/components/ui/__tests__/responsive-sheet.test.tsx b/src/components/ui/__tests__/responsive-sheet.test.tsx index 7ba0c66c2..23d19c281 100644 --- a/src/components/ui/__tests__/responsive-sheet.test.tsx +++ b/src/components/ui/__tests__/responsive-sheet.test.tsx @@ -142,6 +142,46 @@ describe("", () => { expect(html).toContain("pr-3"); }); + it.each([ + [undefined, "sm:max-w-md"], + ["md", "sm:max-w-md"], + ["4xl", "sm:max-w-4xl"], + ["6xl", "sm:max-w-6xl"], + ] as const)( + "maps contentWidth %s to %s on the Dialog branch, and to no other tier", + (contentWidth, expected) => { + mobile = false; + const html = renderToStaticMarkup( + {}} + title="Review" + contentWidth={contentWidth} + > +

body

+ , + ); + const tiers = html.match(/sm:max-w-(?:md|lg|2xl|3xl|4xl|6xl)/g); + expect(tiers).toEqual([expected]); + }, + ); + + it("does not apply a width tier on the Sheet branch", () => { + mobile = true; + const html = renderToStaticMarkup( + {}} + title="Review" + contentWidth="6xl" + > +

body

+ , + ); + expect(html).not.toContain("sm:max-w-6xl"); + mobile = false; + }); + it("renders the bottom Sheet branch on narrow viewports", () => { mobile = true; const html = renderToStaticMarkup( diff --git a/src/components/ui/responsive-sheet.tsx b/src/components/ui/responsive-sheet.tsx index b9b77ca48..c25175db8 100644 --- a/src/components/ui/responsive-sheet.tsx +++ b/src/components/ui/responsive-sheet.tsx @@ -87,7 +87,7 @@ export interface ResponsiveSheetProps { * consumer-passed width silently collide with the hardcoded * default, so the width now resolves from this single source. */ - contentWidth?: "md" | "lg" | "2xl" | "3xl" | "4xl"; + contentWidth?: "md" | "lg" | "2xl" | "3xl" | "4xl" | "6xl"; children: React.ReactNode; } @@ -100,6 +100,7 @@ const CONTENT_WIDTH_CLASS: Record< "2xl": "sm:max-w-2xl", "3xl": "sm:max-w-3xl", "4xl": "sm:max-w-4xl", + "6xl": "sm:max-w-6xl", }; /**