[#156] Replace text asterisks with SVG star icons#166
Conversation
- New StarRating.tsx with StarDisplay (fractional fill via clip-path) and StarInput (hover + click interactive stars) - RatingWidget: star icons for input (24px), average (18px), and recent ratings (12px) — replaces number buttons and text asterisks - RatingSummary: star icons (14px) with fractional fill support Fixes #156 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
T2b APPROVE
Clean component design:
- StarIcon — SVG with clip-path for fractional fill. Properly separates outline (muted) and fill (accent) layers.
- StarDisplay — Fractional fill math (
Math.min(1, Math.max(0, rating - (star - 1)))) is correct. - StarInput — Hover preview + click interaction with proper aria-labels and disabled state.
- Integration — RatingWidget cleaned up nicely (-33 lines), RatingSummary simplified. Size params used consistently (24/18/14/12px).
Minor note (non-blocking): Math.random() for clip-path IDs in StarIcon is technically not SSR-safe (different IDs on server vs client). Not blocking because StarDisplay is only rendered after client-side data fetches complete, so fractional stars won't be server-rendered in practice. Could use useId() if this ever changes.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The move to a shared SVG star component is directionally correct, but the current implementation introduces a hydration risk in the new rating UI. The fractional-fill clipPath ids are generated with Math.random() during render, which is not stable across server and client output.
Findings
- [medium]
StarIconusesMath.random()to generate the SVGclipPathid at render time, so the server-rendered id and the client-hydrated id can diverge and break hydration or clip-path references.- File:
src/components/StarRating.tsx:12 - Suggestion: Replace the random id with a stable React-generated id such as
useId(), or another deterministic per-instance id source that survives SSR hydration.
- File:
Decision
Requesting changes because the new shared star component currently introduces non-deterministic render output in a client-rendered surface. Once the clipPath id generation is made stable, the rest of the refactor looks appropriately scoped to #156.
Replace Math.random() with React useId() for deterministic SVG clipPath IDs, preventing hydration mismatches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up commit fixes the hydration risk in the shared star component by replacing render-time randomness with deterministic useId()-based clip-path ids. The rating UI refactor now looks clean, contained, and validated.
Findings
- [info] No remaining blocking findings after
13ee003.- File:
src/components/StarRating.tsx:61 - Suggestion: None.
- File:
Decision
Approving because the prior SSR/hydration issue is resolved, the shared star component remains appropriately scoped to #156, and lint-and-typecheck plus local typecheck, test, and build validation pass.
Summary
StarRating.tsxshared component withStarDisplay(fractional fill via SVG clip-path) andStarInput(hover + click interactive stars)Fixes #156
Test plan
tsc --noEmit— zero errorsnext build— cleanvitest run— 22/22 tests pass🤖 Generated with Claude Code