Skip to content

fix(kanban): deterministic footer packing for narrow widths - #242

Merged
agent-era merged 3 commits into
mainfrom
kanban-narrow-bottom-bar
May 25, 2026
Merged

fix(kanban): deterministic footer packing for narrow widths#242
agent-era merged 3 commits into
mainfrom
kanban-narrow-bottom-bar

Conversation

@agent-era-ai

Copy link
Copy Markdown
Collaborator

Summary

  • Narrow-width kanban footer no longer wraps mid-word: hints are atomic chunks packed greedily with · separators.
  • Footer width is now selection-independent (unavailable hints render dimmed) — the up/down board "flicker" when moving between items with/without a worktree is gone.
  • Board's colHeight reserves the actual packed footer row count (up to 3 rows at narrow widths) so the layout no longer overflows the terminal.

Test plan

  • All 79 tracker-board terminal tests pass at the existing size matrix (10x80 → 40x160).
  • New per-width assertions verify footer rows split cleanly on · into known chunks (80, 96, 100, 120).
  • New stability test: render the same width twice with cursor in different columns; footer row count matches → no flicker.
  • Golden ASCII frame fixture at tests/fixtures/kanban-narrow-footer.txt (80×20).
  • Full jest suite (815 tests) passes; typecheck clean.

🤖 Generated with Claude Code

agent-era-ai and others added 3 commits May 25, 2026 04:39
…licker

At narrow widths (~80-120 cols) the kanban footer wrapped mid-word ("na" / "v",
"att" / "ach") because each of its ~30 sub-<Text>s wrapped independently. The
total width also varied with cursor position — hasItem/hasSession/hasWorktree
toggled whole hint groups, so moving selection changed wrap row count and
shifted the board: the "flicker".

Refactor Footer to atomic hint chunks with a constant chunk list:

- buildFooterChunks() returns the canonical list. Each chunk has a `plain`
  width (constant) and a `render` callback (state-aware). Unavailable hints
  render dimmed, never disappear — footer width is selection-independent.
- packFooterChunks() greedily packs chunks into rows separated by `  ·  `
  without ever splitting a chunk mid-label.
- Footer takes termCols, packs, renders one Box per row.
- colHeight subtracts the actual packed row count (max 3 footer rows at the
  narrowest), so the wrapped footer never pushes the board off-screen.

Tests:
- New per-width assertions (80/96/100/120) verify every footer row splits
  on `  ·  ` into known chunks — catches mid-word wrap regressions.
- New stability test renders the same width twice with different cursor
  positions and asserts footer row count matches.
- Golden ASCII frame at tests/fixtures/kanban-narrow-footer.txt for visual
  diffing.
- Relaxed minSpan in the existing "column boxes occupy most of the
  terminal height" assertion from height-4 to height-6 (footer can take
  up to 3 rows at narrow widths now).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…om-bar

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

claude Bot commented May 25, 2026

Copy link
Copy Markdown

Code Review — PR #242: fix(kanban): deterministic footer packing for narrow widths

Overview

This PR refactors the kanban board's footer from a single flat flex row to a greedy bin-packing layout that keeps hint chunks atomic, and computes the resulting row count ahead of render so colHeight accounts for multi-row footers. It fixes both mid-word wrapping at narrow widths and the cursor-move flicker that came from conditionally showing/hiding chunks.

The design is solid and the test coverage is notably thorough. A few things worth flagging:


Code Quality

packFooterChunks is called twice per render
TrackerBoardScreen calls packFooterChunks(buildFooterChunks(), ...) to compute colHeight, and then Footer (inside the same render tree) calls it again independently. The algorithm is cheap, but this is a conceptual inconsistency — if Footer diverges from the parent's width assumption, the layout will break. Consider passing the pre-computed rows down as a prop, or hoisting both calls into the parent and passing footerRows + rows together.

buildFooterChunks() creates new closures on every call
The inline dim and k helpers are recreated on every call. Defining them as module-level helpers would be cleaner and avoids unnecessary allocation on each render cycle.

Array index used as key in row map

rows.map((row, ri) => <Box key={ri}></Box>)

Using index as key is fine here because the rows are derived deterministically from a static chunk list, but using the first chunk's key in each row (key={row[0].key}) would be more explicit and React-idiomatic.

'[C' in test stdin — missing ESC prefix?

stdin.write('[C');

A proper ANSI cursor-right sequence is \x1b[C. Ink may strip the leading ESC internally or handle raw bracket sequences, but this looks like it could silently no-op on some environments. Worth adding a comment explaining why the ESC is absent, or switching to \x1b[C and verifying.


Potential Bugs

minSpan relaxed from height - 4 to height - 6 unconditionally
The existing size-matrix test now accepts boards up to 6 rows shorter than the terminal at all widths, not just narrow ones. At wide widths (120+) the footer is still 2–3 rows under the new packing, so the relaxed bound is correct — but the assertion no longer catches a regression where the board shrinks unnecessarily at wide terminals. A width-aware lower bound (e.g. height - 4 at ≥120 cols, height - 6 below) would be tighter.

Fixture test depends on exact ANSI stripping
kanban-narrow-footer.txt stores stripped ASCII. If stripAnsi behavior changes or box-drawing characters differ between Node versions/terminals, this test will fail noisily. A comment explaining how to regenerate the fixture would help (the error message partially does this, but the node command is incomplete — it shows ).


Performance

No significant concerns. packFooterChunks is O(n) in the number of chunks (~15) and runs synchronously on every render tick. Negligible cost.


Security

No security concerns.


Tests

The test coverage is strong:

  • Per-width atomic-chunk assertions at 80, 96, 100, 120
  • Flicker-stability test (cursor moves between columns, row count must match)
  • Golden ASCII fixture at 80×20

One gap: widths above 120 (e.g. 160) aren't covered. At 160 cols the old code sometimes fit in 1 row; the PR notes this now consistently wraps to 2. A note in the test or a single assertion at 160 would confirm this is intentional and not a regression.


Summary

The core algorithm and the fix are correct. The main action items:

  1. Consider passing footerRows/rows from parent to Footer to avoid double-packing.
  2. Fix or explain the '[C' stdin escape in the cursor-move test.
  3. Tighten the minSpan bound to be width-aware, or document why the blanket -6 is acceptable at all widths.

Nice work on the flicker fix — the "always render, dim when unavailable" decision is the right call.

@agent-era
agent-era merged commit 62605eb into main May 25, 2026
2 checks passed
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.

2 participants