Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR introduces a new Progress component to the Raystack UI library with full implementation, documentation, and test coverage. It includes component implementation using a compound component pattern with context API, CSS styling for linear and circular variants, comprehensive prop type definitions, an interactive documentation page with multiple demo examples, and a test suite covering various component states and accessibility features. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/raystack/components/progress/progress-root.tsx`:
- Around line 47-71: Normalize and validate inputs before computing percentage:
clamp value between min and max, ensure the denominator (max - min) is positive
(fallback to 1 if max <= min) to avoid division by zero/negative, compute
percentage = ((clampedValue - min) * 100) / safeDenominator, then clamp the
resulting percentage to 0–100 and use that bounded percentage when setting
ProgressContext (value/percentage) and the CSS variable sent into
ProgressPrimitive.Root; update the logic around percentage calculation in the
component that provides ProgressContext and renders ProgressPrimitive.Root (the
percentage/value handling in this component).
In `@packages/raystack/components/progress/progress-track.tsx`:
- Around line 15-31: The circular branch of ProgressPrimitive.Track is accepting
full Track props but then hard-codes render and injects children into an SVG,
which lets consumers pass a non-SVG `render` or regular HTML children that will
be ignored or produce invalid SVG; fix by narrowing the props passed into
ProgressPrimitive.Track when variant === 'circular' so `render` and non-SVG
`children` are not accepted (e.g., explicitly pick/omit keys: remove `render`
and `children` from the spread into ProgressPrimitive.Track), or else move any
non-SVG `children` outside the <svg> host and only pass SVG-safe props (or cast
to an appropriate SVG prop type) to ProgressPrimitive.Track/Indicator; update
the code around ProgressPrimitive.Track and ProgressPrimitive.Indicator usage to
reflect the narrowed prop set.
In `@packages/raystack/components/progress/progress.module.css`:
- Around line 55-63: The stylelint errors are caused by inconsistent declaration
spacing and multiline calc() formatting in .circularSvg; normalize by making
each custom property declaration use a single space after the colon and format
the calc() expressions as single-line with spaces around operators (e.g.,
--rs-progress-radius: calc((72px - var(--rs-progress-track-size) * 2) / 2); and
--rs-progress-circumference: calc(2 * 3.14159265 * var(--rs-progress-radius));),
and ensure the same spacing convention is applied to the other calc()
declarations referenced elsewhere (the similar declarations around lines 81–84).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b9d309e-ecaf-486e-b178-7ec52c543ab0
📒 Files selected for processing (11)
apps/www/src/content/docs/components/progress/demo.tsapps/www/src/content/docs/components/progress/index.mdxapps/www/src/content/docs/components/progress/props.tspackages/raystack/components/progress/__tests__/progress.test.tsxpackages/raystack/components/progress/index.tsxpackages/raystack/components/progress/progress-misc.tsxpackages/raystack/components/progress/progress-root.tsxpackages/raystack/components/progress/progress-track.tsxpackages/raystack/components/progress/progress.module.csspackages/raystack/components/progress/progress.tsxpackages/raystack/index.tsx
Comment on lines
+47
to
+71
| value = 0, | ||
| min = 0, | ||
| max = 100, | ||
| ...props | ||
| }, | ||
| ref | ||
| ) => { | ||
| const percentage = value === null ? 0 : ((value - min) * 100) / (max - min); | ||
|
|
||
| return ( | ||
| <ProgressContext.Provider | ||
| value={{ variant: variant ?? 'linear', value, percentage }} | ||
| > | ||
| <ProgressPrimitive.Root | ||
| ref={ref} | ||
| className={progress({ variant, className })} | ||
| style={ | ||
| { | ||
| ...style, | ||
| '--rs-progress-percentage': percentage | ||
| } as React.CSSProperties | ||
| } | ||
| value={value} | ||
| min={min} | ||
| max={max} |
Contributor
There was a problem hiding this comment.
Clamp and validate the range before computing percentage.
This derived value is fed straight into --rs-progress-percentage. For determinate progress, max <= min produces invalid math, and out-of-range values can drive the indicator below 0% or above 100%.
🛡️ One safe way to normalize the inputs
- const percentage = value === null ? 0 : ((value - min) * 100) / (max - min);
+ if (value !== null && max <= min) {
+ throw new Error('Progress requires max to be greater than min');
+ }
+
+ const normalizedValue =
+ value === null ? null : Math.min(Math.max(value, min), max);
+ const percentage =
+ normalizedValue === null ? 0 : ((normalizedValue - min) * 100) / (max - min);
@@
- value={{ variant: variant ?? 'linear', value, percentage }}
+ value={{
+ variant: variant ?? 'linear',
+ value: normalizedValue,
+ percentage
+ }}
@@
- value={value}
+ value={normalizedValue}📝 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
| value = 0, | |
| min = 0, | |
| max = 100, | |
| ...props | |
| }, | |
| ref | |
| ) => { | |
| const percentage = value === null ? 0 : ((value - min) * 100) / (max - min); | |
| return ( | |
| <ProgressContext.Provider | |
| value={{ variant: variant ?? 'linear', value, percentage }} | |
| > | |
| <ProgressPrimitive.Root | |
| ref={ref} | |
| className={progress({ variant, className })} | |
| style={ | |
| { | |
| ...style, | |
| '--rs-progress-percentage': percentage | |
| } as React.CSSProperties | |
| } | |
| value={value} | |
| min={min} | |
| max={max} | |
| value = 0, | |
| min = 0, | |
| max = 100, | |
| ...props | |
| }, | |
| ref | |
| ) => { | |
| if (value !== null && max <= min) { | |
| throw new Error('Progress requires max to be greater than min'); | |
| } | |
| const normalizedValue = | |
| value === null ? null : Math.min(Math.max(value, min), max); | |
| const percentage = | |
| normalizedValue === null ? 0 : ((normalizedValue - min) * 100) / (max - min); | |
| return ( | |
| <ProgressContext.Provider | |
| value={{ | |
| variant: variant ?? 'linear', | |
| value: normalizedValue, | |
| percentage | |
| }} | |
| > | |
| <ProgressPrimitive.Root | |
| ref={ref} | |
| className={progress({ variant, className })} | |
| style={ | |
| { | |
| ...style, | |
| '--rs-progress-percentage': percentage | |
| } as React.CSSProperties | |
| } | |
| value={normalizedValue} | |
| min={min} | |
| max={max} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/progress/progress-root.tsx` around lines 47 -
71, Normalize and validate inputs before computing percentage: clamp value
between min and max, ensure the denominator (max - min) is positive (fallback to
1 if max <= min) to avoid division by zero/negative, compute percentage =
((clampedValue - min) * 100) / safeDenominator, then clamp the resulting
percentage to 0–100 and use that bounded percentage when setting ProgressContext
(value/percentage) and the CSS variable sent into ProgressPrimitive.Root; update
the logic around percentage calculation in the component that provides
ProgressContext and renders ProgressPrimitive.Root (the percentage/value
handling in this component).
Comment on lines
+15
to
+31
| if (variant === 'circular') { | ||
| return ( | ||
| <ProgressPrimitive.Track | ||
| ref={ref} | ||
| className={cx(styles.circularSvg, className)} | ||
| {...props} | ||
| render={({ children: trackChildren, ...trackProps }) => ( | ||
| <svg viewBox='0 0 72 72' {...trackProps}> | ||
| <circle className={styles.circularTrackCircle} /> | ||
| {trackChildren} | ||
| </svg> | ||
| )} | ||
| > | ||
| <ProgressPrimitive.Indicator | ||
| render={() => <circle className={styles.circularIndicatorCircle} />} | ||
| /> | ||
| {children} |
Contributor
There was a problem hiding this comment.
Don't expose linear-only Track behavior in circular mode.
This branch still accepts ProgressPrimitive.Track.Props, but it hard-codes render and moves children into an <svg>. A consumer can pass a custom render or regular HTML child that works in linear mode and is silently ignored / becomes invalid SVG content here. Please either narrow the wrapper props for the circular path or keep non-SVG children outside the SVG host.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/progress/progress-track.tsx` around lines 15 -
31, The circular branch of ProgressPrimitive.Track is accepting full Track props
but then hard-codes render and injects children into an SVG, which lets
consumers pass a non-SVG `render` or regular HTML children that will be ignored
or produce invalid SVG; fix by narrowing the props passed into
ProgressPrimitive.Track when variant === 'circular' so `render` and non-SVG
`children` are not accepted (e.g., explicitly pick/omit keys: remove `render`
and `children` from the spread into ProgressPrimitive.Track), or else move any
non-SVG `children` outside the <svg> host and only pass SVG-safe props (or cast
to an appropriate SVG prop type) to ProgressPrimitive.Track/Indicator; update
the code around ProgressPrimitive.Track and ProgressPrimitive.Indicator usage to
reflect the narrowed prop set.
Comment on lines
+55
to
+63
| .circularSvg { | ||
| --rs-progress-track-size: 4px; | ||
| --rs-progress-radius: calc((72px - var(--rs-progress-track-size) * 2) / 2); | ||
| --rs-progress-circumference: calc(2 * 3.14159265 * var(--rs-progress-radius)); | ||
| width: 72px; | ||
| height: 72px; | ||
| aspect-ratio: 1; | ||
| transform: rotate(-90deg); | ||
| } |
Contributor
There was a problem hiding this comment.
Fix the current Stylelint failures.
This rule set already trips the linter, so the PR will stay noisy until the declaration spacing and multiline calc() formatting are normalized.
🧹 Minimal lint-only cleanup
.circularSvg {
--rs-progress-track-size: 4px;
--rs-progress-radius: calc((72px - var(--rs-progress-track-size) * 2) / 2);
--rs-progress-circumference: calc(2 * 3.14159265 * var(--rs-progress-radius));
+
width: 72px;
height: 72px;
aspect-ratio: 1;
transform: rotate(-90deg);
}
@@
stroke: var(--rs-color-background-accent-emphasis);
stroke-dasharray: var(--rs-progress-circumference);
- stroke-dashoffset: calc(
- var(--rs-progress-circumference) *
- (1 - var(--rs-progress-percentage, 0) / 100)
- );
+ stroke-dashoffset: calc(
+ var(--rs-progress-circumference) * (1 - var(--rs-progress-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/progress/progress.module.css` around lines 55 -
63, The stylelint errors are caused by inconsistent declaration spacing and
multiline calc() formatting in .circularSvg; normalize by making each custom
property declaration use a single space after the colon and format the calc()
expressions as single-line with spaces around operators (e.g.,
--rs-progress-radius: calc((72px - var(--rs-progress-track-size) * 2) / 2); and
--rs-progress-circumference: calc(2 * 3.14159265 * var(--rs-progress-radius));),
and ensure the same spacing convention is applied to the other calc()
declarations referenced elsewhere (the similar declarations around lines 81–84).
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
Progresscomponent withlinearandcircularvariants, based on Base UIProgressprimitiveProgress.Root,Progress.Track,Progress.Label,Progress.ValueviewBoxscaling — size via CSSwidth/height, stroke via--rs-progress-track-sizeprogress.module.csswith--rs-progress-*prefixed CSS variablesSummary by CodeRabbit
New Features
Documentation
Tests