fix(alert): slots & variantProps import #4365
Conversation
🦋 Changeset detectedLatest commit: 0d429c4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request addresses an issue with the Alert component in the NextUI library by updating the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
Documentation and Community
|
![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 (1)
packages/components/alert/src/use-alert.ts (1)
Line range hint
73-73: Add JSDoc documentation for the classNames propertyWhile the example in the code comment is helpful, consider adding a brief description of the property's purpose in JSDoc format, consistent with other properties in the interface.
* }} /> * ``` */ + /** + * Classnames for each slot of the Alert component. + * Use this prop to apply custom styles to specific parts of the component. + */ classNames?: SlotsToClasses<AlertSlots>;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/polite-dingos-call.md(1 hunks)packages/components/alert/src/use-alert.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/polite-dingos-call.md
🔇 Additional comments (3)
packages/components/alert/src/use-alert.ts (3)
2-2: LGTM: Import consolidation improves code organization
The consolidation of type imports from @nextui-org/theme package improves code organization and maintainability.
Line range hint 77-77: LGTM: Type definition properly combines Props and AlertVariantProps
The type composition ensures proper typing for both component props and variant props.
Line range hint 142-147: LGTM: Consistent and type-safe implementation of classNames
The implementation correctly handles the optional classNames prop and consistently applies it across all slots.
Let's verify that all slots are properly typed and no slots are missing:
Also applies to: 151-156, 160-165, 169-174, 176-181, 183-188
✅ Verification successful
Let me search for the AlertSlots type definition to ensure we have all the information needed.
Let me check the alert theme function to see all the slots it defines.
Let me fix the regex syntax:
All Alert slots are properly implemented and type-safe
The implementation correctly handles all seven slots defined in the theme package:
- base
- mainWrapper
- description
- title
- closeButton
- alertIcon
- iconWrapper
Each slot is consistently implemented with proper optional chaining for classNames, and the usage matches exactly with the slots defined in the theme package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all Alert slots defined in the theme package are handled in the hook
# Search for Alert slots definition in the theme package
ast-grep --pattern 'export type AlertSlots = {
$$$
}'
# Search for all slot usage in the current file
rg 'slots\.[a-zA-Z]+\(' packages/components/alert/src/use-alert.ts
Length of output: 586
Script:
#!/bin/bash
# Search for AlertSlots type definition in a more flexible way
rg -A 10 "AlertSlots"
Length of output: 2021
Script:
#!/bin/bash
# Search for the alert theme function definition
rg -A 20 "const alert = tv\({" packages/core/theme/src/components/alert.ts
Length of output: 178
Script:
#!/bin/bash
# Search for the alert theme function definition
rg -A 20 "const alert = tv" packages/core/theme/src/components/alert.ts
Length of output: 828
Closes #4336
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information