From 406f8317875489475ecf25652f5592e71e3988cc Mon Sep 17 00:00:00 2001 From: angela-helios Date: Thu, 27 Aug 2026 10:36:39 -0400 Subject: [PATCH] fix(ui): batch/transaction round 3 - footer, errors, focus, header count, tab icons Closes #726. Closes #727. Closes #729. Closes #730. Closes #731. Closes #732. The bottom Cancel/Execute footer goes away; the top row is the only action row, and the execute error moves out of it to a full-width alert directly above the Execution Plan card, where a failure is visible next to the button that raised it regardless of plan length. reset() now hides both error elements - hiding a stage never cleared them, so an abandoned attempt's error greeted the next one. The preflight reveal keeps its focus handoff but passes preventScroll: on a plan taller than the viewport the default scroll-into-view pinned the Execute row to the top of the page. The stage's programmatic focus also no longer paints the browser's default ring around the section. The created count moves into the response card head, left of the status badge; updated/other/failed keep the summary line below, which hides when empty. The Actions and Bundle JSON tabs gain their design icons (sliders, code). --- crates/ui/assets/app.css | 24 +++++++++ crates/ui/assets/batch.js | 24 ++++++--- crates/ui/e2e/tests/batch.spec.ts | 75 +++++++++++++++++++++------- crates/ui/templates/pages/batch.html | 24 ++++----- 4 files changed, 108 insertions(+), 39 deletions(-) diff --git a/crates/ui/assets/app.css b/crates/ui/assets/app.css index cbe14deaf..c27a268f9 100644 --- a/crates/ui/assets/app.css +++ b/crates/ui/assets/app.css @@ -4700,6 +4700,9 @@ details.json-fold[open] > summary .icon { } .batch-tab { + display: inline-flex; + align-items: center; + gap: 8px; padding: 8px 14px; border: 1px solid var(--divider); border-radius: 999px; @@ -4847,6 +4850,27 @@ details.json-fold[open] > summary .icon { color: var(--muted); } +/* The card head's right-hand cluster (#729): created count beside the + status badge. */ +.card-head__meta { + display: flex; + align-items: center; + gap: 10px; +} + +.batch-created { + color: var(--muted); + font-size: 12px; + font-variant-numeric: tabular-nums; +} + +/* The stage takes programmatic focus when revealed (#679); it is not + keyboard-reachable, so the browser's default ring around the whole + section is noise (#726). */ +#batch-preflight:focus { + outline: none; +} + .batch-json { margin: 0; diff --git a/crates/ui/assets/batch.js b/crates/ui/assets/batch.js index 54cb35112..cd762e68d 100644 --- a/crates/ui/assets/batch.js +++ b/crates/ui/assets/batch.js @@ -43,10 +43,9 @@ var outcomes = document.getElementById("batch-outcomes"); var overall = document.getElementById("batch-overall"); var summary = document.getElementById("batch-summary"); - var executeBtn = document.getElementById("batch-execute"); var executeTopBtn = document.getElementById("batch-execute-top"); - var cancelBtn = document.getElementById("batch-cancel"); var cancelTopBtn = document.getElementById("batch-cancel-top"); + var createdBadge = document.getElementById("batch-created"); var doneBtn = document.getElementById("batch-done"); var busyRegion = document.getElementById("batch-busy"); @@ -194,7 +193,9 @@ revealed stage, not its primary action. */ var active = document.activeElement; if (active === document.body || stages.upload.contains(active)) { - stages.preflight.focus(); + /* preventScroll (#732): on a plan taller than the viewport the + default scroll-into-view pinned the Execute row to the top. */ + stages.preflight.focus({ preventScroll: true }); } } finally { busy.done(); @@ -301,6 +302,11 @@ bundle = null; fileInput.value = ""; clearPreflight(); + /* Errors are page-level elements, not stage content: hiding a stage + leaves them set, so an abandoned attempt's error would greet the next + one (#731). */ + uploadError.hidden = true; + executeError.hidden = true; show("upload"); /* Done/Cancel hid the stage that held focus; land on the one action the upload stage offers (#679). Containment, not body: the focus @@ -314,7 +320,6 @@ drop.focus(); } } - cancelBtn.addEventListener("click", reset); cancelTopBtn.addEventListener("click", reset); /* ---- stage 3: execute and report ------------------------------------ */ @@ -331,7 +336,7 @@ crashed the settling renderResponse. Busy holds until the outcome is rendered, not merely until response headers arrive. */ hfsBusy.during( - [executeBtn, executeTopBtn], + [executeTopBtn], function () { return fetch("/", { method: "POST", @@ -350,10 +355,9 @@ executeError.hidden = false; }); }, - { alsoDisable: [cancelBtn, cancelTopBtn], region: busyRegion, label: messages.msgExecuting } + { alsoDisable: [cancelTopBtn], region: busyRegion, label: messages.msgExecuting } ); } - executeBtn.addEventListener("click", execute); executeTopBtn.addEventListener("click", execute); /* The run already happened: Done lands back on a clean upload stage rather than offering to re-run a mutation that succeeded (#675). */ @@ -416,12 +420,16 @@ outcomes.appendChild(li); }); + /* The created count reads in the card head next to the status badge + (#729); the rest of the tally keeps the summary line, failures above + all. */ + createdBadge.textContent = created ? created + " " + messages.msgCreated : ""; var parts = []; - if (created) parts.push(created + " " + messages.msgCreated); if (updated) parts.push(updated + " " + messages.msgUpdated); if (other) parts.push(other + " " + messages.msgOther); if (failed) parts.push(failed + " " + messages.msgFailed); summary.textContent = parts.join(" · "); + summary.hidden = !parts.length; show("response"); /* The disabled trigger was hidden with its stage; land on the one diff --git a/crates/ui/e2e/tests/batch.spec.ts b/crates/ui/e2e/tests/batch.spec.ts index 52a0528f3..67bb3f2c9 100644 --- a/crates/ui/e2e/tests/batch.spec.ts +++ b/crates/ui/e2e/tests/batch.spec.ts @@ -72,16 +72,19 @@ test("a transaction bundle uploads, previews, executes, and reports", async ({ p await expect(page.locator("#batch-json .json-view")).toBeVisible(); await page.locator("#batch-tab-actions").click(); - // The execute error slot lives inside the footer, next to its button (#676). - await expect(page.locator(".batch-footer #batch-execute-error")).toHaveCount(1); + // The execute error slot sits above the plan card (#730), and the bottom + // footer is gone — Cancel/Execute exist once, at the top. + await expect(page.locator("#batch-execute-error + .card")).toHaveCount(1); + await expect(page.locator("#batch-preflight .batch-footer")).toHaveCount(1); // Execute: outcomes per entry plus the aggregate summary. - await page.locator("#batch-execute").click(); + await page.locator("#batch-execute-top").click(); await expect(page.locator("#batch-response")).toBeVisible(); await expect(page.locator("#batch-outcomes .batch-row")).toHaveCount(2); await expect(page.locator("#batch-outcomes .batch-badge").first()).toContainText("201"); - await expect(page.locator("#batch-summary")).toContainText("2"); - await expect(page.locator("#batch-summary")).toContainText(/created/i); + // The created count reads in the card head, beside the status badge (#729). + await expect(page.locator("#batch-created")).toContainText("2"); + await expect(page.locator("#batch-created")).toContainText(/created/i); await expect(page.locator("#batch-overall")).toHaveClass(/--ok/); // Done is the one way out (#675): back to a clean upload stage. Done hid @@ -213,7 +216,7 @@ test("an invalid replacement clears the old preview and cannot execute stale or // Exercise the defensive guard even though the hidden control cannot be // reached by a user while the upload stage is visible. - await page.locator("#batch-execute").evaluate((button: HTMLButtonElement) => button.click()); + await page.locator("#batch-execute-top").evaluate((button: HTMLButtonElement) => button.click()); expect(executeRequests).toBe(0); await expect(page.locator("#batch-upload-error")).toBeVisible(); }); @@ -320,7 +323,7 @@ test("a non-bundle file is rejected with a message, not a crash", async ({ page // The transient states are made deterministically observable: FileReader // delivery and the execute POST are both parked behind manual releases. -const FOOTER_CONTROLS = ["#batch-execute", "#batch-execute-top", "#batch-cancel", "#batch-cancel-top"]; +const FOOTER_CONTROLS = ["#batch-execute-top", "#batch-cancel-top"]; test("picking a file shows the busy region before any file bytes arrive", async ({ page }) => { await page.goto("/ui/batch", { waitUntil: "networkidle" }); @@ -373,12 +376,12 @@ test("execute busies the whole footer, ignores re-entrant clicks, and lands focu await page.goto("/ui/batch", { waitUntil: "networkidle" }); await page.locator("#batch-file").setInputFiles(bundleFile("batch")); await expect(page.locator("#batch-preflight")).toBeVisible(); - await page.locator("#batch-execute").click(); + await page.locator("#batch-execute-top").click(); // Both Execute copies spin, and the Cancels go inert with them: a // mid-flight Cancel raced the settling response and crashed on the nulled // bundle before #679. - await expect(page.locator("#batch-execute")).toHaveAttribute("aria-busy", "true"); + await expect(page.locator("#batch-execute-top")).toHaveAttribute("aria-busy", "true"); await expect(page.locator("#batch-execute-top")).toHaveAttribute("aria-busy", "true"); for (const control of FOOTER_CONTROLS) await expect(page.locator(control)).toBeDisabled(); await expect(page.locator("#batch-busy")).toBeVisible(); @@ -387,7 +390,7 @@ test("execute busies the whole footer, ignores re-entrant clicks, and lands focu // The default-motion busy button: the label yields to the animated ring, // and the filled primary gets the explicit white ring (accent-on-accent // vanishes in dark theme). - const busyStyle = await page.locator("#batch-execute").evaluate((button) => ({ + const busyStyle = await page.locator("#batch-execute-top").evaluate((button) => ({ color: getComputedStyle(button).color, content: getComputedStyle(button, "::after").content, animation: getComputedStyle(button, "::after").animationName, @@ -401,7 +404,7 @@ test("execute busies the whole footer, ignores re-entrant clicks, and lands focu // Re-entrant activation cannot double-POST: a real click on a disabled // button dispatches nothing, and a synthetic event that does reach the // handler is ignored by the busy guard. - await page.locator("#batch-execute").evaluate((button: HTMLButtonElement) => button.click()); + await page.locator("#batch-execute-top").evaluate((button: HTMLButtonElement) => button.click()); await page .locator("#batch-execute-top") .evaluate((button) => button.dispatchEvent(new MouseEvent("click", { bubbles: true }))); @@ -444,10 +447,10 @@ test("a whole-bundle failure clears the busy state and re-enables the footer", a await page.goto("/ui/batch", { waitUntil: "networkidle" }); await page.locator("#batch-file").setInputFiles(bundleFile("batch")); - await page.locator("#batch-execute").click(); + await page.locator("#batch-execute-top").click(); // The busy state is genuinely entered before the failure lands… - await expect(page.locator("#batch-execute")).toHaveAttribute("aria-busy", "true"); + await expect(page.locator("#batch-execute-top")).toHaveAttribute("aria-busy", "true"); await expect(page.locator("#batch-busy")).toBeVisible(); release(); @@ -456,10 +459,44 @@ test("a whole-bundle failure clears the busy state and re-enables the footer", a await expect(page.locator("#batch-execute-error")).toBeVisible(); await expect(page.locator("#batch-preflight")).toBeVisible(); for (const control of FOOTER_CONTROLS) await expect(page.locator(control)).toBeEnabled(); - await expect(page.locator("#batch-execute")).not.toHaveAttribute("aria-busy", "true"); + await expect(page.locator("#batch-execute-top")).not.toHaveAttribute("aria-busy", "true"); await expect(page.locator("#batch-busy")).toBeHidden(); // Focus returns to the trigger, which sits next to the inline error (#676). - await expect(page.locator("#batch-execute")).toBeFocused(); + await expect(page.locator("#batch-execute-top")).toBeFocused(); + + // Cancel wipes the attempt completely: dropping the next file must not + // resurface the abandoned attempt's error (#731). + await page.locator("#batch-cancel-top").click(); + await expect(page.locator("#batch-upload-error")).toBeHidden(); + await page.locator("#batch-file").setInputFiles(bundleFile("batch")); + await expect(page.locator("#batch-preflight")).toBeVisible(); + await expect(page.locator("#batch-execute-error")).toBeHidden(); +}); + +test("revealing a tall preflight neither scrolls the page nor paints a focus ring", async ({ + page, +}) => { + await page.goto("/ui/batch", { waitUntil: "networkidle" }); + // Enough entries that the rendered plan is taller than the viewport — + // the size precondition of the focus() scroll (#732). + const entries = Array.from({ length: 60 }, (_, i) => ({ + request: { method: "POST", url: "Patient" }, + resource: { resourceType: "Patient", name: [{ family: `Tall${i}` }] }, + })); + const file = writeRawFile( + JSON.stringify({ resourceType: "Bundle", type: "batch", entry: entries }), + "tall", + ); + await page.locator("#batch-file").setInputFiles(file); + await expect(page.locator("#batch-preflight")).toBeVisible(); + await expect(page.locator("#batch-rows .batch-row")).toHaveCount(60); + expect(await page.evaluate(() => window.scrollY)).toBe(0); + // The stage holds programmatic focus without the browser's default ring + // tracing its outline (#726). + await expect(page.locator("#batch-preflight")).toBeFocused(); + expect( + await page.locator("#batch-preflight").evaluate((el) => getComputedStyle(el).outlineStyle), + ).toBe("none"); }); test("reduced-motion users get a static ring, not an animated one", async ({ page }) => { @@ -474,12 +511,12 @@ test("reduced-motion users get a static ring, not an animated one", async ({ pag await page.goto("/ui/batch", { waitUntil: "networkidle" }); await page.locator("#batch-file").setInputFiles(bundleFile("batch")); - await page.locator("#batch-execute").click(); - await expect(page.locator("#batch-execute")).toHaveAttribute("aria-busy", "true"); + await page.locator("#batch-execute-top").click(); + await expect(page.locator("#batch-execute-top")).toHaveAttribute("aria-busy", "true"); // The static form: the ring glyph is present but does not animate. A // label-only dimmed button would read as "disabled", not "working". - const after = await page.locator("#batch-execute").evaluate((button) => ({ + const after = await page.locator("#batch-execute-top").evaluate((button) => ({ content: getComputedStyle(button, "::after").content, animation: getComputedStyle(button, "::after").animationName, })); @@ -512,7 +549,7 @@ test("a synchronously-failing operation cannot leave a stale region label", asyn // region keeps the stale label and announces it on its next reveal. const state = await page.evaluate(() => { const region = document.getElementById("batch-busy") as HTMLElement; - const button = document.getElementById("batch-execute") as HTMLButtonElement; + const button = document.getElementById("batch-execute-top") as HTMLButtonElement; const busyApi = (window as { hfsBusy?: { during: Function } }).hfsBusy!; busyApi.during( [button], diff --git a/crates/ui/templates/pages/batch.html b/crates/ui/templates/pages/batch.html index d5688d7c1..01d04cf37 100644 --- a/crates/ui/templates/pages/batch.html +++ b/crates/ui/templates/pages/batch.html @@ -67,25 +67,22 @@

{% include "icons/export.svg" %} {{ i18n.t("batch- server will treat failures — all-or-nothing vs. entry-by-entry. -->

+ + +

{% include "icons/layers.svg" %} {{ i18n.t("batch-plan-heading") }}

- - + +
    - -
    - - - -
    @@ -93,10 +90,13 @@

    {% include "icons/layers.svg" %} {{ i18n.t("batch-

    {% include "icons/check.svg" %} {{ i18n.t("batch-response-heading") }}

    - + +
    + + +
    -