diff --git a/ui/src/components/IssueRow.test.tsx b/ui/src/components/IssueRow.test.tsx index 1817037a7b..8b60584a19 100644 --- a/ui/src/components/IssueRow.test.tsx +++ b/ui/src/components/IssueRow.test.tsx @@ -161,6 +161,53 @@ describe("IssueRow", () => { }); }); + it("reserves the leading dot slot on read rows so unread rows never indent past them", () => { + const root = createRoot(container); + act(() => { + // A read inbox row still supplies `unreadState` (as "hidden"). + root.render(); + }); + + // The desktop dot slot is reserved even when read (empty), so unread rows + // add no column and line up with read rows. + const slot = container.querySelector('[data-testid="issue-row-unread-slot"]'); + expect(slot).not.toBeNull(); + expect(slot?.className).toContain("w-4"); + expect(slot?.className).toContain("sm:inline-flex"); + // In flow, not an absolute overlay. + expect(slot?.className).not.toContain("absolute"); + // Read rows carry no dot button in the slot. + expect(slot?.querySelector('button[aria-label="Mark as read"]')).toBeNull(); + + act(() => { + root.unmount(); + }); + }); + + it("puts the unread dot in the reserved far-left slot on desktop and in flow on mobile", () => { + const root = createRoot(container); + act(() => { + root.render(); + }); + + // Desktop: the dot lives in the reserved leading slot (far left, ahead of + // any leading control such as a parent's collapse caret). + const slot = container.querySelector('[data-testid="issue-row-unread-slot"]'); + expect(slot).not.toBeNull(); + expect(slot?.querySelector('button[aria-label="Mark as read"]')).not.toBeNull(); + + // Mobile: a separate in-flow, order-first dot (mobile has no reserved slot). + const mobileDot = container + .querySelector('button[aria-label="Mark as read"].sm\\:hidden, span.sm\\:hidden button[aria-label="Mark as read"]') + ?.closest("span.sm\\:hidden"); + expect(mobileDot).not.toBeNull(); + expect(mobileDot?.className).toContain("order-first"); + + act(() => { + root.unmount(); + }); + }); + it("preserves the issue detail breadcrumb source and href in the link target", () => { const root = createRoot(container); const issue = createIssue(); diff --git a/ui/src/components/IssueRow.tsx b/ui/src/components/IssueRow.tsx index dfb7706f82..97dd912d4d 100644 --- a/ui/src/components/IssueRow.tsx +++ b/ui/src/components/IssueRow.tsx @@ -90,7 +90,43 @@ export function IssueRow({ }: IssueRowProps) { const issuePathId = issue.identifier ?? issue.id; const identifier = issue.identifier ?? issue.id.slice(0, 8); + // A row participates in the unread system whenever `unreadState` is supplied + // (inbox rows). It then reserves a fixed leading dot slot on all rows — read + // and unread alike — so the mark-read dot sits in the far-left gutter without + // shifting content, matching the sibling non-issue inbox rows. + const showUnreadSlot = unreadState != null; const showUnreadDot = unreadState === "visible" || unreadState === "fading"; + const unreadDotButton = ( + + ); const selectedStatusClass = selected ? "!text-muted-foreground !border-muted-foreground" : undefined; const detailState = withIssueDetailHeaderSeed(issueLinkState, issue); const productivityReview = issue.productivityReview ?? null; @@ -140,7 +176,7 @@ export function IssueRow({ // No color transition on the row band: hover/selection must snap // instantly. A fade (transition-colors) leaves a trail of fading bands // when scrubbing the mouse fast across the list. - "group 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", + "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", !hideDivider && "border-b border-border last:border-b-0", selected ? "hover:bg-transparent" : "hover:bg-accent/50", checklistCurrentStep ? "bg-primary/5" : null, @@ -163,6 +199,19 @@ export function IssueRow({ ) : null} + {showUnreadSlot ? ( + // Reserved leftmost dot gutter (desktop). Present on read and unread + // rows so the mark-read dot lives to the LEFT of any leading control + // (a parent's collapse caret, a tree guide) without indenting the row + // relative to its siblings, and aligns with the non-issue inbox rows + // that reserve the same w-4 slot. + + {showUnreadDot ? unreadDotButton : null} + + ) : null} {treeGuides > 0 ? Array.from({ length: treeGuides }, (_, level) => { // The innermost guide lands on THIS row's own chevron column; if @@ -260,39 +309,11 @@ export function IssueRow({ ) : null} {showUnreadDot ? ( - // Only unread rows reserve this leading mark-read column; read rows - // omit it entirely so their content lines up with the tasks list - // (which has no such column). Archive lives on the right now. - - + // Mobile keeps the dot in flow as the leading item (mobile has no + // reserved desktop dot gutter). Desktop renders the dot in the reserved + // leading slot above instead, so this is mobile-only. + + {unreadDotButton} ) : null} diff --git a/ui/src/pages/Inbox.test.tsx b/ui/src/pages/Inbox.test.tsx index feef05e839..eb0d3f3ea1 100644 --- a/ui/src/pages/Inbox.test.tsx +++ b/ui/src/pages/Inbox.test.tsx @@ -439,7 +439,7 @@ describe("Inbox toolbar", () => { }); }); - it("does not double-indent unread rows: the mark-read dot replaces the leading spacer", async () => { + it("does not indent unread rows: the mark-read dot sits in a reserved leading slot present on every row", async () => { routerMock.location.pathname = "/inbox/mine"; // Two sibling leaf rows, one unread and one read, so their leading columns // are directly comparable. @@ -477,23 +477,36 @@ describe("Inbox toolbar", () => { const rows = Array.from(container.querySelectorAll("[data-inbox-item]")); const rowFor = (text: string) => rows.find((row) => row.textContent?.includes(text)); const linkOf = (row: Element) => row.querySelector("a[data-inbox-issue-link]"); - const hasMarkReadDot = (row: Element) => !!row.querySelector('button[aria-label="Mark as read"]'); - // The empty spacer that reserves the chevron column on read rows. Excludes - // the tree-guide span (`.self-stretch`), which only renders on nested rows. + const markReadButton = (row: Element) => row.querySelector('button[aria-label="Mark as read"]'); + // The empty spacer that reserves the chevron column on every leaf row. + // Excludes the tree-guide span (`.self-stretch`), which only renders on + // nested rows. const hasLeadingSpacer = (row: Element) => !!linkOf(row)?.querySelector("span.hidden.w-4.shrink-0.sm\\:block:not(.self-stretch)"); + // The reserved leading dot slot, present on read AND unread rows. + const dotSlot = (row: Element) => + linkOf(row)?.querySelector('[data-testid="issue-row-unread-slot"]') ?? null; const unreadRow = rowFor("Unread inbox row")!; const readRow = rowFor("Read inbox row")!; - // Unread rows carry the mark-read dot in the chevron column; rendering the - // spacer too would push the status icon + title one column further right - // than read rows (the bug this fix addresses). - expect(hasMarkReadDot(unreadRow)).toBe(true); - expect(hasLeadingSpacer(unreadRow)).toBe(false); + // The dot lives in a fixed leading slot that is reserved on every inbox row + // (in flow, NOT an absolute overlay). Because read and unread rows both + // reserve it — and both keep the chevron spacer — their status icon + title + // land at the same x (the bug this fix addresses: an unread-only dot column + // used to push unread rows right). + const unreadSlot = dotSlot(unreadRow); + const readSlot = dotSlot(readRow); + expect(unreadSlot).not.toBeNull(); + expect(readSlot).not.toBeNull(); + // In flow, not an absolute overlay. + expect(unreadSlot?.className).not.toContain("absolute"); + // Only the unread row carries the dot button; the read slot is empty. + expect(markReadButton(unreadSlot!)).not.toBeNull(); + expect(readSlot?.querySelector('button[aria-label="Mark as read"]')).toBeNull(); + expect(hasLeadingSpacer(unreadRow)).toBe(true); - // Read rows have no dot, so they keep the spacer to hold that same column. - expect(hasMarkReadDot(readRow)).toBe(false); + // Read rows keep the same spacer, so both rows line up. expect(hasLeadingSpacer(readRow)).toBe(true); act(() => { diff --git a/ui/src/pages/Inbox.tsx b/ui/src/pages/Inbox.tsx index 16355d1f5e..ddd30587bc 100644 --- a/ui/src/pages/Inbox.tsx +++ b/ui/src/pages/Inbox.tsx @@ -2582,13 +2582,12 @@ export function Inbox() { > - ) : (isUnread || isFading) ? ( - // Unread rows already carry the leading mark-read - // dot (IssueRow, order-first) in the chevron - // column, so skip the spacer — otherwise the dot - // and this spacer would double-indent the status. - null ) : ( + // Every non-chevron row reserves this spacer so the + // status column lines up under the parent rows' + // collapse chevron. (The unread mark-read dot has + // its own reserved leading slot in IssueRow, to the + // left of this spacer.) ) ) : null}