fix: resolve 4 bugs in termui - #3658
Conversation
📝 WalkthroughWalkthroughThe changes update numeric validation in the calculator, floating-point rounding in the pomodoro timer, and whitespace-only row detection in the development server. ChangesValidation and display corrections
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
🤖 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 `@examples/pomodoro-timer/src/index.tsx`:
- Line 185: Update the percentage calculation in the label expression of the
relevant timer rendering method to add Number.EPSILON to this._value before
multiplying by 100, preserving the existing rounding and formatting behavior.
Add a regression test covering this._value = 0.145 and assert that the displayed
label is 15%.
🪄 Autofix
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: dd67ae93-b310-4cc8-99e2-95a633f2d7ef
📒 Files selected for processing (3)
examples/calculator/src/index.tsxexamples/pomodoro-timer/src/index.tsxpackages/dev-server/src/devtools.ts
| const attrs = styleToCellAttrs(this._style); | ||
|
|
||
| const label = this._showLabel ? ` ${Math.round(this._value * 100)}%` : ''; | ||
| const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : ''; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Move Number.EPSILON before the percentage scale.
At this._value = 0.145, the product can be 14.499999999999998. Adding Number.EPSILON after multiplication may still leave the value below 14.5, so the label can display 14% instead of 15%.
Add the epsilon before multiplying by 100, and add a regression test for this boundary.
Proposed fix
- const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : '';
+ const label = this._showLabel ? ` ${Math.round((this._value + Number.EPSILON) * 100)}%` : '';🤖 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 `@examples/pomodoro-timer/src/index.tsx` at line 185, Update the percentage
calculation in the label expression of the relevant timer rendering method to
add Number.EPSILON to this._value before multiplying by 100, preserving the
existing rounding and formatting behavior. Add a regression test covering
this._value = 0.145 and assert that the displayed label is 15%.
Description
This PR fixes real bugs found in the codebase:
trim()to''misses whitespace-only input;.trim().length === 0is explicit.isNaNwithNumber.isNaN: the global version coerces its argument, soisNaN('1')returns false whileNumber.isNaNis strict.Number.EPSILONtoMath.round: prevents floating-point drift (e.g.1.005 * 100rounding to 100 instead of 101).trim()to''misses whitespace-only input;.trim().length === 0is explicit.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3657
Summary by CodeRabbit