fix(ui): resolve TextArea wrapper imports and add tests#1923
fix(ui): resolve TextArea wrapper imports and add tests#1923Harshithk951 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughFixes missing imports (useState, useInput, KeyEvent) in TextArea.ts and changes the root element type from 'text' to 'box'. Adds a new Vitest test suite for TextArea covering initial render, typing/backspace, and newline/navigation behaviors. ChangesTextArea Fix and Tests
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/TextArea.ts (1)
112-122: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPer-row element type flips between
textandrowfor the samekeyas the cursor moves.The cursor row returns a bare
createElement('text', { key: r }, ...)while every other row returnscreateElement('row', { key: r, ... }, createElement('text', { width: ... }, ...)). Sincecursor.rowchanges as the user navigates, the element type bound to a givenkeyswitches betweentextandrowacross re-renders — the same class of type-mismatch reconciliation issue this PR fixes at the root level (text→box). This risks remounts/misalignment of sibling rows as the cursor moves.Additionally, the cursor row's text omits the
widthprop that the non-cursor branch sets (width: lineStr.length || 1), and its content embeds raw ANSI escape sequences (\x1b[7m...\x1b[27m) inline in the string. If width/layout is derived from raw string length, the non-printing escape bytes will inflate the computed width relative to the sibling branch, producing misaligned columns specifically on the cursor's line.Suggested direction
- if (r === cursor.row) { - const before = lineStr.slice(0, cursor.col); - const char = cursor.col < lineStr.length ? lineStr[cursor.col] : ' '; - const after = cursor.col < lineStr.length ? lineStr.slice(cursor.col + 1) : ''; - - return createElement('text', { key: r }, before + '\x1b[7m' + char + '\x1b[27m' + after); - } - return createElement('row', { key: r, gap: 0, width: '100%', height: 1 }, createElement('text', { width: lineStr.length || 1 }, lineStr || ' ')); + if (r === cursor.row) { + const before = lineStr.slice(0, cursor.col); + const char = cursor.col < lineStr.length ? lineStr[cursor.col] : ' '; + const after = cursor.col < lineStr.length ? lineStr.slice(cursor.col + 1) : ''; + const visibleLen = (before + char + after).length; + return createElement('row', { key: r, gap: 0, width: '100%', height: 1 }, + createElement('text', { width: visibleLen || 1 }, before + '\x1b[7m' + char + '\x1b[27m' + after)); + } + return createElement('row', { key: r, gap: 0, width: '100%', height: 1 }, createElement('text', { width: lineStr.length || 1 }, lineStr || ' '));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/TextArea.ts` around lines 112 - 122, The row rendering in TextArea is using the same key for two different element types, switching between createElement('text', ...) for the cursor row and createElement('row', ...) for non-cursor rows. Update the TextArea render path so the keyed row always uses a consistent wrapper type (for example, keep the same row structure and vary only the inner text content/styling) in the cursor and non-cursor branches. Also make the cursor branch match the non-cursor branch’s width/layout props and avoid embedding raw ANSI escape sequences directly in the rendered string so cursor movement does not trigger remounts or width misalignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/components/TextArea.test.tsx`:
- Around line 25-34: The TextArea test is asserting substrings that pass even
when no real navigation or split behavior occurs. Update the TextArea test to
move the cursor to the intended split point before pressing enter, using the
existing render/fireKey/getOutput flow in TextArea.test.tsx and the TextArea
cursor handling in TextArea.ts, then assert the rendered output reflects the
actual split at that position rather than matching characters already present in
the original value.
---
Outside diff comments:
In `@packages/ui/src/components/TextArea.ts`:
- Around line 112-122: The row rendering in TextArea is using the same key for
two different element types, switching between createElement('text', ...) for
the cursor row and createElement('row', ...) for non-cursor rows. Update the
TextArea render path so the keyed row always uses a consistent wrapper type (for
example, keep the same row structure and vary only the inner text
content/styling) in the cursor and non-cursor branches. Also make the cursor
branch match the non-cursor branch’s width/layout props and avoid embedding raw
ANSI escape sequences directly in the rendered string so cursor movement does
not trigger remounts or width misalignment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 92ae0a19-664f-4705-9743-d4b200b29c3d
📒 Files selected for processing (2)
packages/ui/src/components/TextArea.test.tsxpackages/ui/src/components/TextArea.ts
Description
This PR fixes missing imports (
useState,useInput,KeyEvent) in the functionalTextAreacomponent wrapper insidepackages/ui/src/components/TextArea.tsand changes its outer element fromtexttoboxto ensure children layout elements are reconciled properly. It also adds comprehensive unit tests.Related Issue
Closes #1921
Which package(s)?
@termuijs/ui
Type of Change
type:bug)type:feature)type:docs)type:testing)type:refactor)type:design)type:accessibility)type:performance)type:devops)type:security)Checklist
needs-starcheck blocks your merge otherwise.bun vitest runbun run buildbun run typecheckCONTRIBUTING.md.type: short description.markDirty()(if your change affects rendering).anytypes without an inline comment explaining why.GSSoC 2026 Participation
https://gssoc.girlscript.org/profile/Harshithk951Screenshots / Recordings (UI changes)
N/A
Notes for the Reviewer
None
Summary by CodeRabbit
Bug Fixes
Tests