Skip to content

fix(ui): Batch/Transaction round 3 — single footer, error placement, focus, header count, tab icons #736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
smunini merged 1 commit into from
Aug 27, 2026
+108 −39
Merged

fix(ui): Batch/Transaction round 3 — single footer, error placement, focus, header count, tab icons #736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions crates/ui/assets/app.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
24 changes: 16 additions & 8 deletions crates/ui/assets/batch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -314,7 +320,6 @@
drop.focus();
}
}
cancelBtn.addEventListener("click", reset);
cancelTopBtn.addEventListener("click", reset);

/* ---- stage 3: execute and report ------------------------------------ */
Expand All @@ -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",
Expand All @@ -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). */
Expand Down Expand Up @@ -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
Expand Down
75 changes: 56 additions & 19 deletions crates/ui/e2e/tests/batch.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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" });
Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand All @@ -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 })));
Expand Down Expand Up @@ -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();

Expand All @@ -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 }) => {
Expand All @@ -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,
}));
Expand Down Expand Up @@ -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],
Expand Down
24 changes: 12 additions & 12 deletions crates/ui/templates/pages/batch.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,36 +67,36 @@ <h3><span class="icon">{% include "icons/export.svg" %}</span> {{ i18n.t("batch-
server will treat failures — all-or-nothing vs. entry-by-entry. -->
<p class="notice" id="batch-semantics"></p>

<!-- A failed execute reports here, above the plan (#730) — on a long
plan the page may be scrolled anywhere when the attempt fails. -->
<p class="alert" id="batch-execute-error" role="alert" hidden></p>

<div class="card">
<div class="card-head">
<h3><span class="icon">{% include "icons/layers.svg" %}</span> {{ i18n.t("batch-plan-heading") }}</h3>
</div>
<div class="batch-tabs" role="tablist">
<button type="button" class="batch-tab" id="batch-tab-actions" role="tab" aria-selected="true">{{ i18n.t("batch-tab-actions") }}</button>
<button type="button" class="batch-tab" id="batch-tab-json" role="tab" aria-selected="false">{{ i18n.t("batch-tab-json") }}</button>
<button type="button" class="batch-tab" id="batch-tab-actions" role="tab" aria-selected="true"><span class="icon">{% include "icons/sliders.svg" %}</span> {{ i18n.t("batch-tab-actions") }}</button>
<button type="button" class="batch-tab" id="batch-tab-json" role="tab" aria-selected="false"><span class="icon">{% include "icons/code.svg" %}</span> {{ i18n.t("batch-tab-json") }}</button>
</div>
<ol class="batch-rows" id="batch-rows"></ol>
<div class="batch-json" id="batch-json" hidden></div>
</div>

<!-- The error renders inside the footer, next to the button that raised
it (#676) — not as detached text at the end of the page. -->
<div class="batch-footer">
<button type="button" class="btn" id="batch-cancel">{{ i18n.t("batch-cancel") }}</button>
<p class="alert alert--inline" id="batch-execute-error" role="alert" hidden></p>
<button type="button" class="btn btn--primary" id="batch-execute">{{ i18n.t("batch-execute") }}</button>
</div>
</section>

<!-- ============ Stage 3: response ============ -->
<section id="batch-response" hidden>
<div class="card">
<div class="card-head">
<h3><span class="icon">{% include "icons/check.svg" %}</span> {{ i18n.t("batch-response-heading") }}</h3>
<span class="batch-badge" id="batch-overall"></span>
<!-- The created count reads at a glance next to the status (#729);
the rest of the tally stays in the summary line below. -->
<div class="card-head__meta">
<span class="batch-created" id="batch-created"></span>
<span class="batch-badge" id="batch-overall"></span>
</div>
</div>
<!-- The aggregate Steve asked for: how many resources this run created,
updated, or failed — not just per-row badges. -->
<p class="batch-summary" id="batch-summary"></p>
<ol class="batch-rows" id="batch-outcomes"></ol>
</div>
Expand Down
Loading