feat: indicator classNames and sidebar overflow#800
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR changes the Indicator API to replace a flat Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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}
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/raystack/components/indicator/indicator.tsx (1)
32-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve wrapper base styles when
classNameis passedOn line 43,
...propsis spread afterclassName={styles.wrapper}. SinceclassNameis not explicitly destructured, a consumer-providedclassNameprop is captured in...propsand will overridestyles.wrapper, dropping required wrapper styles.Suggested fix
export const Indicator = ({ classNames, + className, variant, label, children, 'aria-label': ariaLabel, ...props }: IndicatorProps) => { const accessibilityLabel = ariaLabel || label || `${variant} indicator`; return ( - <div className={styles.wrapper} {...props}> + <div {...props} className={[styles.wrapper, className].filter(Boolean).join(' ')}>🤖 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/raystack/components/indicator/indicator.tsx` around lines 32 - 43, The wrapper's base styles get overwritten because consumer `className` is captured in `...props`; update the Indicator component to explicitly destructure `className` (and keep existing `classNames`) from props, then merge it with `styles.wrapper` (e.g., via template string or a utility like clsx) and pass that merged value into the div's `className`, leaving the rest of `...props` spread afterward; reference: Indicator component, `styles.wrapper`, `className`, `classNames`, and `...props`.
🤖 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/raystack/components/indicator/indicator.tsx`:
- Around line 32-43: The wrapper's base styles get overwritten because consumer
`className` is captured in `...props`; update the Indicator component to
explicitly destructure `className` (and keep existing `classNames`) from props,
then merge it with `styles.wrapper` (e.g., via template string or a utility like
clsx) and pass that merged value into the div's `className`, leaving the rest of
`...props` spread afterward; reference: Indicator component, `styles.wrapper`,
`className`, `classNames`, and `...props`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1da9731f-3337-46e8-acff-9a34a59408d6
📒 Files selected for processing (3)
apps/www/src/content/docs/components/indicator/props.tspackages/raystack/components/indicator/indicator.tsxpackages/raystack/components/sidebar/sidebar.module.css
💤 Files with no reviewable changes (1)
- packages/raystack/components/sidebar/sidebar.module.css
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/www/src/content/docs/components/indicator/props.ts`:
- Around line 17-24: IndicatorProps currently exposes both legacy className and
the new classNames.container which confuses consumers; decide on a single
source-of-truth (either keep className or keep classNames.container) and update
the IndicatorProps declaration and docs accordingly: remove the redundant
property from the props/interface (or mark it deprecated with a clear note) and
update any JSDoc/comments so only the chosen symbol (IndicatorProps.className or
IndicatorProps.classNames.container) is documented; ensure consumers see one
styling API choice and include a short migration note if deprecating the other.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a3c1a860-3abe-4bea-acb9-7103be9bcdfd
📒 Files selected for processing (2)
apps/www/src/content/docs/components/indicator/props.tspackages/raystack/components/indicator/indicator.tsx
2 participants
Summary
classNamesslot prop toIndicator(currently exposes theindicatorslot) so consumers can target the inner badge without relying on the bareclassNamepass-through.props.ts) to replace the removedclassNameentry with the newclassNamesslot map.sidebar.module.cssto fix layout bleed.