fix(date-picker): resolve override of calendarProps className #3826
Conversation
WalkthroughThe changes introduce a significant update to the Changes
Possibly related PRs
Suggested reviewers
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
|
🦋 Changeset detectedLatest commit: 952d6dc 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 |
|
@alexbeno is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
![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
🧹 Outside diff range and nitpick comments (1)
packages/components/date-picker/src/use-date-picker.ts (1)
207-211: Approved: Improved class name handlingThe changes successfully address the PR objective by correctly merging the class names from
calendarProps.classNamesandclassNames. This ensures that user-defined styles are not overridden.However, for consistency, consider applying the same pattern to all properties in
classNames:classNames: { ...calendarProps.classNames, base: slots.calendar({class: cn(calendarProps?.classNames?.base, classNames?.calendar)}), content: slots.calendarContent({class: cn(calendarProps?.classNames?.content, classNames?.calendarContent)}), // Apply the same pattern to other properties }This would ensure uniform handling of all class name properties.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- .changeset/hungry-kangaroos-kick.md (1 hunks)
- packages/components/date-picker/src/use-date-picker.ts (2 hunks)
🔇 Additional comments (4)
.changeset/hungry-kangaroos-kick.md (2)
1-3: Verify the necessity of a major version bump.The changeset indicates a major version bump for
@nextui-org/date-picker. However, the PR objectives state that this change is not breaking. Major version bumps are typically reserved for breaking changes or significant new features.Could you please clarify why this change requires a major version bump? If it's not a breaking change, consider using a minor version bump instead.
5-5: LGTM! Clear and informative change description.The change description accurately reflects the issue that was fixed and the expected outcome. It aligns well with the PR objectives and provides clear information for users and developers.
packages/components/date-picker/src/use-date-picker.ts (2)
13-13: LGTM: Import ofcnfunctionThe addition of the
cnimport from "@nextui-org/theme" is appropriate and necessary for the changes made in thegetCalendarPropsfunction. This import allows for better class name handling, which aligns with the PR objective.
Line range hint
1-254: Summary: Successful resolution of calendarProps className overrideThe changes in this file effectively address the PR objective of resolving the override of calendarProps className. The implementation now correctly merges user-defined and default class names, enhancing the customization capabilities of the date-picker component.
Key improvements:
- Addition of the
cnutility function for class name merging.- Updated
getCalendarPropsfunction to usecnfor combining class names.These changes ensure that custom styles applied to the date-picker's calendar are no longer ignored, improving the component's flexibility and usability.
The implementation is sound, with only a minor suggestion for consistency in class name handling. Overall, this is a valuable enhancement that aligns well with the PR's goals.
|
handled in #3773 already. |
📝 Description
This PR fixes an issue where the classNames property in the date-picker's calendarProps was being overridden, preventing custom styles from being applied.
⛳️ Current behavior (updates)
Currently, any custom styles set for the date-picker's calendar are ignored due to the classNames being overridden.
🚀 New behavior
With this change, the classNames property in calendarProps is now respected, allowing developers to apply custom styles as intended.
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
Summary by CodeRabbit
New Features
Bug Fixes