fix: resolve 4 bugs in termui - #3520
Closed
saurabhhhcodes wants to merge 1 commit into
Closed
Conversation
📝 WalkthroughWalkthroughThe PR updates three examples. The calculator uses ChangesCalculator validation
Form shortcut handling
Pomodoro percentage rounding
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/forms-and-validation/src/index.tsx`:
- Line 125: Update the clear-form keyboard shortcut condition in the key event
handler to use boolean negation on event.ctrl, so it matches plain “c” without
Ctrl and does not use the postfix non-null assertion.
In `@examples/pomodoro-timer/src/index.tsx`:
- Line 185: Update the percentage calculation in the label expression to add
Number.EPSILON to the fractional value before multiplying by 100, so values such
as setValue(29 / 200) round to 15%. Preserve the existing conditional label
behavior and rounding.
🪄 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: 2d148c50-24f2-43c9-b991-37e241a7a628
📒 Files selected for processing (3)
examples/calculator/src/index.tsxexamples/forms-and-validation/src/index.tsxexamples/pomodoro-timer/src/index.tsx
| } | ||
|
|
||
| if (event.key === 'c' && event.ctrl === false) { | ||
| if (event.key === 'c' && event.ctrl !) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="examples/forms-and-validation/src/index.tsx"
! rg -n 'event\.ctrl !' "$file"
rg -n 'event\.key === .c. && !event\.ctrl' "$file"Repository: Karanjot786/TermUI
Length of output: 209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="examples/forms-and-validation/src/index.tsx"
echo "== surrounding code =="
sed -n '115,132p' "$file" | cat -n
echo
echo "== TypeScript compiler availability =="
(command -v tsc && tsc --version) || true
echo
echo "== parse/type behavior for event.ctrl ! if tsc is available =="
if command -v tsc >/dev/null 2>&1; then
tmp="$(mktemp -d)"
cat > "$tmp/probe.ts" <<'TS'
interface KeyEvent { ctrl: boolean }
declare const event: KeyEvent;
if (event.key === 'c' && event.ctrl !) {
const x: boolean = event.ctrl;
}
TS
tsc --noEmit --strict "$tmp/probe.ts" 2>&1 || true
else
echo "tsc not available"
fi
echo
echo "== exact current line =="
sed -n '125p' "$file"Repository: Karanjot786/TermUI
Length of output: 1956
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TS 7.0.2 parse/type behavior for event.ctrl ! (no DOM globals) =="
tmp="$(mktemp -d)"
cat > "$tmp/probe.ts" <<'TS'
declare function test(event: { key: string; ctrl: boolean }): void {
if (event.key === 'c' && event.ctrl !) {
const x: boolean = event.ctrl;
}
}
TS
tsc --noEmit --strict "$tmp/probe.ts" 2>&1 || true
echo
echo "== TS 7.0.2 parse/type behavior without exclamation spacing =="
cat > "$tmp/probe2.ts" <<'TS'
declare function test(event: { key: string; ctrl: boolean }): void {
if (event.key === 'c' && event.ctrl!) {
const x: boolean = event.ctrl;
}
}
TS
tsc --noEmit --strict "$tmp/probe2.ts" 2>&1 || trueRepository: Karanjot786/TermUI
Length of output: 739
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
cat > "$tmp/keyboard-handler.tsx" <<'TS'
interface KeyEvent { key: string; ctrl: boolean }
function handle(event: KeyEvent) {
if (event.key === 'q' || (event.ctrl && event.key === 'c')) {
return false;
}
if (event.key === 'c' && event.ctrl !) {
const yes: boolean = event.ctrl;
return true;
}
}
TS
tsc --noEmit --strict "$tmp/keyboard-handler.tsx" 2>&1 | sed -n '1,40p'Repository: Karanjot786/TermUI
Length of output: 156
Use boolean negation for the clear-form shortcut.
event.ctrl ! becomes the postfix non-null assertion event.ctrl!, so the condition requires Ctrl. Plain c never opens the modal, while Ctrl+C exits earlier. Replace it with !event.ctrl.
Proposed fix
- if (event.key === 'c' && event.ctrl !) {
+ if (event.key === 'c' && !event.ctrl) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (event.key === 'c' && event.ctrl !) { | |
| if (event.key === 'c' && !event.ctrl) { |
🤖 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/forms-and-validation/src/index.tsx` at line 125, Update the
clear-form keyboard shortcut condition in the key event handler to use boolean
negation on event.ctrl, so it matches plain “c” without Ctrl and does not use
the postfix non-null assertion.
Source: Coding guidelines
| 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
🧩 Analysis chain
🏁 Script executed:
node <<'NODE'
const value = 29 / 200;
const current = Math.round(value * 100 + Number.EPSILON);
const corrected = Math.round((value + Number.EPSILON) * 100);
if (current !== 14 || corrected !== 15) {
throw new Error(JSON.stringify({ value, current, corrected }));
}
NODERepository: Karanjot786/TermUI
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant implementation around setValue and the label rendering.
file="examples/pomodoro-timer/src/index.tsx"
if [ -f "$file" ]; then
wc -l "$file"
sed -n '1,230p' "$file" | nl -ba
else
echo "File not found: $file"
fi
# Read-only behavioral probe for the edge cases that can appear from clamped [0,1] values.
node <<'NODE'
function current(value, showLabel = true) {
return showLabel ? Math.round(value * 100 + Number.EPSILON) : '';
}
function corrected(value, showLabel = true) {
return showLabel ? Math.round((value + Number.EPSILON) * 100) : '';
}
const cases = [
29 / 200,
1 / 200,
28 / 200,
30 / 200,
50 / 200,
];
for (const value of cases) {
const exact = value * 100;
const displayCurrent = current(value);
const displayCorr = corrected(value);
const mathExactRound = Math.round(exact);
console.log(JSON.stringify({ value, exact, current: displayCurrent, corrected: displayCorr, mathExactRound }));
}
NODERepository: Karanjot786/TermUI
Length of output: 238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="examples/pomodoro-timer/src/index.tsx"
printf '--- line count ---\n'
wc -l "$file"
printf '\n--- lines 140-210 ---\n'
sed -n '140,210p' "$file"
printf '\n--- setValue and progress symbol occurrences ---\n'
grep -nE 'setValue|_value|showLabel|Number.EPSILON|Math.round' "$file" || trueRepository: Karanjot786/TermUI
Length of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="examples/pomodoro-timer/src/index.tsx"
printf '%s\n' '--- line count ---'
wc -l "$file"
printf '%s\n' ''
printf '%s\n' '--- lines 140-210 ---'
sed -n '140,210p' "$file"
printf '%s\n' ''
printf '%s\n' '--- setValue and progress symbol occurrences ---'
grep -nE 'setValue|_value|showLabel|Number.EPSILON|Math.round' "$file" || true
printf '%s\n' ''
printf '%s\n' '--- behavioral probe ---'
node <<'NODE'
function current(value, showLabel = true) {
return showLabel ? Math.round(value * 100 + Number.EPSILON) : '';
}
function corrected(value, showLabel = true) {
return showLabel ? Math.round((value + Number.EPSILON) * 100) : '';
}
const cases = [
29 / 200,
1 / 200,
28 / 200,
30 / 200,
50 / 200,
];
for (const value of cases) {
const exact = value * 100;
const displayCurrent = current(value);
const displayCorr = corrected(value);
const mathExactRound = Math.round(exact);
console.log(JSON.stringify({ value, exact, current: displayCurrent, corrected: displayCorr, mathExactRound }));
}
NODERepository: Karanjot786/TermUI
Length of output: 4790
Fix the progress percentage rounding.
setValue(29 / 200) rounds to 14% because 0.145 * 100 + Number.EPSILON still rounds down. Add epsilon before scaling the percentage.
- const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : '';
+ const label = this._showLabel ? ` ${Math.round((this._value + Number.EPSILON) * 100)}%` : '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 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 to add Number.EPSILON to the fractional
value before multiplying by 100, so values such as setValue(29 / 200) round to
15%. Preserve the existing conditional label behavior and rounding.
4 tasks
This was referenced Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes real bugs found in the codebase:
x === trueis equivalent tox(andx === falseto!x), and shorter to read.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).Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3519
Summary by CodeRabbit