Add styling to the alert component #15470
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe pull request introduces styling enhancements for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/connect-react/src/components/Alert.tsx (4)
9-16: Consider enhancing the base styles for better theme consistency.While the current styles are good, consider these improvements:
- Use CSS variables for colors to maintain theme consistency
- Add a subtle border for better visual separation
const baseStyles = { - background: "#e2e3e5", + background: "var(--pd-alert-background, #e2e3e5)", borderRadius: "10px", paddingTop: "2px", paddingLeft: "10px", paddingRight: "10px", paddingBottom: "2px", + border: "1px solid var(--pd-alert-border, rgba(0,0,0,0.1))", }
18-36: Consider accessibility and semantic color naming.The alert styles should ensure sufficient contrast ratios for accessibility and use semantic color naming.
const warningStyles = { ...baseStyles, - background: "#fff3cd", + background: "var(--pd-alert-warning-bg, #fff3cd)", + color: "var(--pd-alert-warning-text, #664d03)", }; const infoStyles = { ...baseStyles, - background: "#d1ecf1", + background: "var(--pd-alert-info-bg, #d1ecf1)", + color: "var(--pd-alert-info-text, #055160)", } const errorStyles = { ...baseStyles, - background: "#f8d7da", + background: "var(--pd-alert-error-bg, #f8d7da)", + color: "var(--pd-alert-error-text, #842029)", } const neutralStyles = { ...baseStyles, - background: "#fffff2", + background: "var(--pd-alert-neutral-bg, #fffff2)", + color: "var(--pd-alert-neutral-text, #1f2937)", }
38-54: Consider simplifying style selection with an object lookup.The switch statement could be replaced with a more concise object lookup pattern.
-let alertStyles = {} -switch (prop.alertType) { -case "info": - alertStyles = infoStyles - break -case "neutral": - alertStyles = neutralStyles - break -case "warning": - alertStyles = warningStyles - break -case "error": - alertStyles = errorStyles - break -default: - alertStyles = baseStyles -} +const styleMap = { + info: infoStyles, + neutral: neutralStyles, + warning: warningStyles, + error: errorStyles, +}; +const alertStyles = styleMap[prop.alertType] || baseStyles;
56-56: Enhance accessibility with ARIA attributes.Add appropriate ARIA attributes and roles for better screen reader support.
-return <div className={`pd-alert-${prop.alertType}`} style={alertStyles}><Markdown>{prop.content}</Markdown></div> +return ( + <div + role="alert" + aria-live="polite" + className={`pd-alert-${prop.alertType}`} + style={alertStyles} + > + <Markdown>{prop.content}</Markdown> + </div> +)packages/connect-react/CHANGELOG.md (1)
4-7: Consider adding more details to the changelog entry.The current entry could be more descriptive about the specific styling changes and Markdown support added.
# [1.0.0-preview.27] - 2025-01-30 -Add styling to alerts +- Add styled alert components with support for warning, info, error, and neutral types +- Add Markdown rendering support for alert content +- Implement consistent styling with customizable backgrounds and borders
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/connect-react/examples/nextjs/package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/connect-react/CHANGELOG.md(1 hunks)packages/connect-react/package.json(1 hunks)packages/connect-react/src/components/Alert.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/connect-react/package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
packages/connect-react/src/components/Alert.tsx (1)
1-8: LGTM! Good addition of Markdown support.Adding Markdown rendering capability enhances the alert content's formatting flexibility.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/connect-react/src/components/Description.tsx (2)
Line range hint
17-18: Address technical debt regarding component naming and context requirements.The XXX comment raises valid concerns about component naming and context usage. A more specific name like
FieldDescriptionwould better communicate the component's requirements and prevent misuse.Would you like me to help create an issue to track:
- Renaming the component to
FieldDescription- Documenting context requirements
- Reviewing usage in components like
OptionalFieldButton
44-50: Format the JSX for better readability.While the implementation correctly handles external links with proper security attributes, the formatting could be improved for better readability.
Apply this formatting:
- return <div className={getClassNames("description", props)} style={getStyles("description", baseStyles, props)}> <Markdown components={{ - a: ({ ...props }) => { - return <a {...props} target="_blank" rel="noopener noreferrer" />; - }, - }}> - {markdown} - </Markdown></div>; + return ( + <div + className={getClassNames("description", props)} + style={getStyles("description", baseStyles, props)} + > + <Markdown + components={{ + a: ({ ...props }) => ( + <a {...props} target="_blank" rel="noopener noreferrer" /> + ), + }} + > + {markdown} + </Markdown> + </div> + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/connect-react/examples/nextjs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
packages/connect-react/src/components/Alert.tsx(1 hunks)packages/connect-react/src/components/Description.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/connect-react/src/components/Alert.tsx
🧰 Additional context used
🪛 GitHub Actions: Pull Request Checks
packages/connect-react/src/components/Description.tsx
[error] 3-3: Expected a line break after this opening brace (object-curly-newline)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pnpm publish
bzwrk
deleted the
biz/dj-2852-alert-prop-type-is-unstyled-in-connect-react
branch
WHY
Summary by CodeRabbit
New Features
Version Update
1.0.0-preview.27