mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 10:26:26 +01:00
fix(ui): pass AA contrast on the recovery disclosure and stop mobile row overflow
Two defects from the the design review design review of the recessed recovery
treatment (the accessibility follow-up).
Contrast: the "Details" / "Hide details" hint used
`text-muted-foreground/70`, which measures 2.71:1 in light and 4.20:1 in
dark against the surfaces this line renders on — a serious axe
`color-contrast` violation at 12px. The hint now inherits the button's
undiluted `text-muted-foreground` (4.73:1 light, 7.63:1 dark, and 4.53:1
against the `hover:bg-muted/50` wash) and carries its subordinate rank by
weight instead of by contrast, so the quiet-vs-warning hierarchy is
unchanged.
Overflow: at 390px the row painted the recovery chip twice — once in the
`sm:hidden` leading cluster and again beside the identifier, where the
copy had no desktop gate. Because badges are `whitespace-nowrap
shrink-0` inside a cluster with no `min-w-0`, that duplicate is what
pushed the document from 390px to 403px and carried the chip past the row
boundary. The identifier-side copies are now gated `hidden
sm:inline-flex`, with a `max-w-(--sz-12rem)` ceiling and a truncating
label as backstops against a future longer label.
Verified in Chromium against the built storybook, with negative controls
that re-introduce each old treatment in-page to prove the harness is
sensitive to what it clears: 19/19 checks — 390px light/dark documents
stay at 390px (control reproduces the overflow), axe reports zero
contrast violations on the disclosure in both themes (control is flagged
serious at 2.71/4.17, matching the computed ratios), desktop labels and
Enter/Space disclosure behaviour unchanged.
Also pins each `InboxRowChips` story row to the state it demonstrates:
post-liveness the fixture's live `executionRunId` had collapsed several
rows into `observe_only`, leaving no `in_progress` row and hiding the
widest label ("Workspace recovery needed") the row must survive.
pnpm check:token-gates clean; 134 focused Vitest tests pass.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
a88bde1def
commit
d30902f38d
5 files changed
+136
-6
No files matched your search
@@ -597,6 +597,66 @@ describe("IssueRow", () => {
|
||||
),
|
||||
).toEqual({ state: "in_progress", text: "Recovery in progress" });
|
||||
});
|
||||
|
||||
// PAP-17070: the chip was painted twice on mobile — once in the `sm:hidden`
|
||||
// leading cluster and again beside the identifier, whose copy had no desktop
|
||||
// gate. Because badges are `whitespace-nowrap shrink-0`, that duplicate
|
||||
// pushed a 390px document out to 403px. Exactly one copy may be visible at
|
||||
// any one breakpoint.
|
||||
it.each([
|
||||
["needed", {}],
|
||||
[
|
||||
"in_progress",
|
||||
{ status: "in_progress", executionRunId: "run-1", assigneeAgentId: "cto-agent" },
|
||||
],
|
||||
[
|
||||
"observe_only",
|
||||
{ status: "in_progress", executionRunId: "run-1", assigneeAgentId: "coder-agent" },
|
||||
],
|
||||
["escalated", { activeRecoveryAction: { ...recoveryAction, status: "escalated" } }],
|
||||
] as const)("renders one %s chip per breakpoint, never two at once", (state, overrides) => {
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<IssueRow
|
||||
issue={createIssue({
|
||||
activeRecoveryAction: recoveryAction,
|
||||
...(overrides as Partial<Issue>),
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
});
|
||||
|
||||
const chips = Array.from(
|
||||
container.querySelectorAll('[data-testid="issue-row-recovery-indicator"]'),
|
||||
);
|
||||
expect(chips.map((chip) => chip.getAttribute("data-recovery-state"))).toEqual([state, state]);
|
||||
|
||||
// Classify each copy by the breakpoint gate on its ancestors.
|
||||
const visibility = chips.map((chip) => {
|
||||
let node = chip.parentElement;
|
||||
while (node && node !== container) {
|
||||
const classes = node.className;
|
||||
if (typeof classes === "string") {
|
||||
if (classes.includes("sm:hidden")) return "mobile";
|
||||
if (classes.includes("hidden") && classes.includes("sm:inline-flex")) return "desktop";
|
||||
}
|
||||
node = node.parentElement;
|
||||
}
|
||||
return "always";
|
||||
});
|
||||
expect(visibility.sort()).toEqual(["desktop", "mobile"]);
|
||||
|
||||
// A hard width ceiling is the backstop for a long label in either slot.
|
||||
chips.forEach((chip) => {
|
||||
expect(chip.className).toContain("max-w-(--sz-12rem)");
|
||||
expect(chip.querySelector("span")?.className).toContain("truncate");
|
||||
});
|
||||
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the hover wash on the row root while the overlay link stays a bare positioning layer", () => {
|
||||
|
||||
@@ -279,8 +279,18 @@ export function IssueRow({
|
||||
<span className="shrink-0 font-mono text-xs text-muted-foreground">
|
||||
{identifier}
|
||||
</span>
|
||||
{parkedBlockerIndicator}
|
||||
{recoveryIndicator}
|
||||
{/* Desktop-only copies. The mobile row already renders both chips
|
||||
in its `sm:hidden` leading cluster above, so leaving these
|
||||
ungated painted a SECOND nowrap chip beside the identifier at
|
||||
390px — and because badges are `whitespace-nowrap shrink-0`
|
||||
inside a cluster with no `min-w-0`, that duplicate pushed the
|
||||
document from 390px to 403px (PAP-17070). */}
|
||||
{parkedBlockerIndicator || recoveryIndicator ? (
|
||||
<span className="hidden min-w-0 shrink-0 items-center sm:inline-flex">
|
||||
{parkedBlockerIndicator}
|
||||
{recoveryIndicator}
|
||||
</span>
|
||||
) : null}
|
||||
</>
|
||||
)}
|
||||
{mobileMeta ? (
|
||||
@@ -357,14 +367,18 @@ function renderRecoveryChip(
|
||||
role="status"
|
||||
aria-label={label}
|
||||
className={cn(
|
||||
"ml-1.5 gap-0.5 text-(length:--text-nano)",
|
||||
// `max-w` caps the chip even though badges are `shrink-0`: it is the only
|
||||
// thing standing between a long recovery label and a row wider than a
|
||||
// 390px viewport. Every current label fits well inside 12rem, so this is
|
||||
// a ceiling, not a truncation the operator will normally see.
|
||||
"ml-1.5 max-w-(--sz-12rem) gap-0.5 text-(length:--text-nano)",
|
||||
tone.className,
|
||||
selected ? "!border-muted-foreground !text-muted-foreground" : null,
|
||||
)}
|
||||
title={`${label} — open the source task to act.`}
|
||||
>
|
||||
<Icon className="h-2.5 w-2.5" aria-hidden />
|
||||
{label}
|
||||
<span className="min-w-0 truncate">{label}</span>
|
||||
</Badge>
|
||||
);
|
||||
}
|
||||
@@ -212,8 +212,37 @@ describe("RecoveryProgressLine", () => {
|
||||
const label = toggle?.querySelector("span");
|
||||
expect(label?.className).toContain("truncate");
|
||||
// The "Details" hint is desktop-only so the owner name keeps the mobile row.
|
||||
const hint = toggle?.querySelectorAll("span")[1];
|
||||
const hint = host.querySelector('[data-testid="recovery-progress-line-hint"]');
|
||||
expect(hint?.className).toContain("hidden");
|
||||
expect(hint?.className).toContain("sm:inline");
|
||||
});
|
||||
|
||||
// PAP-17070: `text-muted-foreground/70` measured 2.71:1 (light) and 4.20:1
|
||||
// (dark) against the surfaces this line renders on — a serious axe
|
||||
// `color-contrast` violation at 12px. The undiluted token measures 4.73:1 and
|
||||
// 7.63:1, and 4.53:1 against the `hover:bg-muted/50` wash, so the hint must
|
||||
// inherit the button's colour rather than fade it further.
|
||||
it("does not dilute the disclosure hint below WCAG AA contrast", () => {
|
||||
for (const open of [false, true]) {
|
||||
const host = render(
|
||||
<RecoveryProgressLine
|
||||
state="in_progress"
|
||||
defaultOpen={open}
|
||||
action={buildAction()}
|
||||
agentMap={agentMap}
|
||||
/>,
|
||||
);
|
||||
const hint = host.querySelector('[data-testid="recovery-progress-line-hint"]');
|
||||
expect(hint?.textContent).toContain(open ? "Hide details" : "Details");
|
||||
// No opacity-diluted foreground token anywhere on the hint...
|
||||
expect(hint?.className).not.toMatch(/text-(muted-)?foreground\/\d+/);
|
||||
// ...and no diluted token on the button it inherits from either.
|
||||
const toggle = host.querySelector('[data-testid="recovery-progress-line-toggle"]');
|
||||
expect(toggle?.className).toContain("text-muted-foreground");
|
||||
expect(toggle?.className).not.toMatch(/text-(muted-)?foreground\/\d+/);
|
||||
// Hierarchy is carried by weight: the label is medium, the hint is not.
|
||||
expect(host.querySelector(".truncate")?.className).toContain("font-medium");
|
||||
expect(hint?.className).toContain("font-normal");
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -67,7 +67,11 @@ export function RecoveryProgressLine({
|
||||
>
|
||||
<Icon className="h-3.5 w-3.5 shrink-0" aria-hidden />
|
||||
<span className="truncate font-medium">{label}</span>
|
||||
<span className="hidden shrink-0 text-muted-foreground/70 sm:inline">
|
||||
{/* Inherits the button's `text-muted-foreground`; the hint must NOT be
|
||||
diluted further (`text-muted-foreground/70` measured 2.71:1 light /
|
||||
4.20:1 dark and failed WCAG AA — PAP-17070). Weight, not contrast,
|
||||
is what keeps this subordinate to the label. */}
|
||||
<span data-testid="recovery-progress-line-hint" className="hidden shrink-0 font-normal sm:inline">
|
||||
· {open ? "Hide details" : "Details"}
|
||||
</span>
|
||||
<ChevronDown
|
||||
|
||||
@@ -364,6 +364,10 @@ function ActiveRunPanel() {
|
||||
|
||||
function InboxRowPanel() {
|
||||
const baseIssue = createIssue();
|
||||
// Liveness now decides the chip, so each row pins the signals for the state it
|
||||
// is meant to demonstrate: an ownerless task with no live run reads `needed`,
|
||||
// the recovery owner holding the live run reads `in_progress`, and a live run
|
||||
// owned by somebody else folds down to `observe_only`.
|
||||
return (
|
||||
<div className="rounded-lg border border-border/70 bg-background/80">
|
||||
<IssueRow
|
||||
@@ -372,6 +376,20 @@ function InboxRowPanel() {
|
||||
identifier: "PAP-9065",
|
||||
title: "Add full company search page",
|
||||
status: "in_progress",
|
||||
executionRunId: null,
|
||||
scheduledRetry: null,
|
||||
activeRecoveryAction: buildAction(),
|
||||
}}
|
||||
/>
|
||||
<IssueRow
|
||||
issue={{
|
||||
...baseIssue,
|
||||
id: "issue-recovery-in-progress",
|
||||
identifier: "PAP-9101",
|
||||
title: "Owner is live on the source task — recessed in-progress chip",
|
||||
status: "in_progress",
|
||||
assigneeAgentId: claudeAgent.id,
|
||||
executionRunId: "run-recovery-owner-live",
|
||||
activeRecoveryAction: buildAction(),
|
||||
}}
|
||||
/>
|
||||
@@ -402,6 +420,11 @@ function InboxRowPanel() {
|
||||
identifier: "PAP-10409",
|
||||
title: "Phase 6 follow-up failed: git workspace lost",
|
||||
status: "blocked",
|
||||
// No live run, so this keeps the `needed` state and its longer
|
||||
// "Workspace recovery needed" label — the widest chip the row has to
|
||||
// survive at 390px.
|
||||
executionRunId: null,
|
||||
scheduledRetry: null,
|
||||
activeRecoveryAction: buildAction({
|
||||
kind: "workspace_validation",
|
||||
cause: "workspace_validation_failed",
|
||||
|
||||
Reference in new issue
Block a user