Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughA new Meter component is introduced to the raystack library with support for linear and circular variants. The component includes subcomponents (Label, Value, Track), context-based state sharing, comprehensive CSS styling for both variants, full test coverage, and detailed MDX documentation with multiple demo examples. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/raystack/components/meter/__tests__/meter.test.tsx (1)
88-97: Assert that only one track is rendered in this branch.This test is meant to protect the “custom children instead of implicit default track” path, but it currently only proves that a track exists. If the component rendered both tracks, this would still pass.
Suggested fix
it('renders custom children instead of default track', () => { const { container } = render( <Meter value={50}> <Meter.Label>Custom</Meter.Label> <Meter.Track /> </Meter> ); expect(screen.getByText('Custom')).toBeInTheDocument(); - expect(container.querySelector(`.${styles.track}`)).toBeInTheDocument(); + expect(container.querySelectorAll(`.${styles.track}`)).toHaveLength(1); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/meter/__tests__/meter.test.tsx` around lines 88 - 97, The test should assert that exactly one track is rendered to prevent both the implicit default and the provided <Meter.Track /> from coexisting; update the test in meter.test.tsx (the 'renders custom children instead of default track' case) to query all elements matching the track class (use container.querySelectorAll(`.${styles.track}`) or equivalent) and assert the node list length is 1, while keeping the existing asserts for the custom label and presence of a track.packages/raystack/components/meter/meter-root.tsx (1)
57-59: Consider removing redundant fallback.The
variant ?? 'linear'fallback on line 58 is redundant sincevariantalready defaults to'linear'in the destructuring on line 45.♻️ Proposed simplification
<MeterContext.Provider - value={{ variant: variant ?? 'linear', value, percentage }} + value={{ variant, value, percentage }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/meter/meter-root.tsx` around lines 57 - 59, Remove the redundant nullish fallback when providing context: in the MeterRoot component where MeterContext.Provider is given value={{ variant: variant ?? 'linear', value, percentage }}, drop the "?? 'linear'" because the component's props destructuring already sets variant = 'linear'; change the provider to pass variant directly (value={{ variant, value, percentage }}) so the default from the prop destructuring is used consistently.packages/raystack/components/meter/meter-track.tsx (1)
28-30: Indicator render prop discards incoming props.The
renderfunction forMeterPrimitive.Indicatorignores all props passed by the primitive. Compare with lines 21-25 wheretrackPropsare correctly spread onto the<svg>element. The indicator may not receive necessary attributes (e.g., ARIA props, data attributes) from the primitive.♻️ Proposed fix to forward indicator props
<MeterPrimitive.Indicator - render={() => <circle className={styles.circularIndicatorCircle} />} + render={(indicatorProps) => ( + <circle className={styles.circularIndicatorCircle} {...indicatorProps} /> + )} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/meter/meter-track.tsx` around lines 28 - 30, The Indicator's render prop currently ignores props from MeterPrimitive.Indicator; update the render to accept and forward those props to the DOM element so ARIA/data attributes and event handlers are preserved. Specifically, change the render used in MeterPrimitive.Indicator to a function like render={(indicatorProps) => <circle {...indicatorProps} className={styles.circularIndicatorCircle} />} so the incoming props are spread onto the <circle> while still applying the styles.circularIndicatorCircle class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/src/content/docs/components/meter/demo.ts`:
- Around line 11-20: The playground controls can produce min >= max and
out-of-range values; update the controls object in demo.ts so the 'max' control
always stays greater than 'min' and 'value' is clamped to [min, max] whenever
any control changes: enforce a relation when updating 'min' and 'max' (e.g., if
newMin >= currentMax, set max = newMin + 1; if newMax <= currentMin, set min =
newMax - 1), and ensure the 'value' control is adjusted/clamped to Math.min(max,
Math.max(min, value)) after min/max changes; also update the 'value' control's
metadata to reflect the current min/max bounds so the playground UI cannot set
values outside the valid range (referencing the controls object and the 'min',
'max', and 'value' fields and the meter-root calculation that divides by (max -
min)).
In `@apps/www/src/content/docs/components/meter/props.ts`:
- Around line 1-25: The MeterProps interface is missing documented customization
and accessibility hooks referenced in the docs; update the MeterProps interface
(and any related exported prop types) to include a trackStyle prop for styling
Meter.Track (e.g., trackStyle?: React.CSSProperties) and add accessibility props
ariaLabel?: string and ariaValueText?: string (or similarly named camelCase
variants used across the codebase) so the API tables match the documented usage
of Meter.Track style={...} and the aria-label / aria-valuetext props. Ensure the
new props are documented with the same JSDoc defaults/comments as the rest of
the interface.
In `@packages/raystack/components/meter/meter-root.tsx`:
- Line 54: Guard the percentage calc in meter-root.tsx against a zero
denominator by checking if max === min before computing percentage; if they are
equal, set percentage to 0 (or 100 when value >= max) to produce a stable CSS
value, otherwise compute percentage = ((value - min) * 100) / (max - min);
finally clamp the resulting percentage to the 0–100 range so the CSS variable
used by the meter never receives NaN/Infinity or out-of-range values.
In `@packages/raystack/components/meter/meter.module.css`:
- Around line 55-63: The .circularSvg rule violates Stylelint: add a blank line
before the width declaration and fix the broken operator/newline split in the
calc expression used for stroke-dashoffset (and the similar calc usage around
lines 81-84) so the operator isn't orphaned on a new line; locate the
.circularSvg block and the stroke-dashoffset declaration and reformat them to
have the empty line before width and a single-line or correctly spaced calc()
expression to satisfy the stylelint operator/newline rule.
- Around line 1-6: The .meter root class sets width: 100% which causes circular
meters to stretch; update the circular variant to reset that rule (e.g., in the
CSS rule that targets the circular variant such as the class used for circular
meters like .meter.circular or .meter--circular) by setting width: auto or
width: unset (and remove any conflicting inline or inherited width) so the
circular meter sizes to its SVG content rather than the full row.
---
Nitpick comments:
In `@packages/raystack/components/meter/__tests__/meter.test.tsx`:
- Around line 88-97: The test should assert that exactly one track is rendered
to prevent both the implicit default and the provided <Meter.Track /> from
coexisting; update the test in meter.test.tsx (the 'renders custom children
instead of default track' case) to query all elements matching the track class
(use container.querySelectorAll(`.${styles.track}`) or equivalent) and assert
the node list length is 1, while keeping the existing asserts for the custom
label and presence of a track.
In `@packages/raystack/components/meter/meter-root.tsx`:
- Around line 57-59: Remove the redundant nullish fallback when providing
context: in the MeterRoot component where MeterContext.Provider is given
value={{ variant: variant ?? 'linear', value, percentage }}, drop the "??
'linear'" because the component's props destructuring already sets variant =
'linear'; change the provider to pass variant directly (value={{ variant, value,
percentage }}) so the default from the prop destructuring is used consistently.
In `@packages/raystack/components/meter/meter-track.tsx`:
- Around line 28-30: The Indicator's render prop currently ignores props from
MeterPrimitive.Indicator; update the render to accept and forward those props to
the DOM element so ARIA/data attributes and event handlers are preserved.
Specifically, change the render used in MeterPrimitive.Indicator to a function
like render={(indicatorProps) => <circle {...indicatorProps}
className={styles.circularIndicatorCircle} />} so the incoming props are spread
onto the <circle> while still applying the styles.circularIndicatorCircle class.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db6ef343-434a-4b36-b6ac-ebcac84827cb
📒 Files selected for processing (11)
apps/www/src/content/docs/components/meter/demo.tsapps/www/src/content/docs/components/meter/index.mdxapps/www/src/content/docs/components/meter/props.tspackages/raystack/components/meter/__tests__/meter.test.tsxpackages/raystack/components/meter/index.tsxpackages/raystack/components/meter/meter-misc.tsxpackages/raystack/components/meter/meter-root.tsxpackages/raystack/components/meter/meter-track.tsxpackages/raystack/components/meter/meter.module.csspackages/raystack/components/meter/meter.tsxpackages/raystack/index.tsx
Comment on lines
+11
to
+20
| controls: { | ||
| value: { type: 'number', initialValue: 40, min: 0, max: 100 }, | ||
| variant: { | ||
| type: 'select', | ||
| initialValue: 'linear', | ||
| options: ['linear', 'circular'] | ||
| }, | ||
| min: { type: 'number', defaultValue: 0, min: 0, max: 99 }, | ||
| max: { type: 'number', defaultValue: 100, min: 1, max: 100 } | ||
| }, |
Contributor
There was a problem hiding this comment.
Constrain the playground controls to a valid range.
These controls can currently produce min >= max and values outside [min, max]. packages/raystack/components/meter/meter-root.tsx:54-68 divides by (max - min) directly, so the docs demo can end up rendering Infinity/NaN or percentages above 100. Please keep max > min and clamp value when the controls change so the playground can't enter a broken state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/www/src/content/docs/components/meter/demo.ts` around lines 11 - 20, The
playground controls can produce min >= max and out-of-range values; update the
controls object in demo.ts so the 'max' control always stays greater than 'min'
and 'value' is clamped to [min, max] whenever any control changes: enforce a
relation when updating 'min' and 'max' (e.g., if newMin >= currentMax, set max =
newMin + 1; if newMax <= currentMin, set min = newMax - 1), and ensure the
'value' control is adjusted/clamped to Math.min(max, Math.max(min, value)) after
min/max changes; also update the 'value' control's metadata to reflect the
current min/max bounds so the playground UI cannot set values outside the valid
range (referencing the controls object and the 'min', 'max', and 'value' fields
and the meter-root calculation that divides by (max - min)).
Comment on lines
+1
to
+25
| export interface MeterProps { | ||
| /** The current value of the meter. */ | ||
| value: number; | ||
|
|
||
| /** | ||
| * Minimum value. | ||
| * @default 0 | ||
| */ | ||
| min?: number; | ||
|
|
||
| /** | ||
| * Maximum value. | ||
| * @default 100 | ||
| */ | ||
| max?: number; | ||
|
|
||
| /** | ||
| * The visual style of the meter. | ||
| * @default "linear" | ||
| */ | ||
| variant?: 'linear' | 'circular'; | ||
|
|
||
| /** Additional CSS class name. */ | ||
| className?: string; | ||
| } |
Contributor
There was a problem hiding this comment.
Keep the generated API tables aligned with the documented surface.
The docs page demonstrates Meter.Track style={...} customization and explicitly calls out aria-label / aria-valuetext, but those props never appear in the interfaces that drive the API reference. Right now the generated tables under-document the main customization and accessibility hooks for this component.
Suggested fix
export interface MeterProps {
/** The current value of the meter. */
value: number;
@@
/** Additional CSS class name. */
className?: string;
+
+ /** Accessible name for screen readers. */
+ 'aria-label'?: string;
+
+ /** Screen-reader-specific text for the current value. */
+ 'aria-valuetext'?: string;
}
@@
export interface MeterTrackProps {
/** Additional CSS class name. */
className?: string;
+
+ /** Inline styles for sizing and CSS custom properties like `--rs-meter-track-size`. */
+ style?: React.CSSProperties;
}Also applies to: 40-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/www/src/content/docs/components/meter/props.ts` around lines 1 - 25, The
MeterProps interface is missing documented customization and accessibility hooks
referenced in the docs; update the MeterProps interface (and any related
exported prop types) to include a trackStyle prop for styling Meter.Track (e.g.,
trackStyle?: React.CSSProperties) and add accessibility props ariaLabel?: string
and ariaValueText?: string (or similarly named camelCase variants used across
the codebase) so the API tables match the documented usage of Meter.Track
style={...} and the aria-label / aria-valuetext props. Ensure the new props are
documented with the same JSDoc defaults/comments as the rest of the interface.
| }, | ||
| ref | ||
| ) => { | ||
| const percentage = ((value - min) * 100) / (max - min); |
Contributor
There was a problem hiding this comment.
Guard against division by zero when max === min.
If max equals min, the denominator becomes zero, resulting in NaN or Infinity. This will produce an invalid CSS variable value and break the meter rendering.
🛡️ Proposed fix to handle edge case
- const percentage = ((value - min) * 100) / (max - min);
+ const range = max - min;
+ const percentage = range > 0 ? ((value - min) * 100) / range : 0;📝 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 percentage = ((value - min) * 100) / (max - min); | |
| const range = max - min; | |
| const percentage = range > 0 ? ((value - min) * 100) / range : 0; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/meter/meter-root.tsx` at line 54, Guard the
percentage calc in meter-root.tsx against a zero denominator by checking if max
=== min before computing percentage; if they are equal, set percentage to 0 (or
100 when value >= max) to produce a stable CSS value, otherwise compute
percentage = ((value - min) * 100) / (max - min); finally clamp the resulting
percentage to the 0–100 range so the CSS variable used by the meter never
receives NaN/Infinity or out-of-range values.
Comment on lines
+1
to
+6
| .meter { | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: var(--rs-space-3); | ||
| width: 100%; | ||
| } |
Contributor
There was a problem hiding this comment.
Reset the root width for the circular variant.
.meter forces width: 100%, and the circular override never undoes it. That makes a circular meter take the full row width instead of matching its SVG footprint, which is especially noticeable in the horizontal demos. A circular meter should size to its content here.
Suggested fix
.meter-variant-circular {
+ width: fit-content;
align-items: center;
justify-content: center;
position: relative;
}Also applies to: 49-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/meter/meter.module.css` around lines 1 - 6, The
.meter root class sets width: 100% which causes circular meters to stretch;
update the circular variant to reset that rule (e.g., in the CSS rule that
targets the circular variant such as the class used for circular meters like
.meter.circular or .meter--circular) by setting width: auto or width: unset (and
remove any conflicting inline or inherited width) so the circular meter sizes to
its SVG content rather than the full row.
Comment on lines
+55
to
+63
| .circularSvg { | ||
| --rs-meter-track-size: 4px; | ||
| --rs-meter-radius: calc((72px - var(--rs-meter-track-size) * 2) / 2); | ||
| --rs-meter-circumference: calc(2 * 3.14159265 * var(--rs-meter-radius)); | ||
| width: 72px; | ||
| height: 72px; | ||
| aspect-ratio: 1; | ||
| transform: rotate(-90deg); | ||
| } |
Contributor
There was a problem hiding this comment.
This block currently fails the reported Stylelint rules.
The formatting here matches the static-analysis errors: missing empty line before width and the operator/newline split inside stroke-dashoffset. If lint runs in CI, this file will keep failing until those are normalized.
Suggested fix
.circularSvg {
--rs-meter-track-size: 4px;
--rs-meter-radius: calc((72px - var(--rs-meter-track-size) * 2) / 2);
--rs-meter-circumference: calc(2 * 3.14159265 * var(--rs-meter-radius));
+
width: 72px;
height: 72px;
aspect-ratio: 1;
transform: rotate(-90deg);
}
@@
.circularIndicatorCircle {
stroke: var(--rs-color-background-accent-emphasis);
stroke-dasharray: var(--rs-meter-circumference);
- stroke-dashoffset: calc(
- var(--rs-meter-circumference) *
- (1 - var(--rs-meter-percentage, 0) / 100)
- );
+ stroke-dashoffset: calc(var(--rs-meter-circumference) * (1 - var(--rs-meter-percentage, 0) / 100));
stroke-linecap: butt;
transition: stroke-dashoffset 500ms;
}Also applies to: 81-84
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 59-59: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/meter/meter.module.css` around lines 55 - 63,
The .circularSvg rule violates Stylelint: add a blank line before the width
declaration and fix the broken operator/newline split in the calc expression
used for stroke-dashoffset (and the similar calc usage around lines 81-84) so
the operator isn't orphaned on a new line; locate the .circularSvg block and the
stroke-dashoffset declaration and reformat them to have the empty line before
width and a single-line or correctly spaced calc() expression to satisfy the
stylelint operator/newline rule.
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.
Summary
Metercomponent withlinearandcircularvariantsMeter.Root,Meter.Track,Meter.Label,Meter.ValueviewBoxscaling — size controlled via CSSwidth/height, stroke via--rs-meter-track-sizevariable--rs-meter-track-size--rs-meter-percentageCSS variable set on rootSummary by CodeRabbit
New Features
Documentation
Tests