Skip to content

fix: move worktree badges below names - #233

Merged
centdix merged 1 commit into
mainfrom
worktree-badges-below-name
May 13, 2026
Merged

fix: move worktree badges below names#233
centdix merged 1 commit into
mainfrom
worktree-badges-below-name

Conversation

@centdix

@centdix centdix commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Move archived, closed/opening/creating, PR, and Linear badges onto a separate row in the worktree sidebar so long worktree names keep their available width.

Changes

  • Split the worktree list item header into a name row and a wrapping badge row.
  • Keep the agent status and notification indicator beside the worktree name.
  • Add a regression test for archived and closed badges rendering below the name.

Test plan

  • bun run test -- src/lib/WorktreeList.test.ts
  • bun run check
  • bun run test

Generated with Claude Code

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented May 12, 2026

Copy link
Copy Markdown

Claude finished @centdix's task in 1m 57s —— View job


Code Review

Tight, well-scoped change. The two-row layout reads cleanly and the regression test is reasonable. A few observations below — none are blockers.

What I like

  • DOM validity improvement (WorktreeList.svelte:101-146): the old layout nested a <div> inside the <button> element, which is technically invalid (buttons only allow phrasing content). The new structure is <span> end-to-end. Nice incidental fix.
  • hasBadgeRow gate (WorktreeList.svelte:87): explicitly skipping the badge container when there's nothing to show avoids an empty gap-1 slot under the name. Good touch.
  • Behavior preservation: the agent status icon visibility condition (!isCreating && !isInitializing && !isClosed) and the archived-stays-with-icon case both match main. No regressions in semantics.

Observations / nits

  1. items-start on the outer flex (WorktreeList.svelte:101) — changed from items-center. With a label (two-line name) plus a badge row, the depth arrow (line 103) now anchors to the top rather than centering. For a single-line name this is identical to items-center, but for nested worktrees with labels it may look slightly off. Worth a quick visual check; if it's intentional, ignore.

  2. Long @const line (WorktreeList.svelte:87) — 6 conditions in one expression. Purely cosmetic but a multi-line form would scan more easily:

    {@const hasBadgeRow =
      isArchived ||
      isCreating ||
      isInitializing ||
      isClosed ||
      wt.prs.length > 0 ||
      !!wt.linearIssue}
  3. Test selectors via data attributes (WorktreeList.svelte:106,121 + WorktreeList.test.ts:169-170) — adds two markup-only attributes that exist purely for tests. Pragmatic and clearer than DOM-walking with closest("div"), so this is fine, but worth flagging since the project's testing-library-style tests elsewhere lean on getByText / getByRole. Not a request to change.

  4. Test asserts DOM separation, not visual order (WorktreeList.test.ts:176-178) — since both rows sit inside flex flex-col, DOM order implies vertical stacking, so the test is sufficient today. If anyone later swaps the column to a grid or absolute layout, the test would still pass while the visual contract broke. Minor; flagging only because the PR title is specifically "below the name".

  5. Redundant min-w-0 on nested spans (WorktreeList.svelte:101, 105, 106, 107) — min-w-0 is repeated at every level of the flex column. It's needed for truncation to work, but at 107 it's the only one strictly required for the name; 101/105/106 could likely be pruned. Tailwind classes are cheap, so this is a nit.

Suggested follow-up tests (optional)

The new regression test covers archived + closed. The other badge-row cases — PR badges, Linear badge, and the creating / initializing text — share the same container and would presumably regress together, but a single parametrized assertion proving the badge row remains a stable hook would harden the contract.


Approved from my side — ship it after the items-start visual check.

Todos

  • Read changed files (WorktreeList.svelte, WorktreeList.test.ts)
  • Analyze layout/styling correctness
  • Check Svelte 5 / DRY / project-convention compliance
  • Verify test coverage
  • Post review feedback
    · Branch: worktree-badges-below-name

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
webmux 10a2f6e Commit Preview URL

Branch Preview URL
May 12 2026, 04:31 PM

@centdix
centdix marked this pull request as ready for review May 13, 2026 08:41
@centdix
centdix merged commit 63130ac into main May 13, 2026
4 checks passed
@centdix
centdix deleted the worktree-badges-below-name branch May 13, 2026 08:41
@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @centdix's task in 1m 49s —— View job


Code Review

The previous review already covered the main observations well. Adding a few of my own notes; nothing is a blocker, and the merged state is fine.

Confirms / extra context

  • items-start is the right call (WorktreeList.svelte:101) — with a single-line name it's identical to items-center. With a label (2-line name) or a name + badge row, items-start aligns the depth arrow to the top of the content column rather than centering it between rows, which would have looked off. So this is an intentional improvement, not a regression.

  • DOM-validity nit fixed — the prior <div> inside <button> was technically invalid HTML (buttons accept phrasing content only). New structure is <span> end-to-end and is valid. Good incidental fix.

  • Behavior preservation is good: the agent icon's visibility predicate !isCreating && !isInitializing && !isClosed is exactly equivalent to the prior {:else} chain. Archived rows still show the agent icon (no behavior drift). Notification dot still renders whenever notifiedBranches.has(wt.branch).

Observations / nits

  1. @const hasBadgeRow is long enough to wrap (WorktreeList.svelte:87) — six predicates in one expression. Cosmetic, but multi-line would be easier to scan:

    {@const hasBadgeRow =
      isArchived ||
      isCreating ||
      isInitializing ||
      isClosed ||
      wt.prs.length > 0 ||
      !!wt.linearIssue}
  2. Test asserts DOM containment, not vertical order (WorktreeList.test.ts:176-178). Today that's sufficient because both rows live in the same flex flex-col, but the PR title literally says "below the name" — if someone later swaps to a grid or absolute layout, the test would still pass while the visual contract breaks. A compareDocumentPosition check would harden it:

    expect(nameRow.compareDocumentPosition(badgeRow) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
  3. Test selectors via data-* attributes (WorktreeList.svelte:106,121) are pragmatic here — getByText/getByRole can't easily distinguish the two rows. Worth noting only because the project's other Testing Library tests lean role-/text-first; this is a fine exception for a structural layout test.

  4. Redundant min-w-0s (101, 105, 106, 107) — they're cheap and harmless, but only 107 (the truncating column) strictly needs it. min-w-0 on a non-flex-child parent is a no-op. Pure nit.

  5. @const hasLabel (WorktreeList.svelte:86) is used only once (line 109). Could inline as {#if wt.label} and drop the alias. Existing in pre-PR code, so probably not worth touching here.

  6. Test default uses linearIssue set — the createWorktree factory in WorktreeList.test.ts:25-33 always returns a Linear issue, so the new test renders the LinearBadge in the badge row implicitly. It doesn't affect what's asserted, but the badge row in this test has more than just archived/closed in it. If someone reads the test in isolation they may be surprised — a one-line comment or an explicit linearIssue: null override would clarify intent.

Suggested follow-up (optional)

The new test covers archived + closed. The other badge-row paths (PRs, Linear, creating/initializing text) share the same container and would presumably regress together, but a single parameterized assertion proving the badge row is a stable hook for any badge would harden the contract — useful since data-worktree-badge-row is now load-bearing for at least one test.


LGTM — merged code is sound, the layout change is well-scoped, and the regression test gives reasonable coverage. The follow-ups above are all optional polish.
· Branch: worktree-badges-below-name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant