[alexandrumorariu]

Checklist

• I have read and understand the contribution guide
• A CHANGELOG entry is included
• At least one test case is included for this feature or bug fix
• I have run automated testing and it is passed
• Documentation was added or is not needed
• This is an API breaking change

Issue Resolved / Feature Added

I added a StarRatings component to limestone.

Resolution

Additional Considerations

Links

NXT-20677

Comments

[codecov]

Codecov Report

❌ Patch coverage is 94.73684% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.70%. Comparing base (dcdb8df) to head (e00fa7e).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
StarRatings/StarRatings.js	92.68%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #449      +/-   ##
===========================================
+ Coverage    83.46%   83.70%   +0.23%     
===========================================
  Files          157      159       +2     
  Lines         7864     8003     +139     
  Branches      2432     2491      +59     
===========================================
+ Hits          6564     6699     +135     
- Misses        1010     1014       +4     
  Partials       290      290              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[daniel-stoian-lgp]

pointer mode does not behave as expected
Spec mentions that the also the all stars until the pointer position get focus and then, on click, the new value gets saved
But at the current implementation, only the currently saved stars get focus state

[daniel-stoian-lgp]

Each star rounds only its own share of the value (shownValue - i).
Subtracting decimals in floating point often gives a number just below the halfway point, so it rounds down when it should round up. For example, for 2.55, star 3 gets 2.55 - 2 = 0.5499999999999998, Math.round(5.4999…) gives 5, and the star fills only 50%.

The spec says to round half up to one decimal place

Suggested fix: round the whole value once, then work in whole tenths so no further decimal subtraction happens:

computed: {
// rating in whole tenths (0–50), rounded half-up per spec: 2.54 → 25, 2.55 → 26
tenths: ({active, displayValue, max, value}) => {
const shown = (active ? value : displayValue) || 0;
return Math.min(Math.max(Math.round(shown * 10), 0), max * 10);
}
},

render: ({tenths, ...rest}) => (
// ...
{Array.from({length: max}, (_, i) => {
const starValue = Math.min(Math.max(tenths - i * 10, 0), 10) / 10;
// ...
})}
)

[daniel-stoian-lgp]

please also add a unit test for the spec examples (2.54 → 2.5 filled, 2.55 → 2.6 filled), to stop this from coming back.

[daniel-stoian-lgp]

same comment as on starRatings:163

[daniel-stoian-lgp]

let's add TODOs for JSDocs, so we don't forget them

[daniel-stoian-lgp]

let's add TODOs for JSDocs, so we don't forget them

[daniel-stoian-lgp]

The StarRatings story only has controls for active and displayValue, and displayValue is limited to 0–5. There’s no control for value, defaultValue or max.

Becuase of this, there are problems which the storybook does not show directly:

No defaultValue gives 0 stars. You can see this one in the sampler: switch active on, and all five stars are empty.
The story passes no value or defaultValue.
Changeable then passes value as undefined.
shownValue turns that into 0 (StarRatings.js:147).

The spec’s lowest selectable value is 1, with 3 recommended as the default. Either the component should default to 3, or the docs and story should tell apps to pass defaultValue={3}.

[daniel-stoian-lgp]

I'm not sure we would need a story for this component. TBD in part 2

[daniel-stoian-lgp]

1. we need unit tests - there are none created. These don't depend on ux guide, so please add them
2. in RTL, the star position are correct, but the decimal value is still LTR

[daniel-stoian-lgp]

check my comments

[daniel-stoian-lgp]

Suggestion: use one value prop and a readOnly flag instead of active + displayValue for StarRating

Right now the mode decides which prop is read: active={false} shows displayValue, and active={true} shows value. This causes a few problems:

Apps need two props for one number. A screen showing an app’s rating and a popup letting the user rate it are both showing “the rating”, but one uses displayValue and the other value. Switching modes means renaming the prop. If the wrong one is passed, the stars are silently empty, with no warning.

The unused prop is still accepted. and both show 0 stars with no hint why.

• active already means something else in limestone. In Slider, it means the knob is grabbed and moves with 5-way . In InputField/NumberField, it means the input is focused for editing. Here it means the component can be edited at all.
  Changeable only manages value. displayValue is outside it, so the two modes behave differently for controlled vs. uncontrolled use.

Proposal:

// display (spec: 0.0–5.0, one decimal, max fixed at 5)
< StarRatings readOnly value={3.3} />

// selecting 
<StarRatings defaultValue={3} onChange={…} />

value: the only rating prop, managed by Changeable (so defaultValue also works).
readOnly: true gives display mode. It is not focusable, not interactive, and allows decimals rounded half-up to one decimal place. false (default) gives selecting mode, which rounds to an integer and keeps the value between 1 and max. The name
displayValue and active are removed.