From 351509838cd14ad251ca4b152c0bb4577c9d7b79 Mon Sep 17 00:00:00 2001 From: Erik Kotsiuba Date: Sat, 25 Apr 2026 20:26:07 +0200 Subject: [PATCH] fix(ui): tighten ask layout rendering --- src/ui/constants.ts | 2 +- src/ui/render-frame.ts | 124 ++++++++++++++++------------------ src/ui/render-helpers.ts | 13 ++++ src/ui/render-question.ts | 24 +++---- src/ui/render-submit.ts | 65 ++++++++++-------- tests/render-frame.test.ts | 109 ++++++++++++++++++++++++++++-- tests/render-question.test.ts | 2 +- tests/render-submit.test.ts | 95 ++++++++++++++++++++------ 8 files changed, 301 insertions(+), 133 deletions(-) diff --git a/src/ui/constants.ts b/src/ui/constants.ts index 5f4def4..d209b72 100644 --- a/src/ui/constants.ts +++ b/src/ui/constants.ts @@ -10,7 +10,7 @@ export const UI_DIMENSIONS = { previewLeftMaxWidth: 34, previewLeftRatio: 0.34, submitWideMinWidth: 64, - submitActionsWidth: 24, + submitMinReviewWidth: 24, } as const; export const UI_TEXT = { diff --git a/src/ui/render-frame.ts b/src/ui/render-frame.ts index 10c78dc..9cab6a4 100644 --- a/src/ui/render-frame.ts +++ b/src/ui/render-frame.ts @@ -45,6 +45,10 @@ export function renderFrameFooter(args: { add(theme.fg("accent", "─".repeat(Math.max(1, width)))); } +const TAB_PREFIX = " ← "; +const TAB_SUFFIX = " →"; +const TAB_SEPARATOR_WIDTH = visibleWidth(" "); + function renderTabs(state: AskState, theme: Theme, width: number): string { const tabs = state.questions.map((question, index) => { const active = state.activeTabIndex === index; @@ -70,18 +74,25 @@ function renderTabs(state: AskState, theme: Theme, width: number): string { }); const activeIndex = Math.min(state.activeTabIndex, tabs.length - 1); + const availableTabWidth = Math.max( + 1, + width - visibleWidth(TAB_PREFIX) - visibleWidth(TAB_SUFFIX) + ); const { start, end } = getVisibleTabRange( tabs.map((tab) => tab.width), activeIndex, - Math.max(1, width - 2) + availableTabWidth ); - const leftArrow = theme.fg("dim", "← "); - const rightArrow = theme.fg("dim", " →"); + const leftArrow = theme.fg("dim", TAB_PREFIX.trimStart()); + const rightArrow = theme.fg("dim", TAB_SUFFIX); const visibleTabs = tabs .slice(start, end + 1) .map((tab) => tab.render) .join(" "); - return truncateToWidth(` ${leftArrow}${visibleTabs}${rightArrow}`, width); + return truncateToWidth( + `${TAB_PREFIX[0]}${leftArrow}${visibleTabs}${rightArrow}`, + width + ); } function getVisibleTabRange( @@ -95,92 +106,76 @@ function getVisibleTabRange( const totalWidth = widths.reduce((sum, width) => sum + width, 0) + - Math.max(0, widths.length - 1); + Math.max(0, widths.length - 1) * TAB_SEPARATOR_WIDTH; if (totalWidth <= availableWidth) { return { start: 0, end: widths.length - 1 }; } - const zone = activeIndex / Math.max(1, widths.length - 1); - const mode = getViewportMode(zone); - return expandTabs(widths, activeIndex, availableWidth, mode); -} - -function getViewportMode(zone: number): "left" | "right" | "balanced" { - if (zone <= 0.33) { - return "right"; - } - if (zone >= 0.67) { - return "left"; - } - return "balanced"; -} - -function expandTabs( - widths: number[], - activeIndex: number, - availableWidth: number, - mode: "left" | "right" | "balanced" -): { start: number; end: number } { const range = { start: activeIndex, end: activeIndex }; let usedWidth = widths[activeIndex] ?? 0; - let preferRight = mode !== "left"; + let preferRight = activeIndex <= widths.length - activeIndex - 1; while (true) { - const directions = getGrowthDirections(mode, preferRight); - const grew = directions.some((direction) => { - const nextWidth = getNextTabWidth(widths, range, direction); - if ( - nextWidth === undefined || - usedWidth + 1 + nextWidth > availableWidth - ) { - return false; - } - usedWidth += 1 + nextWidth; - growRange(range, direction); - return true; - }); - if (!grew) { + const expansion = getExpandableTab( + widths, + range, + availableWidth, + usedWidth, + preferRight + ); + if (!expansion) { return range; } - if (mode === "balanced") { - preferRight = !preferRight; - } + usedWidth += TAB_SEPARATOR_WIDTH + expansion.width; + applyTabGrowth(range, expansion.nextIndex, expansion.direction); + preferRight = !preferRight; } } -function getGrowthDirections( - mode: "left" | "right" | "balanced", +function getExpandableTab( + widths: number[], + range: { start: number; end: number }, + availableWidth: number, + usedWidth: number, preferRight: boolean -): Array<"left" | "right"> { - if (mode === "left") { - return ["left", "right"]; - } - if (mode === "right") { - return ["right", "left"]; +): + | { direction: "left" | "right"; nextIndex: number; width: number } + | undefined { + for (const direction of getGrowthDirections(preferRight)) { + const nextIndex = getNextTabIndex(range, direction); + const nextWidth = widths[nextIndex]; + if ( + nextWidth === undefined || + usedWidth + TAB_SEPARATOR_WIDTH + nextWidth > availableWidth + ) { + continue; + } + return { direction, nextIndex, width: nextWidth }; } + return; +} + +function getGrowthDirections(preferRight: boolean): Array<"left" | "right"> { return preferRight ? ["right", "left"] : ["left", "right"]; } -function getNextTabWidth( - widths: number[], +function getNextTabIndex( range: { start: number; end: number }, direction: "left" | "right" -): number | undefined { - if (direction === "left") { - return widths[range.start - 1]; - } - return widths[range.end + 1]; +): number { + return direction === "left" ? range.start - 1 : range.end + 1; } -function growRange( +function applyTabGrowth( range: { start: number; end: number }, + nextIndex: number, direction: "left" | "right" ) { if (direction === "left") { - range.start -= 1; + range.start = nextIndex; return; } - range.end += 1; + range.end = nextIndex; } function renderFooter(state: AskState, width: number): string[] { @@ -195,10 +190,10 @@ function renderFooter(state: AskState, width: number): string[] { const question = getCurrentQuestion(state); footer = renderFooterText(question?.type === "multi" ? "multi" : "default"); } - return wrapFooterHints(footer, width); + return wrapDelimitedFooterHints(footer, width); } -function wrapFooterHints(footer: string, width: number): string[] { +function wrapDelimitedFooterHints(footer: string, width: number): string[] { return footer.split(" · ").reduce((lines, chunk) => { const current = lines.at(-1) ?? ""; const next = current ? `${current} · ${chunk}` : chunk; @@ -217,6 +212,7 @@ function wrapFooterHints(footer: string, width: number): string[] { lines.push(...wrappedChunk); return lines; } + lines.push(current); lines.push(...wrappedChunk); return lines; }, []); diff --git a/src/ui/render-helpers.ts b/src/ui/render-helpers.ts index abe8674..6e5b46e 100644 --- a/src/ui/render-helpers.ts +++ b/src/ui/render-helpers.ts @@ -330,6 +330,19 @@ export function measurePreviewLeftWidth( ); } +export function getSavedNotePrefixes( + theme: Theme, + args: { indent: string; label?: string } +) { + const title = args.label + ? `${args.label} ${UI_TEXT.questionNoteTitle}` + : UI_TEXT.questionNoteTitle; + return { + prefix: `${args.indent}${theme.fg("syntaxString", title)} `, + continuationPrefix: `${args.indent}${" ".repeat(visibleWidth(title) + 1)}`, + }; +} + export function renderFooterText( mode: "input" | "note" | "submit" | "multi" | "default" ) { diff --git a/src/ui/render-question.ts b/src/ui/render-question.ts index 8d381cf..cefa20f 100644 --- a/src/ui/render-question.ts +++ b/src/ui/render-question.ts @@ -11,6 +11,7 @@ import { import type { AskDisplayOption } from "../types.ts"; import { UI_DIMENSIONS, UI_TEXT } from "./constants.ts"; import { + getSavedNotePrefixes, measurePreviewLeftWidth, mergeColumns, pushWrappedText, @@ -57,14 +58,17 @@ function renderQuestionNote(context: QuestionRenderContext): boolean { if (!existingNote) { return false; } + const { prefix, continuationPrefix } = getSavedNotePrefixes(theme, { + indent: " ", + }); pushWrappedText( lines, existingNote, width, theme, "muted", - getSavedNotePrefix(theme, " "), - getSavedNoteContinuationPrefix(" ") + prefix, + continuationPrefix ); lines.push(""); return true; @@ -154,6 +158,7 @@ function renderPreviewQuestion(context: QuestionRenderContext) { answer, option: selectedOption, selected: true, + // Custom preview options already render their inline editor/content above. withGap: !selectedOption?.isCustomOption, }); } @@ -370,14 +375,17 @@ function renderSavedOptionNote( return; } const { lines, theme, width } = context; + const { prefix, continuationPrefix } = getSavedNotePrefixes(theme, { + indent: " ", + }); pushWrappedText( lines, optionNote, width, theme, "muted", - getSavedNotePrefix(theme, " "), - getSavedNoteContinuationPrefix(" ") + prefix, + continuationPrefix ); } @@ -429,14 +437,6 @@ function renderIndentedEditor( }); } -function getSavedNotePrefix(theme: Theme, indent: string): string { - return `${indent}${theme.fg("syntaxString", UI_TEXT.questionNoteTitle)} `; -} - -function getSavedNoteContinuationPrefix(indent: string): string { - return `${indent}${" ".repeat(visibleWidth(UI_TEXT.questionNoteTitle) + 1)}`; -} - function getOptionPrefix( questionType: QuestionRenderContext["question"]["type"], option: AskDisplayOption, diff --git a/src/ui/render-submit.ts b/src/ui/render-submit.ts index 13c8849..a43ed90 100644 --- a/src/ui/render-submit.ts +++ b/src/ui/render-submit.ts @@ -3,7 +3,11 @@ import { SUBMIT_CHOICES } from "../constants.ts"; import { serializeAnswer } from "../state/answers.ts"; import type { AskResult, AskState, AskStateAnswer } from "../types.ts"; import { UI_DIMENSIONS, UI_TEXT } from "./constants.ts"; -import { mergeColumns, pushWrappedText } from "./render-helpers.ts"; +import { + getSavedNotePrefixes, + mergeColumns, + pushWrappedText, +} from "./render-helpers.ts"; import type { Theme } from "./render-types.ts"; type ReviewAnswer = AskResult["answers"][string] & { @@ -20,21 +24,21 @@ export function renderSubmitScreen( width: number ) { const showAllNotes = state.activeSubmitActionIndex === 1; + const actionColumnWidth = getSubmitActionColumnWidth(); - if (shouldUseWideSubmitLayout(width)) { - const leftWidth = UI_DIMENSIONS.submitActionsWidth; - const rightWidth = Math.max(1, width - leftWidth - 2); + if (shouldUseWideSubmitLayout(width, actionColumnWidth)) { + const rightWidth = Math.max(1, width - actionColumnWidth - 2); const reviewLines = renderSubmitReviewLines( state, theme, rightWidth, showAllNotes ); - const actionLines = renderSubmitActionLines(state, theme, leftWidth); + const actionLines = renderSubmitActions(state, theme, actionColumnWidth); for (const line of mergeColumns( actionLines, reviewLines, - leftWidth, + actionColumnWidth, width )) { lines.push(line); @@ -48,7 +52,7 @@ export function renderSubmitScreen( width, showAllNotes ); - const actionLines = renderSubmitActionLines(state, theme, width); + const actionLines = renderSubmitActions(state, theme, width); lines.push(...reviewLines); lines.push(""); lines.push(...actionLines); @@ -81,18 +85,22 @@ function renderSubmitReviewLines( return lines; } -function renderSubmitActionLines( - state: AskState, - theme: Theme, - width: number -): string[] { - const lines: string[] = []; - renderSubmitActions(lines, state, theme, width); - return lines; +function getSubmitActionColumnWidth(): number { + return Math.max( + ...SUBMIT_CHOICES.map((choice, index) => + visibleWidth(`❯ ${index + 1}. ${choice}`) + ) + ); } -function shouldUseWideSubmitLayout(width: number): boolean { - return width >= UI_DIMENSIONS.submitWideMinWidth; +function shouldUseWideSubmitLayout( + width: number, + actionColumnWidth: number +): boolean { + return ( + width >= UI_DIMENSIONS.submitWideMinWidth && + width - actionColumnWidth - 2 >= UI_DIMENSIONS.submitMinReviewWidth + ); } function renderSubmittedQuestion( @@ -188,16 +196,16 @@ function renderSubmittedNote( theme: Theme, width: number ) { - const indent = " "; - const notePrefix = `${indent}${theme.fg("syntaxString", UI_TEXT.questionNoteTitle)} `; - const continuationPrefix = `${indent}${" ".repeat(visibleWidth(UI_TEXT.questionNoteTitle) + 1)}`; + const { prefix, continuationPrefix } = getSavedNotePrefixes(theme, { + indent: " ", + }); pushWrappedText( lines, note, width, theme, "muted", - notePrefix, + prefix, continuationPrefix ); } @@ -209,17 +217,17 @@ function renderLabeledSubmittedNote( theme: Theme, width: number ) { - const indent = " "; - const title = `${label} ${UI_TEXT.questionNoteTitle}`; - const notePrefix = `${indent}${theme.fg("syntaxString", title)} `; - const continuationPrefix = `${indent}${" ".repeat(visibleWidth(title) + 1)}`; + const { prefix, continuationPrefix } = getSavedNotePrefixes(theme, { + indent: " ", + label, + }); pushWrappedText( lines, note, width, theme, "muted", - notePrefix, + prefix, continuationPrefix ); } @@ -282,11 +290,11 @@ function toReviewAnswer( } function renderSubmitActions( - lines: string[], state: AskState, theme: Theme, width: number -) { +): string[] { + const lines: string[] = []; for (let index = 0; index < SUBMIT_CHOICES.length; index++) { const selected = index === state.activeSubmitActionIndex; const prefix = selected ? "❯ " : " "; @@ -300,4 +308,5 @@ function renderSubmitActions( prefix ); } + return lines; } diff --git a/tests/render-frame.test.ts b/tests/render-frame.test.ts index 6b3debf..9698976 100644 --- a/tests/render-frame.test.ts +++ b/tests/render-frame.test.ts @@ -28,6 +28,20 @@ function mockEditor() { } as never; } +function plainTheme() { + return { + fg(_color: string, text: string) { + return text; + }, + bg(_color: string, text: string) { + return text; + }, + bold(text: string) { + return text; + }, + } as never; +} + test("tab strip always shows dimmed framing arrows on wide screens", () => { const state = createInitialState({ title: "Demo", @@ -98,13 +112,15 @@ test("narrow tab strip keeps active middle tab visible", () => { const lines = renderAskScreen({ state, - theme: mockTheme(), + theme: plainTheme(), width: 28, editor: mockEditor(), }); - assert(lines.some((line) => line.includes("Three"))); - assert(!lines.some((line) => line.includes("{selectedBg} One "))); + const tabLine = lines.find((line) => line.includes("← ")); + assert.ok(tabLine); + assert(tabLine.includes("Three")); + assert(!tabLine.includes("One")); }); test("narrow tab strip keeps submit tab visible when active", () => { @@ -136,13 +152,69 @@ test("narrow tab strip keeps submit tab visible when active", () => { const lines = renderAskScreen({ state, - theme: mockTheme(), + theme: plainTheme(), width: 24, editor: mockEditor(), }); - assert(lines.some((line) => line.includes("Submit"))); - assert(lines.some((line) => line.includes("←") || line.includes("→"))); + const tabLine = lines.find((line) => line.includes("Submit")); + assert.ok(tabLine); + assert(tabLine.includes("Submit")); + assert(tabLine.includes("←") || tabLine.includes("→")); +}); + +test("tab strip avoids truncation at narrow boundary widths", () => { + const state = createInitialState({ + title: "Demo", + questions: [ + { + id: "q1", + label: "One", + prompt: "One", + options: [{ value: "a", label: "A" }], + }, + { + id: "q2", + label: "Two", + prompt: "Two", + options: [{ value: "a", label: "A" }], + }, + { + id: "q3", + label: "Three", + prompt: "Three", + options: [{ value: "a", label: "A" }], + }, + { + id: "q4", + label: "Four", + prompt: "Four", + options: [{ value: "a", label: "A" }], + }, + { + id: "q5", + label: "Five", + prompt: "Five", + options: [{ value: "a", label: "A" }], + }, + ], + }); + state.activeTabIndex = 2; + + for (const width of [28, 29, 30, 31, 32]) { + const lines = renderAskScreen({ + state, + theme: plainTheme(), + width, + editor: mockEditor(), + }); + const tabLine = lines.find((line) => line.includes("← ")); + assert.ok(tabLine); + assert(tabLine.includes("Three")); + assert(!tabLine.includes("...")); + assert(!tabLine.includes("Fou...")); + assert(!tabLine.includes("Fi...")); + } }); test("footer hints wrap on narrow screens instead of truncating", () => { @@ -160,7 +232,7 @@ test("footer hints wrap on narrow screens instead of truncating", () => { const lines = renderAskScreen({ state, - theme: mockTheme(), + theme: plainTheme(), width: 26, editor: mockEditor(), }); @@ -169,3 +241,26 @@ test("footer hints wrap on narrow screens instead of truncating", () => { assert(lines.some((line) => line.includes("Enter continue"))); assert(lines.some((line) => line.includes("N/Shift+N note"))); }); + +test("footer keeps earlier hint chunks when later chunks wrap", () => { + const state = createInitialState({ + questions: [ + { + id: "q1", + label: "One", + prompt: "One", + options: [{ value: "a", label: "A" }], + }, + ], + }); + + const lines = renderAskScreen({ + state, + theme: plainTheme(), + width: 22, + editor: mockEditor(), + }); + + assert(lines.some((line) => line.includes("⇆ tab · ↑↓ select"))); + assert(lines.some((line) => line.includes("Enter confirm"))); +}); diff --git a/tests/render-question.test.ts b/tests/render-question.test.ts index 66f1b45..030cf36 100644 --- a/tests/render-question.test.ts +++ b/tests/render-question.test.ts @@ -210,5 +210,5 @@ test("preview custom option reuses the normal inline editor", () => { const optionIndex = lines.findIndex((line) => line.includes("Type your own")); const inputIndex = lines.findIndex((line) => line.includes("Type answer...")); assert.equal(inputIndex, optionIndex + 1); - assert(!lines.some((line) => line.includes("asdasddasdasdasdere"))); + assert(!lines.some((line) => line.includes("Preview A"))); }); diff --git a/tests/render-submit.test.ts b/tests/render-submit.test.ts index ca1982c..eb6a4f5 100644 --- a/tests/render-submit.test.ts +++ b/tests/render-submit.test.ts @@ -7,15 +7,16 @@ import { enterQuestionNoteMode, saveNote, } from "../src/state/transitions.ts"; +import { UI_DIMENSIONS } from "../src/ui/constants.ts"; import { renderSubmitScreen } from "../src/ui/render-submit.ts"; -function mockTheme() { +function plainTheme() { return { - fg(color: string, text: string) { - return `<${color}>${text}`; + fg(_color: string, text: string) { + return text; }, - bg(color: string, text: string) { - return `{${color}}${text}{/${color}}`; + bg(_color: string, text: string) { + return text; }, bold(text: string) { return text; @@ -36,7 +37,12 @@ test("submit screen uses shorter action labels without extra prompt text", () => }); const lines: string[] = []; - renderSubmitScreen(lines, state, mockTheme(), 80); + renderSubmitScreen( + lines, + state, + plainTheme(), + UI_DIMENSIONS.submitWideMinWidth + 16 + ); assert(!lines.some((line) => line.includes("Submit answers?"))); assert(lines.some((line) => line.includes("1. Submit"))); @@ -58,7 +64,12 @@ test("submit screen uses side-by-side layout on wide screens", () => { }); const lines: string[] = []; - renderSubmitScreen(lines, state, mockTheme(), 120); + renderSubmitScreen( + lines, + state, + plainTheme(), + UI_DIMENSIONS.submitWideMinWidth + 16 + ); assert( lines.some( @@ -83,7 +94,12 @@ test("submit screen stacks review above actions on narrow screens", () => { }); const lines: string[] = []; - renderSubmitScreen(lines, state, mockTheme(), 50); + renderSubmitScreen( + lines, + state, + plainTheme(), + UI_DIMENSIONS.submitWideMinWidth - 14 + ); const reviewIndex = lines.findIndex((line) => line.includes("Review answers") @@ -127,11 +143,14 @@ test("submit screen shows notes only for answered questions in submit mode", () state = saveNote(state, "Second note"); const lines: string[] = []; - renderSubmitScreen(lines, state, mockTheme(), 140); - - const firstQuestionIndex = lines.findIndex((line) => - line.includes("Single") + renderSubmitScreen( + lines, + state, + plainTheme(), + UI_DIMENSIONS.submitWideMinWidth + 76 ); + + const firstQuestionIndex = lines.findIndex((line) => line.includes("Single")); const firstQuestionNoteIndex = lines.findIndex((line) => line.includes("Question note") ); @@ -141,9 +160,7 @@ test("submit screen shows notes only for answered questions in submit mode", () const optionNoteIndex = lines.findIndex((line) => line.includes("Option note") ); - const secondQuestionIndex = lines.findIndex((line) => - line.includes("Multi") - ); + const secondQuestionIndex = lines.findIndex((line) => line.includes("Multi")); assert.notEqual(firstQuestionIndex, -1); assert.notEqual(firstQuestionNoteIndex, -1); @@ -155,9 +172,7 @@ test("submit screen shows notes only for answered questions in submit mode", () assert(lines[firstQuestionNoteIndex]?.startsWith(" ")); assert(lines[optionNoteIndex]?.startsWith(" ")); assert.equal(lines[secondQuestionIndex - 1]?.trim(), ""); - assert( - lines.some((line) => line.includes("Note:")) - ); + assert(lines.some((line) => line.includes("Note:"))); assert(!lines.some((line) => line.includes("Second note"))); assert(!lines.some((line) => line.includes("Question note:"))); assert(!lines.some((line) => line.includes("Code demo note:"))); @@ -200,7 +215,12 @@ test("submit screen shows all notes when elaborate action is selected", () => { state = { ...state, activeSubmitActionIndex: 1 }; const lines: string[] = []; - renderSubmitScreen(lines, state, mockTheme(), 120); + renderSubmitScreen( + lines, + state, + plainTheme(), + UI_DIMENSIONS.submitWideMinWidth + 16 + ); assert(lines.some((line) => line.includes("Question note"))); assert(lines.some((line) => line.includes("Selected option note"))); @@ -233,7 +253,12 @@ test("submit screen renders multi-select option notes under their related answer state = saveNote(state, "Second note"); const lines: string[] = []; - renderSubmitScreen(lines, state, mockTheme(), 140); + renderSubmitScreen( + lines, + state, + plainTheme(), + UI_DIMENSIONS.submitWideMinWidth + 76 + ); const firstAnswerIndex = lines.findIndex((line) => line.includes("→ Follow-up action") @@ -254,3 +279,33 @@ test("submit screen renders multi-select option notes under their related answer assert(firstNoteIndex < secondAnswerIndex); assert(secondAnswerIndex < secondNoteIndex); }); + +test("submit action column fits all action labels at the wide breakpoint", () => { + const state = createInitialState({ + questions: [ + { + id: "q1", + label: "Color", + prompt: "Pick one color.", + options: [{ value: "blue", label: "Blue" }], + }, + ], + }); + + const lines: string[] = []; + renderSubmitScreen( + lines, + state, + plainTheme(), + UI_DIMENSIONS.submitWideMinWidth + ); + + assert( + lines.some( + (line) => line.includes("1. Submit") && line.includes("Review answers") + ) + ); + assert(lines.some((line) => line.includes("2. Elaborate"))); + assert(lines.some((line) => line.includes("3. Cancel"))); + assert(!lines.some((line) => line.includes("Elaborat\n"))); +});