feat(ui): add Rating widget#728
Open
Satvik-art-creator wants to merge 3 commits into
Open
Conversation
Add a star-rating selector widget to @termuijs/ui that renders filled/empty star glyphs controlled by arrow keys. - right/left arrows adjust the value (capped at 0..max) - enter fires onChange with the current value - home/end jump to min/max - readonly mode disables key input - caps.unicode glyph fallback (★☆ vs *-) - 15 tests covering all acceptance criteria Closes Karanjot786#258
github-actions
Bot
added
type:feature
📝 WalkthroughWalkthroughAdds a new Rating widget (keyboard-driven star selector) with tests and package exports, and refactors MeterOptions to simplified thresholds plus a new setLabel method. ChangesRating Widget
Meter Options Simplification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 `@packages/ui/src/Rating.test.ts`:
- Around line 37-50: The tests call Rating.handleKey directly; update them to
exercise the real widget lifecycle by creating a Screen from `@termuijs/core`,
mounting a Rating instance into that screen, calling updateRect() and render(),
then sending the key event (e.g., using the existing key('right')/key('left')
helper) through the Screen so the Rating receives it via its normal input path;
after render/update assert Rating.value. Apply the same pattern for the other
tests referenced (lines ~52-58, 79-109, 123-129) using Rating, Screen,
updateRect, and render instead of directly invoking handleKey.
In `@packages/ui/src/Rating.ts`:
- Around line 19-27: Update the Rating public API to match the stated contract
by adding a compatibility layer: modify the constructor of the Rating class to
accept constructor(style?, opts?) (i.e., support an optional first style arg and
an options object second) and map that into the existing internal initialization
that currently consumes constructor(options); add an onSelect property that
aliases/forwards to the existing onChange callback (so both names work);
implement setValue(value) and getValue() methods that delegate to the existing
value setter/getter property accessors; keep the existing readonly and onChange
behavior intact so existing consumers still work.
🪄 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: 87f07288-0ab6-4c00-947d-818790d72895
📒 Files selected for processing (3)
packages/ui/src/Rating.test.tspackages/ui/src/Rating.tspackages/ui/src/index.ts
…uplicate export Rating (packages/ui): - constructor(style?, opts?) to match issue Karanjot786#258 API contract - rename onChange to onSelect per spec - add setValue()/getValue() methods with markDirty() - add filledChar/emptyChar/filledColor options - remove out-of-scope readonly mode - add full widget lifecycle (Screen+updateRect+render) to all key tests Meter (packages/widgets): - remove duplicate class definition causing Multiple exports build error - fix threshold color logic to match test expectations All 2019 tests pass. Closes Karanjot786#258
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/Rating.ts (1)
125-135:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
filledColoroption is currently ignored in rendering.Line [125]-Line [135] always render with
styleToCellAttrs(this.style), sofilledColorset at Line [66] never affects filled stars. This makes part of the public API non-functional.Suggested patch
protected _renderSelf(screen: Screen): void { const { x, y, width } = this._rect; if (width <= 0) return; const attrs = styleToCellAttrs(this.style); const filled = this._filledChar ?? (caps.unicode ? '★' : '*'); const empty = this._emptyChar ?? (caps.unicode ? '☆' : '-'); - let text = ''; - for (let i = 0; i < this._max; i++) { - text += i < this._value ? filled : empty; - } - - screen.writeString(x, y, text.slice(0, width), attrs); + const visibleCount = Math.min(this._max, width); + const filledCount = Math.min(this._value, visibleCount); + const emptyCount = visibleCount - filledCount; + + const filledAttrs = this._filledColor === undefined + ? attrs + : { ...attrs, fg: this._filledColor }; + + if (filledCount > 0) { + screen.writeString(x, y, filled.repeat(filledCount), filledAttrs); + } + if (emptyCount > 0) { + screen.writeString(x + filledCount, y, empty.repeat(emptyCount), attrs); + } }🤖 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/ui/src/Rating.ts` around lines 125 - 135, The render currently always uses styleToCellAttrs(this.style) so the filledColor option is ignored; update the render logic in Rating (the block using styleToCellAttrs, filled/empty chars, and screen.writeString) to derive separate attrs for filled and empty characters (e.g., attrsFilled = styleToCellAttrs({ ...this.style, color: this.filledColor }) and attrsEmpty = styleToCellAttrs(this.style) or by overriding the foreground color in the attrs), then write the filled and empty runs with their respective attrs (either by building runs and calling screen.writeString per run or using whatever per-char attr API exists) instead of a single attrs for the whole text.
🤖 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.
Outside diff comments:
In `@packages/ui/src/Rating.ts`:
- Around line 125-135: The render currently always uses
styleToCellAttrs(this.style) so the filledColor option is ignored; update the
render logic in Rating (the block using styleToCellAttrs, filled/empty chars,
and screen.writeString) to derive separate attrs for filled and empty characters
(e.g., attrsFilled = styleToCellAttrs({ ...this.style, color: this.filledColor
}) and attrsEmpty = styleToCellAttrs(this.style) or by overriding the foreground
color in the attrs), then write the filled and empty runs with their respective
attrs (either by building runs and calling screen.writeString per run or using
whatever per-char attr API exists) instead of a single attrs for the whole text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 53fa26bc-8ae7-4064-a7a5-c2aad69e63eb
📒 Files selected for processing (3)
packages/ui/src/Rating.test.tspackages/ui/src/Rating.tspackages/widgets/src/data/Meter.ts
Contributor
Author
|
Hi @Karanjot786, While working on the Rating widget (#258), I discovered a pre-existing bug in I filed a separate bug report for this (issue #737) but had to close it temporarily because my Rating PR was still open — if someone commented on the Meter issue, the bot would auto-assign it to them before I could work on it. Could you please reopen issue #737 after this PR is merged? I'd like to submit a fix for the Meter duplicate as a separate PR scoped to Thanks! |
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.
Related Issue
Closes #258
Summary
Add a star-rating selector widget to
@termuijs/uithat renders filled/empty star glyphs controlled by arrow keys.Which package(s) does this affect?
@termuijs/uiType of Change
Changes
[NEW]
packages/ui/src/Rating.tsRatingclass extendingWidgetwith:constructor(style?, opts?)matching the issue API contractsetValue(value)/getValue()methods;setValuecallsmarkDirty()valueandmaxgettersonSelectcallback fired onenterhandleKey()supportingright,left,home,end,enter_renderSelf()withcaps.unicodefallback (★☆vs*-)filledChar,emptyChar,filledColoroptionsfocusable = true[NEW]
packages/ui/src/Rating.test.tsonSelectwith current value*and-setValueclamps to max and callsmarkDirty()getValue()Screen+updateRect()+render())createKeyEventfrom@termuijs/corefor properKeyEventconstructionvi.spyOnforcaps, never mutates shared state[MODIFY]
packages/ui/src/index.tsRatingandRatingOptionsexportsChecklist
packages/uionlyindex.tsToggle.ts)bun vitest run packages/uipasses (46 files, 333+ tests)bun.lockchangespackages/uiScreenfrom@termuijs/corevi.spyOnforcaps, never direct mutationVerification
bun vitest run packages/ui— all test files passbun vitest run packages/ui/src/Rating.test.ts— 15/15 passedgit diff upstream/main --statconfirms only 3 files inpackages/uiGSSoC 2026