Skip to content

Commit 5e1d9d1

Browse files
committed
Stop the provider setup screen from garbling text on short terminals
Every direct child of the setup screen's root column needs flexShrink: 0. header, intro, step, instruction, statusLine, guidance, and footer were all missing it while every sibling box already had it, so a terminal too short for the full column let the flex algorithm compress these unprotected single-line rows onto each other instead of clipping from the bottom. Reproduces on the provider pick-list and independently on the failed-connection-test screen, where statusLine and guidance are populated together. The list height budget also reserved more chrome rows than the picker actually uses, so the footer could go missing even on terminals that had room for it.
1 parent 1e617b1 commit 5e1d9d1

2 files changed

Lines changed: 104 additions & 1 deletion

File tree

src/tui-opentui/provider-setup.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,3 +693,86 @@ describe("runProviderSetup paste", () => {
693693
expect(values?.apiKey).toBe(key)
694694
})
695695
})
696+
697+
describe("runProviderSetup pick-list height cap", () => {
698+
// Every terminal size gets a bounded frame — no chrome row overlaps
699+
// another (the header/intro/step/instruction rows used to compress into
700+
// each other when the flex column ran out of room), and the picker never
701+
// paints past the terminal's own row count.
702+
for (const height of [24, 16, 12, 8, 6]) {
703+
test(`stays within a ${height}-row terminal with no overlapping chrome`, async () => {
704+
const harness = await createHarness({ width: 80, height })
705+
runProviderSetup({
706+
onSubmit: async () => {},
707+
showTelemetryNotice: false,
708+
createRenderer: async () => harness.renderer,
709+
})
710+
await harness.renderOnce()
711+
await harness.renderOnce()
712+
const lines = harness.captureCharFrame().split("\n")
713+
expect(lines.length).toBeLessThanOrEqual(height + 1)
714+
// The garbled-overlap bug glued the step line and the intro line
715+
// together on one row; each survives as its own line, or is clipped
716+
// entirely, but never merges into the other.
717+
const stepLine = lines.find((l) => l.includes("step 1 of 3"))
718+
if (stepLine !== undefined) {
719+
expect(stepLine).not.toContain("connect an inference provider")
720+
}
721+
})
722+
}
723+
724+
test("keyboard navigation scrolls a long provider list and keeps the active row visible", async () => {
725+
const harness = await createHarness({ width: 80, height: 16 })
726+
runProviderSetup({
727+
onSubmit: async () => {},
728+
showTelemetryNotice: false,
729+
createRenderer: async () => harness.renderer,
730+
})
731+
await harness.renderOnce()
732+
await harness.renderOnce()
733+
const ids = providerChoiceRows(providerChoices()).map((r) => r.id)
734+
for (let i = 0; i < ids.length - 1; i++) harness.pressKey("ARROW_DOWN")
735+
await harness.renderOnce()
736+
const frame = harness.captureCharFrame()
737+
const last = providerChoiceRows(providerChoices()).at(-1)
738+
expect(last).toBeDefined()
739+
expect(frame).toContain(last!.label.slice(0, 20))
740+
})
741+
742+
// statusLine and guidance are both blank on the first screen these tests
743+
// exercised — the garbling only showed up once a failed connection test
744+
// populates both of them at once, so walk the flow there instead of
745+
// stopping at the provider pick-list.
746+
test("a failed connection test at a short terminal shows status and guidance on their own lines", async () => {
747+
const harness = await createHarness({ width: 80, height: 16 })
748+
runProviderSetup({
749+
onSubmit: async (_values, _setPhase, opts) => {
750+
if (!opts.skipValidation) throw new Error("connection refused")
751+
},
752+
showTelemetryNotice: false,
753+
createRenderer: async () => harness.renderer,
754+
})
755+
await harness.renderOnce()
756+
await harness.renderOnce()
757+
await pickRow(harness, PROVIDER_IDS, "openai")
758+
type(harness, "sk-key")
759+
harness.pressKey("Enter")
760+
await harness.renderOnce()
761+
harness.pressKey("Enter")
762+
await harness.renderOnce()
763+
await new Promise((r) => setTimeout(r, 0))
764+
await harness.renderOnce()
765+
766+
const lines = harness.captureCharFrame().split("\n")
767+
expect(lines.length).toBeLessThanOrEqual(17)
768+
const statusRow = lines.find((l) => l.includes("connection refused"))
769+
const guidanceRow = lines.find((l) => l.includes("esc to re-enter"))
770+
expect(statusRow).toBeDefined()
771+
expect(guidanceRow).toBeDefined()
772+
// The garbling bug glued these two rows together; each must survive as
773+
// its own line, never merged into the other.
774+
expect(statusRow).not.toBe(guidanceRow)
775+
expect(statusRow).not.toContain("esc to re-enter")
776+
expect(guidanceRow).not.toContain("connection refused")
777+
})
778+
})

src/tui-opentui/provider-setup.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -645,8 +645,15 @@ export async function runProviderSetup(
645645
})
646646

647647
function listHeight(): number {
648+
// Fixed chrome above the list: root padding, header, intro, step,
649+
// instruction, one populated summary row, and the list box's own
650+
// padding — plus the footer below it, and slack for a long label
651+
// wrapping onto a second terminal row. A tighter budget than the old
652+
// flat -14 so the list actually uses the room a short terminal leaves it
653+
// instead of reserving rows nothing else needs and sitting short.
654+
const chromeRows = 12
648655
const rows = renderer.height || 24
649-
return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - 14))
656+
return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - chromeRows))
650657
}
651658

652659
const steps = (): readonly SetupStep[] => stepsFor(choice)
@@ -669,25 +676,35 @@ export async function runProviderSetup(
669676
paddingRight: margin,
670677
})
671678

679+
// Every direct child of `root` needs flexShrink: 0, full stop — a plain
680+
// TextRenderable defaults to shrinkable, and a short terminal makes the
681+
// flex algorithm compress unprotected single-line rows into each other
682+
// (garbled overlapping text) instead of clipping the column from the
683+
// bottom. header/intro/step/instruction here, and statusLine/guidance/
684+
// footer further down, all needed this; it is not specific to one step.
672685
const header = new TextRenderable(renderer, {
673686
id: "provider-setup-header",
674687
content: `${PRODUCT_NAME.toLowerCase()} · setup`,
675688
fg: UI.inFlightBright,
689+
flexShrink: 0,
676690
})
677691
const intro = new TextRenderable(renderer, {
678692
id: "provider-setup-welcome",
679693
content: "connect an inference provider — switch later with /model",
680694
fg: UI.textDim,
695+
flexShrink: 0,
681696
})
682697
const step = new TextRenderable(renderer, {
683698
id: "provider-setup-step",
684699
content: "",
685700
fg: UI.action,
701+
flexShrink: 0,
686702
})
687703
const instruction = new TextRenderable(renderer, {
688704
id: "provider-setup-instruction",
689705
content: "",
690706
fg: UI.text,
707+
flexShrink: 0,
691708
})
692709

693710
const summary = new BoxRenderable(renderer, {
@@ -777,11 +794,13 @@ export async function runProviderSetup(
777794
id: "provider-setup-status",
778795
content: "",
779796
fg: UI.textDim,
797+
flexShrink: 0,
780798
})
781799
const guidance = new TextRenderable(renderer, {
782800
id: "provider-setup-guidance",
783801
content: "",
784802
fg: UI.textDim,
803+
flexShrink: 0,
785804
})
786805
const telemetry = new BoxRenderable(renderer, {
787806
id: "provider-setup-telemetry",
@@ -816,6 +835,7 @@ export async function runProviderSetup(
816835
id: "provider-setup-footer",
817836
content: "",
818837
fg: UI.textFaint,
838+
flexShrink: 0,
819839
})
820840

821841
root.add(header)

0 commit comments

Comments
 (0)