Skip to content

fix: code quality and safety improvements - #3367

Closed
saurabhhhcodes wants to merge 1 commit into
Karanjot786:mainfrom
saurabhhhcodes:fix/termui-40507
Closed

saurabhhhcodes wants to merge 1 commit into
Karanjot786:mainfrom
saurabhhhcodes:fix/termui-40507

Conversation

@saurabhhhcodes

@saurabhhhcodes saurabhhhcodes commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Improved numeric input validation across calculator examples and UI controls.
    • Prevented non-numeric values from being misinterpreted during number entry, pagination, and selection prompts.
    • Preserved existing clamping and validation behavior for valid inputs.

@github-actions github-actions Bot added type:bug +10 pts. Bug fix. area:examples Example apps. area:ui @termuijs/ui labels Aug 2, 2026
@coderabbitai

coderabbitai Bot commented Aug 2, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6de3817e-1d5b-4a80-889b-4abe0ae52d79

📥 Commits

Reviewing files that changed from the base of the PR and between 6c7584e and 3bd8c6b.

📒 Files selected for processing (4)
  • examples/calculator/src/index.tsx
  • packages/ui/src/NumberInput.ts
  • packages/ui/src/Pagination.ts
  • packages/ui/src/prompts.ts

📝 Walkthrough

Walkthrough

The change replaces global isNaN checks with strict Number.isNaN checks in the calculator and UI input validation paths. Existing clamping and fallback behavior remains unchanged.

Changes

NaN Validation

Layer / File(s) Summary
Calculator validation
examples/calculator/src/index.tsx
Negative-number detection and final-result validation now use Number.isNaN.
UI input validation
packages/ui/src/NumberInput.ts, packages/ui/src/Pagination.ts, packages/ui/src/prompts.ts
Parsed input checks now use Number.isNaN. Existing clamping, infinity handling, and fallback behavior remain unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: quality:clean

Suggested reviewers: karanjot786, tomeshwari-02

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided, so the required sections, issue link, package scope, change type, and checklist are missing. Add a description that completes the required template, including the linked issue, affected packages, change type, checklist, and reviewer notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title describes the code-quality and safety focus of the changes, although it does not identify the Number.isNaN validation updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:examples Example apps. area:ui @termuijs/ui type:bug +10 pts. Bug fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant