Enhance Pomodoro Timer UI and improve terminal UX#1926
Conversation
📝 WalkthroughWalkthroughThe Pomodoro timer example is refactored to introduce custom widgets: ChangesPomodoro Timer UI Refactor
Estimated code review effort: 3 (Moderate) | ~25 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
examples/pomodoro-timer/src/index.tsx (1)
440-486: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStatusBadge shows "FINISHED" while the toast simultaneously announces the new phase "started".
_updateDisplaysetsstatus = 'Finished'wheneverthis._toastTimer !== null(line 477-478)._toastTimeris set non-null on every phase transition — both work→break and break→work — via_showToast(), whose toast text reads">> Work/Break phase started! <<". For the full 3-second toast window after every transition, the badge contradicts the toast: one says the phase "started", the other says "FINISHED", even though the timer is actively counting down a fresh, running phase. This directly undermines the PR's stated goal of clear Running/Paused/Finished indicators.Separately,
if (this._toastTimer !== null) { clearTimeout(this._toastTimer); }(line 447-449) doesn't reset_toastTimertonullbefore the immediate_updateDisplay()call on line 451 — it's only reassigned afterward by the newsetTimeouton line 453, so the stale non-null handle is what_updateDisplay()observes at that instant too.Consider decoupling "toast currently showing" from the "Finished" badge state — e.g. only mark
Finishedwhen the countdown itself reaches zero at the exact transition instant, not for the full toast display duration of the next phase.🤖 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` around lines 440 - 486, The status logic in _updateDisplay is incorrectly using _toastTimer to mean “Finished,” which makes the StatusBadge show FINISHED during the 3-second phase-start toast even though the timer is running. Update _updateDisplay and the _showToast transition flow so the badge is driven by the actual timer state (_running and whether the countdown has reached zero), not by whether a toast is visible; keep _toastTimer only for toast display timing and avoid letting it affect Finished vs Running.
🧹 Nitpick comments (3)
examples/pomodoro-timer/src/index.tsx (3)
313-321: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFixed container
width: 54/height: 23conflicts with the "responds well to terminal resizing" objective.The linked issue explicitly asks that the layout respond well to terminal resizing, and this constructor block (part of this change) hardcodes absolute dimensions.
hintsBoxelsewhere in the same constructor already useswidth: '100%', showing percentage sizing is supported — the outer container could adopt the same approach (or clamp to terminal size) to actually satisfy this requirement instead of rendering a fixed-size box regardless of terminal dimensions.🤖 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` around lines 313 - 321, The constructor for the main container is using fixed absolute dimensions, which prevents the layout from adapting when the terminal is resized. Update the container setup in the class constructor so it uses responsive sizing like the existing hints box pattern (for example percentage-based width/height or terminal-clamped dimensions) instead of hardcoded values, while keeping the rest of the styling intact.
120-122: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStyle spread order lets callers override the widget's required fixed height.
super({ height: 5, ...style })(and the analogousheight: 1calls inGradientProgressBarandStatusBadge) puts the forced height before the spread, so anyheightpassed instylesilently overrides it.BigTimer._renderSelfunconditionally writes 5 rows — if height ever gets overridden smaller, digit rows past the content rect would be clipped/misaligned with no guard. Not triggered today (no caller currently passesheight), but it's a latent footgun for a value these widgets structurally depend on.🛡️ Proposed fix (apply the same pattern to all three constructors)
- super({ height: 5, ...style }); + super({ ...style, height: 5 });Also applies to: 163-166, 250-252
🤖 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` around lines 120 - 122, The fixed-height widgets are allowing caller styles to override their required height because the spread order in the constructors puts the default height before the incoming style. Update the constructors for BigTimer, GradientProgressBar, and StatusBadge so their required height is applied after merging style (or otherwise enforced), keeping the widget height immutable from callers and matching the assumptions in BigTimer._renderSelf.
103-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate/diverging color-threshold logic between
getDynamicColorandGradientProgressBar.getColorAtFraction.Both functions map a 0-1 fraction to a color using the same 0.25/0.50/0.75 breakpoints, but with different endpoints (
getDynamicColorends flat at#22c55e;getColorAtFractioninterpolates further to emerald#10b981past 0.75). At value=1, BigTimer/phaseLabel/border show#22c55ewhile the progress bar's tail and label show#10b981— a visible palette mismatch across widgets that are all meant to represent the same "elapsed fraction". Consider consolidating into one shared color-mapping utility to keep visuals consistent and avoid future drift.Also applies to: 191-219
🤖 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` around lines 103 - 113, The elapsed-fraction color logic is duplicated and has drifted between getDynamicColor and GradientProgressBar.getColorAtFraction, causing mismatched colors at the same fraction value. Unify the threshold-to-color mapping into a single shared utility and have both getDynamicColor and GradientProgressBar.getColorAtFraction use it so BigTimer, phaseLabel, border, and the progress bar all resolve the same palette at 0.25/0.50/0.75 and 1.0.
🤖 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`:
- Around line 428-433: The white flash assignment in the timer update logic uses
unnecessary `as const` assertions, which should be removed because
`this._bigTimer.style.fg` already accepts a `Color`. Update the `progress < 0.5`
branch in the `index.tsx` timer rendering code so the `white` color object is
expressed plainly, and keep the `getDynamicColor(elapsed)` path unchanged.
---
Outside diff comments:
In `@examples/pomodoro-timer/src/index.tsx`:
- Around line 440-486: The status logic in _updateDisplay is incorrectly using
_toastTimer to mean “Finished,” which makes the StatusBadge show FINISHED during
the 3-second phase-start toast even though the timer is running. Update
_updateDisplay and the _showToast transition flow so the badge is driven by the
actual timer state (_running and whether the countdown has reached zero), not by
whether a toast is visible; keep _toastTimer only for toast display timing and
avoid letting it affect Finished vs Running.
---
Nitpick comments:
In `@examples/pomodoro-timer/src/index.tsx`:
- Around line 313-321: The constructor for the main container is using fixed
absolute dimensions, which prevents the layout from adapting when the terminal
is resized. Update the container setup in the class constructor so it uses
responsive sizing like the existing hints box pattern (for example
percentage-based width/height or terminal-clamped dimensions) instead of
hardcoded values, while keeping the rest of the styling intact.
- Around line 120-122: The fixed-height widgets are allowing caller styles to
override their required height because the spread order in the constructors puts
the default height before the incoming style. Update the constructors for
BigTimer, GradientProgressBar, and StatusBadge so their required height is
applied after merging style (or otherwise enforced), keeping the widget height
immutable from callers and matching the assumptions in BigTimer._renderSelf.
- Around line 103-113: The elapsed-fraction color logic is duplicated and has
drifted between getDynamicColor and GradientProgressBar.getColorAtFraction,
causing mismatched colors at the same fraction value. Unify the
threshold-to-color mapping into a single shared utility and have both
getDynamicColor and GradientProgressBar.getColorAtFraction use it so BigTimer,
phaseLabel, border, and the progress bar all resolve the same palette at
0.25/0.50/0.75 and 1.0.
🪄 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: aa50b944-7f82-4369-9b22-fcbd38f2c45b
📒 Files selected for processing (1)
examples/pomodoro-timer/src/index.tsx
| // Flash the timer white then fade back to dynamic progress color | ||
| if (progress < 0.5) { | ||
| this._timerDisplay.style.fg = { type: 'named', name: 'white' }; | ||
| this._bigTimer.style.fg = { type: 'named' as const, name: 'white' as const }; | ||
| } else { | ||
| this._timerDisplay.style.fg = this._phase === 'work' | ||
| ? { type: 'named' as const, name: 'red' as const } | ||
| : { type: 'named' as const, name: 'green' as const }; | ||
| const elapsed = this._elapsedFraction(); | ||
| this._bigTimer.style.fg = getDynamicColor(elapsed); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file structure first.
ast-grep outline examples/pomodoro-timer/src/index.tsx --view expanded || true
# Read the relevant region around the cited lines.
sed -n '240,290p' examples/pomodoro-timer/src/index.tsx
echo '---'
sed -n '420,440p' examples/pomodoro-timer/src/index.tsx
# Locate Color type / fg declarations.
rg -n "type Color|interface Color|fg: Color|style\.fg|named.*white|named.*black" examples/pomodoro-timer/src/index.tsx . --glob '!**/node_modules/**'Repository: Karanjot786/TermUI
Length of output: 24943
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' packages/core/src/style/Color.ts
echo '---'
sed -n '150,210p' packages/core/src/style/Style.ts
echo '---'
sed -n '1,80p' examples/calculator/src/index.tsxRepository: Karanjot786/TermUI
Length of output: 8563
Remove the unnecessary as const assertions
this._bigTimer.style.fg is already typed as Color, so { type: 'named', name: 'white' } is enough here. The current assertions add noise and break the TypeScript guideline about inline-explained assertions.
🤖 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` around lines 428 - 433, The white
flash assignment in the timer update logic uses unnecessary `as const`
assertions, which should be removed because `this._bigTimer.style.fg` already
accepts a `Color`. Update the `progress < 0.5` branch in the `index.tsx` timer
rendering code so the `white` color object is expressed plainly, and keep the
`getDynamicColor(elapsed)` path unchanged.
Source: Coding guidelines
Karanjot786
added
gssoc:approved
2 participants
Description
This PR enhances the Pomodoro Timer example by improving its terminal UI and overall user experience while preserving the existing timer functionality. The update introduces a cleaner layout, improved visual hierarchy, dynamic progress states, refined status indicators, and better keyboard shortcut presentation. All changes are strictly limited to
examples/pomodoro-timer.Related Issue
Closes #1925
Which package(s)?
examples/pomodoro-timerType 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
You are a GSSoC 2026 contributor.
Your GSSoC profile:
https://gssoc.girlscript.org/profile/79bbc5b9-0fdc-457e-b8cb-23220ab16312Screenshots / Recordings (UI changes)
Notes for the Reviewer
examples/pomodoro-timer.Summary by CodeRabbit
New Features
Bug Fixes