Skip to content

fix(widgets): mark TextInput dirty after state mutations#677

Merged
Karanjot786 merged 1 commit into
Karanjot786:mainfrom
srushti-panara:bug/textinput-markdirty
Jun 4, 2026
Merged

fix(widgets): mark TextInput dirty after state mutations#677
Karanjot786 merged 1 commit into
Karanjot786:mainfrom
srushti-panara:bug/textinput-markdirty

Conversation

@srushti-panara
Copy link
Copy Markdown
Contributor

@srushti-panara srushti-panara commented Jun 4, 2026

Description

This PR fixes a rendering issue in the TextInput widget where several state-mutating methods updated internal state without calling markDirty().

Added markDirty() calls to all methods that modify rendered state (value setter, text insertion/deletion, cursor movement, and clear operation) so programmatic updates trigger immediate re-rendering.

Related Issue

Closes #675

Which package(s)?

@termuijs/widgets

Type of Change

  • 🐛 Bug fix (type:bug)
  • ✨ Feature (type:feature)
  • 📝 Docs (type:docs)
  • 🧪 Tests (type:testing)
  • ♻️ Refactor (type:refactor)
  • 🎨 Design / UX (type:design)
  • ♿ Accessibility (type:accessibility)
  • ⚡ Performance (type:performance)
  • 🔧 DevOps / CI (type:devops)
  • 🔒 Security (type:security)

Checklist

  • ⭐ You starred the repo. The needs-star check blocks your merge otherwise.
  • Tests pass locally: bun vitest run
  • Build passes: bun run build
  • Typecheck passes: bun run typecheck
  • You read CONTRIBUTING.md.
  • Your PR title follows type: short description.
  • Widget state mutators call markDirty() (if your change affects rendering).
  • No new any types without an inline comment explaining why.
  • No unrelated refactors bundled into this PR.

GSSoC 2026 Participation

  • You are a GSSoC 2026 contributor.
  • Your GSSoC profile: https://gssoc.girlscript.org/profile/a80c7e63-fb5a-4d58-aec9-115f9a2798b7

Screenshots / Recordings (UI changes)

N/A

Notes for the Reviewer

This change is intentionally limited to TextInput state-mutating methods and does not modify rendering logic or widget behavior beyond ensuring proper dirty-state propagation after updates.

Validation completed:

  • bun vitest run packages/widgets ✅
  • bun run typecheck ✅
  • bun run build✅

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced TextInput widget to reliably update the display during all user interactions. The component now consistently refreshes when entering text, navigating the cursor, deleting characters, and clearing the field, providing a more responsive and stable user experience.

@github-actions github-actions Bot added type:bug +10 pts. Bug fix. area:widgets @termuijs/widgets and removed type:bug +10 pts. Bug fix. labels Jun 4, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

TextInput.ts now calls markDirty() after each state mutation to ensure reliable widget redraws. The value setter, character insertion, deletion, cursor movement, and clear methods each mark the widget dirty after updating internal state and notifying via onChange.

Changes

TextInput dirty state tracking

Layer / File(s) Summary
Add markDirty() calls to all state-mutating methods
packages/widgets/src/input/TextInput.ts
Value setter, insertChar, deleteBack, deleteForward, clear, and cursor movement helpers (moveCursorLeft, moveCursorRight, moveCursorHome, moveCursorEnd) each now call markDirty() after mutating \_value or \_cursorPos so the widget redraws immediately.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 The input field bounces with glee,
Each keystroke and move now brings clarity,
With markDirty() calls in place so right,
The widget redraws quick and bright,
No hidden edits—just instant sight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the main change: adding markDirty() calls to TextInput state mutation methods.
Description check ✅ Passed The PR description follows the template with all required sections completed: description, related issue (#675), package identification, type of change (bug fix), and a comprehensive checklist.
Linked Issues check ✅ Passed The PR directly addresses issue #675 by adding markDirty() calls to all state-mutating methods (value setter, insertChar, deleteBack, deleteForward, cursor movement, clear) that modify _value and _cursorPos.
Out of Scope Changes check ✅ Passed All changes are scoped to TextInput state-mutation methods as required; no unrelated refactoring or rendering logic modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the type:bug +10 pts. Bug fix. label Jun 4, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/widgets/src/input/TextInput.ts (1)

94-105: 💤 Low value

Consider guarding markDirty() calls for consistency.

The cursor movement methods call markDirty() unconditionally, even when the cursor position doesn't actually change (e.g., moving left when already at position 0). This differs from deleteBack and deleteForward, which only call markDirty() when state actually changes.

While the current implementation is functionally correct (markDirty() is idempotent and cheap), aligning these methods with the conditional pattern would improve consistency.

♻️ Optional refactor for consistency
-moveCursorLeft(): void { this._cursorPos = Math.max(0, this._cursorPos - 1); 
-    this.markDirty();
-}
-moveCursorRight(): void { this._cursorPos = Math.min(this._value.length, this._cursorPos + 1); 
-    this.markDirty();
-}
-moveCursorHome(): void { this._cursorPos = 0; 
-    this.markDirty();
-}
-moveCursorEnd(): void { this._cursorPos = this._value.length;
-    this.markDirty();
- }
+moveCursorLeft(): void {
+    const newPos = Math.max(0, this._cursorPos - 1);
+    if (newPos !== this._cursorPos) {
+        this._cursorPos = newPos;
+        this.markDirty();
+    }
+}
+moveCursorRight(): void {
+    const newPos = Math.min(this._value.length, this._cursorPos + 1);
+    if (newPos !== this._cursorPos) {
+        this._cursorPos = newPos;
+        this.markDirty();
+    }
+}
+moveCursorHome(): void {
+    if (this._cursorPos !== 0) {
+        this._cursorPos = 0;
+        this.markDirty();
+    }
+}
+moveCursorEnd(): void {
+    if (this._cursorPos !== this._value.length) {
+        this._cursorPos = this._value.length;
+        this.markDirty();
+    }
+}
🤖 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/widgets/src/input/TextInput.ts` around lines 94 - 105, The cursor
movement methods (moveCursorLeft, moveCursorRight, moveCursorHome,
moveCursorEnd) call markDirty() unconditionally; change each to compute the new
position first, compare it against this._cursorPos, update this._cursorPos and
call this.markDirty() only when the position actually changed so behavior
matches deleteBack/deleteForward's conditional dirty-marking.
🤖 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.

Nitpick comments:
In `@packages/widgets/src/input/TextInput.ts`:
- Around line 94-105: The cursor movement methods (moveCursorLeft,
moveCursorRight, moveCursorHome, moveCursorEnd) call markDirty()
unconditionally; change each to compute the new position first, compare it
against this._cursorPos, update this._cursorPos and call this.markDirty() only
when the position actually changed so behavior matches
deleteBack/deleteForward's conditional dirty-marking.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 08f39626-f885-47c0-bacd-40eb5fec80dd

📥 Commits

Reviewing files that changed from the base of the PR and between c36cc8c and 41e0f08.

📒 Files selected for processing (1)
  • packages/widgets/src/input/TextInput.ts

@Karanjot786 Karanjot786 added gssoc:approved Approved PR. Earns +50 base points. quality:clean x 1.2 multiplier. Clean implementation. level:beginner +20 pts. Entry-level task. type:feature +10 pts. New feature. labels Jun 4, 2026
@Karanjot786 Karanjot786 merged commit 651760e into Karanjot786:main Jun 4, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:widgets @termuijs/widgets gssoc:approved Approved PR. Earns +50 base points. level:beginner +20 pts. Entry-level task. quality:clean x 1.2 multiplier. Clean implementation. type:bug +10 pts. Bug fix. type:feature +10 pts. New feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] TextInput state mutation methods missing markDirty()

2 participants