diff --git a/ui/src/components/BlockedInboxView.tsx b/ui/src/components/BlockedInboxView.tsx index 871f537324..1880fe2e0c 100644 --- a/ui/src/components/BlockedInboxView.tsx +++ b/ui/src/components/BlockedInboxView.tsx @@ -359,6 +359,7 @@ function BlockedInboxRow({ { root.render(); }); + // The hover wash lives on the ROOT row band (not the overlay link) so the + // tint paints behind the content. Selected rows suppress the accent hover. + const row = container.firstElementChild as HTMLElement | null; const link = container.querySelector("[data-inbox-issue-link]") as HTMLAnchorElement | null; - expect(link).not.toBeNull(); - expect(link?.className).toContain("hover:bg-transparent"); - expect(link?.className).not.toContain("hover:bg-accent/50"); + expect(row).not.toBeNull(); + expect(row?.className).toContain("hover:bg-transparent"); + expect(row?.className).not.toContain("hover:bg-accent/50"); + // The overlay link no longer carries the hover wash. + expect(link?.className ?? "").not.toContain("hover:bg-transparent"); + expect(link?.className ?? "").not.toContain("hover:bg-accent/50"); act(() => { root.unmount(); @@ -348,12 +354,17 @@ describe("IssueRow", () => { ); }); + // `aria-current="step"` stays on the overlay link (the focusable target), + // but the current-step wash moved to the ROOT row band alongside the hover + // wash so it paints behind the content. + const row = container.firstElementChild as HTMLElement | null; const link = container.querySelector("[data-inbox-issue-link]") as HTMLAnchorElement | null; expect(link).not.toBeNull(); expect(link?.getAttribute("aria-current")).toBe("step"); - expect(link?.className).toContain("bg-primary/5"); - expect(link?.className).not.toContain("border-l-"); + expect(row?.className).toContain("bg-primary/5"); + expect(row?.className).not.toContain("border-l-"); + expect(link?.className ?? "").not.toContain("bg-primary/5"); act(() => { root.unmount(); @@ -446,4 +457,65 @@ describe("IssueRow", () => { root.unmount(); }); }); + + it("renders no bottom divider by default", () => { + const root = createRoot(container); + + act(() => { + root.render(); + }); + + // Dividers are opt-in: without `showDivider` no row-separating border + // renders on either the root band or the overlay link. + const row = container.firstElementChild as HTMLElement | null; + const link = container.querySelector("[data-inbox-issue-link]") as HTMLAnchorElement | null; + expect(row).not.toBeNull(); + expect(row?.className).not.toContain("border-b"); + expect(link?.className ?? "").not.toContain("border-b"); + + act(() => { + root.unmount(); + }); + }); + + it("renders an opt-in bottom divider on the row root when showDivider is set", () => { + const root = createRoot(container); + + act(() => { + root.render(); + }); + + // The divider lives on the ROOT row band with `last:border-b-0` so the real + // last row drops its border — it is not on the overlay link. + const row = container.firstElementChild as HTMLElement | null; + const link = container.querySelector("[data-inbox-issue-link]") as HTMLAnchorElement | null; + expect(row?.className).toContain("border-b"); + expect(row?.className).toContain("last:border-b-0"); + expect(link?.className ?? "").not.toContain("border-b"); + + act(() => { + root.unmount(); + }); + }); + + it("keeps the hover wash on the row root while the overlay link stays a bare positioning layer", () => { + const root = createRoot(container); + + act(() => { + root.render(); + }); + + const row = container.firstElementChild as HTMLElement | null; + const link = container.querySelector("[data-inbox-issue-link]") as HTMLAnchorElement | null; + // Hover wash paints behind the content on the root band... + expect(row?.className).toContain("hover:bg-accent/50"); + expect(link?.className ?? "").not.toContain("hover:bg-accent/50"); + // ...and the overlay link keeps only positioning + focus concerns. + expect(link?.className).toContain("absolute"); + expect(link?.className).toContain("inset-0"); + + act(() => { + root.unmount(); + }); + }); }); diff --git a/ui/src/components/IssueRow.tsx b/ui/src/components/IssueRow.tsx index edf45747d5..a175417c43 100644 --- a/ui/src/components/IssueRow.tsx +++ b/ui/src/components/IssueRow.tsx @@ -57,8 +57,8 @@ interface IssueRowProps { * not crossed out by it. */ chevronInGuide?: boolean; - /** Suppress the row divider (parents with expanded children keep visual attachment to their subtree). */ - hideDivider?: boolean; + /** Opt in to a bottom divider on this row (default off; used by views that intentionally keep separators). */ + showDivider?: boolean; } export function IssueRow({ @@ -86,7 +86,7 @@ export function IssueRow({ onMouseEnter, treeGuides = 0, chevronInGuide = false, - hideDivider = false, + showDivider = false, }: IssueRowProps) { const issuePathId = issue.identifier ?? issue.id; const identifier = issue.identifier ?? issue.id.slice(0, 8); @@ -170,6 +170,13 @@ export function IssueRow({ // when scrubbing the mouse fast across the list. "group relative flex items-start gap-2 rounded-lg py-2.5 pl-2 pr-3 text-sm no-underline text-inherit sm:items-center sm:py-2 sm:pl-1", "[&_button]:relative [&_button]:z-10", + // Divider + hover/selected/checklist wash live on the ROOT row band so + // the tint paints BEHIND the content and `last:border-b-0` matches the + // real last row. Keeping these on the overlay Link (PR #10526) made the + // last row keep its border and the hover wash paint over the text. + showDivider && "border-b border-border last:border-b-0", + selected ? "hover:bg-transparent" : "hover:bg-accent/50", + checklistCurrentStep ? "bg-primary/5" : null, className, )} > @@ -183,11 +190,9 @@ export function IssueRow({ aria-current={checklistCurrentStep ? "step" : undefined} onClickCapture={() => rememberIssueDetailLocationState(issuePathId, detailState)} className={cn( + // Overlay Link keeps ONLY positioning + focus ring so header controls + // stay clickable above it; visual washes belong on the root above. "absolute inset-0 rounded-lg no-underline text-inherit focus-visible:z-10 focus-visible:outline-none focus-visible:ring-(length:--rad-3) focus-visible:ring-ring", - !hideDivider && "border-b border-border last:border-b-0", - selected ? "hover:bg-transparent" : "hover:bg-accent/50", - checklistCurrentStep ? "bg-primary/5" : null, - className, )} > Open {identifier}: {issue.title} diff --git a/ui/src/components/IssuesList.tsx b/ui/src/components/IssuesList.tsx index bdc780966a..2aeb017322 100644 --- a/ui/src/components/IssuesList.tsx +++ b/ui/src/components/IssuesList.tsx @@ -2097,7 +2097,6 @@ export function IssuesList({ onMouseEnter={() => setNavSelectionFromPointer(`issue:${issue.id}`)} treeGuides={depth} chevronInGuide={depth > 0 && hasChildren} - hideDivider={hasChildren && isExpanded} checklistStepNumber={checklistStepNumber} checklistCurrentStep={checklistMeta?.currentStepIssueId === issue.id} checklistDependencyChips={checklistDependencyChips} diff --git a/ui/src/pages/Inbox.test.tsx b/ui/src/pages/Inbox.test.tsx index 69518d147f..49b9bdaa7c 100644 --- a/ui/src/pages/Inbox.test.tsx +++ b/ui/src/pages/Inbox.test.tsx @@ -465,12 +465,14 @@ describe("Inbox toolbar", () => { const rows = container.querySelectorAll("[data-inbox-item]"); - const linkOf = (row: Element): HTMLAnchorElement | null => - row.querySelector("a[data-inbox-issue-link]"); + // The hover wash lives on the IssueRow root band (the overlay link's + // parent), not the overlay link itself. + const bandOf = (row: Element): HTMLElement | null => + row.querySelector("a[data-inbox-issue-link]")?.parentElement ?? null; // Nothing selected before hover — both rows show the hover-accent class. - expect(linkOf(rows[0]!)?.className).toContain("hover:bg-accent/50"); - expect(linkOf(rows[1]!)?.className).toContain("hover:bg-accent/50"); + expect(bandOf(rows[0]!)?.className).toContain("hover:bg-accent/50"); + expect(bandOf(rows[1]!)?.className).toContain("hover:bg-accent/50"); // Hovering paints via CSS `:hover` only — it must NOT flip a row into the // state-selected band (which would swap to hover:bg-transparent). Coupling @@ -482,9 +484,9 @@ describe("Inbox toolbar", () => { rows[1]!.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })); rows[1]!.dispatchEvent(new MouseEvent("mouseenter", { bubbles: false })); }); - expect(linkOf(rows[0]!)?.className).toContain("hover:bg-accent/50"); - expect(linkOf(rows[1]!)?.className).toContain("hover:bg-accent/50"); - expect(linkOf(rows[1]!)?.className).not.toContain("hover:bg-transparent"); + expect(bandOf(rows[0]!)?.className).toContain("hover:bg-accent/50"); + expect(bandOf(rows[1]!)?.className).toContain("hover:bg-accent/50"); + expect(bandOf(rows[1]!)?.className).not.toContain("hover:bg-transparent"); act(() => { root.unmount(); @@ -578,12 +580,13 @@ describe("Inbox toolbar", () => { }); const root = createRoot(container); - const linkOf = (row: Element): HTMLAnchorElement | null => - row.querySelector("a[data-inbox-issue-link]"); - // The keyboard-selected row swaps to `hover:bg-transparent`; find its index. + // The keyboard-selected row swaps to `hover:bg-transparent` on its root + // band (the overlay link's parent, where the wash now lives); find its index. + const bandOf = (row: Element): HTMLElement | null => + row.querySelector("a[data-inbox-issue-link]")?.parentElement ?? null; const selectedRowIndex = () => [...container.querySelectorAll("[data-inbox-item]")].findIndex((row) => - linkOf(row)?.className.includes("hover:bg-transparent"), + bandOf(row)?.className.includes("hover:bg-transparent"), ); try { diff --git a/ui/src/pages/Inbox.tsx b/ui/src/pages/Inbox.tsx index 2cfa3569c3..c86873fe70 100644 --- a/ui/src/pages/Inbox.tsx +++ b/ui/src/pages/Inbox.tsx @@ -2689,7 +2689,6 @@ export function Inbox() { issue={issue} issueLinkState={issueLinkState} treeGuides={depth} - hideDivider={hasChildren && isExpanded} selected={selected} className={ isArchiving