Constrain PickerMenu description width to the terminal's real width - #988
Constrain PickerMenu description width to the terminal's real width#988Ref34t wants to merge 1 commit into
Conversation
Multi-select options with a description wrapped that text in a Box fixed at 56 columns, regardless of the actual terminal width. On a narrow terminal this let the row render wider than the physical screen, and the terminal's own line-wrapping (which Ink doesn't control) then misaligned Ink's cursor bookkeeping for every row after it, producing overlapping/corrupted text. Reads the live terminal width via Ink's useStdout() and clamps the description width to it (floor 20, cap 56 — the original constant, kept as an upper bound), accounting for the outer margin, grid columns, and the description's own indent. Adds a regression test that drives real Ink renders at several terminal widths by patching ink-testing-library's Stdout prototype, since the library hardcodes 100 columns with no override. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Closing this. On investigation the premise is wrong — see #986 (comment) for the detail. Short version: The actual trigger looks to be resizing the terminal window mid-run: Ink's Not reopening as a hardening change either — I'd rather not add a speculative patch to a component I no longer think is at fault. If narrow-terminal support for option descriptions is something you want, happy to raise it separately on its own merits. |
Fixes #986.
(Reopened from #987, which was accidentally branched from
wordpress-framework-supportinstead ofmainand dragged in unrelated files. This branch is based on currentmainand contains only the fix below.)The bug
Multi-select options with a
descriptionwrapped that text in aBoxfixed atwidth={56}, regardless of the actual terminal width. On a narrow terminal this let the row render wider than the physical screen. The terminal's own line-wrapping (which Ink doesn't control) then misaligned Ink's cursor bookkeeping for every row after it — producing the overlapping/corrupted text in #986: checkbox labels merging into their descriptions (No+Skip custom scouts...→NoSkip custom scouts...), the Confirm button overlapping the last option's wrapped tail, and — on a second checklist in the same run — fragments of multiple different tool names bleeding together (FrillrebaseghtVMreads like "Frill" + "Firebase" + something ending "...VM" overlapping).How I found it
Reproduced headlessly with
ink-testing-library, patching itsStdoutprototype to drive real widths (100 → 40 columns) since the library hardcodes 100 columns with no override. Themessagetext reflowed correctly at every width; the description did not — identical wrap points from 100 down to 40 columns, confirming the fixed width never adapted.The fix
descriptionWidth()inPickerMenu.tsxreads the live terminal width via Ink'suseStdout()and clamps to it — floor of 20 characters, cap of 56 (the original constant, kept as an upper bound on wide terminals), accounting for the outer margin, the per-column share when the grid has more than one column, and the description's own indent.Verified the fix directly: re-ran the same headless harness after the change — longest rendered line now tracks the terminal width exactly (100 cols → 62, 60 cols → 60, 40 cols → 40), instead of staying near-constant regardless of width.
Also found while investigating
PickerMenu(InputDemo.tsx) never exercises thedescriptionprop at all — the exact code path that broke had zero coverage in the one place built to catch this visually.ink-testing-libraryis a real devDependency but was unused anywhere in this repo before this fix. This is the first.tsxtest file —vitest.config.tswas already configured for it (esbuild: { jsx: 'automatic' }), just never exercised.Testing
pnpm build— clean, including smoke + warlock security testspnpm test— 1544/1544 passing against currentmain(1541 existing + 3 new)pnpm lint— 0 errorssrc/ui/tui/primitives/__tests__/PickerMenu.test.tsx, using the same monkeypatch-the-Stdout-prototype technique used to find the bug, kept as a permanent regression testOpened as a draft — background on how this surfaced (a real self-driving run against a Laravel/Vue app, not a wizard-workbench fixture) is in #986.