Cal #4859
Cal #4859
Conversation
… annotations for clarity
|
@1amageek is attempting to deploy a commit to the HeroUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: 2c2b85c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
WalkthroughThis pull request introduces custom cell content support for both Calendar and RangeCalendar components. Developers can now provide a custom renderer through a new prop ( Changes
Sequence Diagram(s)sequenceDiagram
actor Developer
participant Calendar
participant useCalendar
participant CalendarCell
participant CustomRenderer as "Custom Cell Renderer"
participant DefaultRenderer as "Default Renderer (CellContentDefault)"
Developer->>Calendar: Provide custom cellContent prop
Calendar->>useCalendar: Pass cellContent prop
useCalendar->>CalendarCell: Include cellContent in context
CalendarCell->>CalendarCell: Check if cellContent exists
alt Custom cellContent provided
CalendarCell->>CustomRenderer: Render custom cell using provided function
else No custom content
CalendarCell->>DefaultRenderer: Render default cell content
end
Suggested reviewers
✨ Finishing Touches
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: 1
🧹 Nitpick comments (19)
apps/docs/content/components/range-calendar/custom-cell-content.tsx (2)
3-5: Add a comment explaining the purpose of this mapping.While the code is correct, adding documentation would help other developers understand the purpose of this virtual file system mapping.
+// Maps virtual file paths to their raw content for the documentation playground const react = { "/App.jsx": App, };
7-9: Simplify the export statement.The spread operator is unnecessary when exporting a single object.
-export default { - ...react, -}; +export default react;packages/components/calendar/src/calendar-cell.tsx (1)
117-121: Enhance the custom renderer’s flexibility
Currently,cellContent(props.date)only provides the date. Consider also passing the full context for more advanced custom content (e.g.,cellContent(props.date, cellContextValue)), enabling developers to leverage selection states, hover states, etc.- {typeof cellContent === "function" - ? cellContent(props.date) - : cellContent ?? <CalendarCellContentDefault date={props.date} />} + {typeof cellContent === "function" + ? cellContent(props.date, cellContextValue) + : cellContent ?? <CalendarCellContentDefault date={props.date} />}packages/core/theme/src/components/calendar.ts (2)
133-133: Hidden disabled dates
WhenhideDisabledDates: true, the new ruledata-[disabled=true]:data-[outside-month=true]:opacity-0hides those dates. Ensure this does not conflict with any accessibility guidelines that might require uniform display of grid cells.
316-364: Color-coded range styling
These successive lines for primary, secondary, success, warning, and danger provide consistent theming for multi-range scenarios. Consider adding test coverage to confirm all color variants render as expected.packages/components/calendar/src/calendar-cell-content-default.tsx (1)
11-17: Consider memoizing the component for better performance.The implementation is clean, but since this is a presentational component that only depends on the
dateprop, it could benefit from memoization.-export const CalendarCellContentDefault: React.FC<CalendarCellContentDefaultProps> = ({date}) => { +export const CalendarCellContentDefault = React.memo<CalendarCellContentDefaultProps>(({date}) => { return ( <CalendarCellContent> <CalendarCellHeader>{date.day}</CalendarCellHeader> </CalendarCellContent> ); -}; +});packages/components/calendar/src/calendar-cell-content.tsx (1)
9-21: Consider adding ref forwarding for better component composition.The implementation is good, but adding ref forwarding would improve component composability.
-export const CalendarCellContent = ({children, ...props}: CalendarCellContentProps) => { +export const CalendarCellContent = React.forwardRef<HTMLDivElement, CalendarCellContentProps>( + ({children, ...props}, ref) => { const {slots, classNames} = useCalendarContext(); return ( <div className={slots?.cellContent({class: classNames?.cellContent})} data-slot="cell-content" + ref={ref} {...props} > {children} </div> ); -}; +});packages/components/calendar/src/calendar-cell-body.tsx (2)
1-11: Consider renaming interface for consistency.The generic
Propsinterface name could be more specific for better maintainability.-interface Props extends HTMLHeroUIProps<"div"> { +interface CalendarCellBodyProps extends HTMLHeroUIProps<"div"> { children: React.ReactNode; } -export type CalendarCellBodyProps = Props;
13-25: Consider reordering props for better override control.The props spreading order should allow overriding of default props.
const bodyProps = { - ...props, ref: ref, className: slots?.cellBody({class: classNames?.cellBody}), "data-slot": "cell-body", + ...props, };packages/components/calendar/src/index.ts (1)
3-5: Consider exporting types for new components.While the components are exported, their corresponding types should also be exported for consistency with existing type exports.
// export types export type {CalendarProps} from "./calendar"; export type {RangeCalendarProps} from "./range-calendar"; +export type {CalendarCellContentProps} from "./calendar-cell-content"; +export type {CalendarCellHeaderProps} from "./calendar-cell-header"; +export type {CalendarCellBodyProps} from "./calendar-cell-body"; export type {CalendarDate} from "@internationalized/date";Also applies to: 22-22
packages/components/calendar/src/calendar-cell-context.tsx (1)
7-38: LGTM! Well-structured interface with comprehensive state properties.The
CalendarCellContextTypeinterface is well-organized with clear categorization of states (selection, interaction, validation, display). The properties cover all necessary aspects of calendar cell customization.Consider adding JSDoc comments to document the purpose of each state category and their properties.
apps/docs/content/components/calendar/custom-cell-content.raw.jsx (2)
13-19: Inconsistent ARIA label casing.The
aria-labelfor the first event is "Calendar event" while others use "calendar event". Maintain consistency in ARIA labels.- aria-label="Calendar event" + aria-label="calendar event"
11-39: Refactor duplicate event span code.The event spans share similar structure and only differ in color classes. Consider extracting this into a reusable component.
const EventIndicator = ({ day, modulo, color }) => { if (day % modulo !== 0) return null; return ( <span aria-label="calendar event" className={`bg-${color}-500/20 w-full rounded-md px-1 text-${color}-400 line-clamp-1`} role="status" > MTG </span> ); };Then use it like:
<div className="flex flex-col w-full text-tiny gap-0.5 px-0.5"> <EventIndicator day={date.day} modulo={7} color="red" /> <EventIndicator day={date.day} modulo={5} color="green" /> <EventIndicator day={date.day} modulo={3} color="yellow" /> </div>apps/docs/content/components/range-calendar/custom-cell-content.raw.jsx (1)
1-49: Consider sharing custom cell content implementation.This component is nearly identical to the Calendar version. Consider creating a shared component that can be used by both Calendar and RangeCalendar to avoid code duplication.
Create a new shared component:
// shared/CalendarCellEvents.jsx export const CalendarCellEvents = ({ date }) => ( <CalendarCellContent> <CalendarCellHeader /> <CalendarCellBody> {/* Event indicator implementation */} </CalendarCellBody> </CalendarCellContent> );Then use it in both calendar examples:
// Calendar <Calendar calendarWidth={400}> {(date) => <CalendarCellEvents date={date} />} </Calendar> // RangeCalendar <RangeCalendar calendarWidth={400}> {(date) => <CalendarCellEvents date={date} />} </RangeCalendar>packages/components/calendar/src/calendar-cell-header.tsx (1)
12-68: LGTM! Well-implemented component with comprehensive state handling.The component effectively uses context hooks and provides extensive data attributes for styling. Good job setting the displayName for debugging.
Consider using TypeScript's
FCtype for better type safety:-export const CalendarCellHeader = ({children}: CalendarCellHeaderProps) => { +export const CalendarCellHeader: React.FC<CalendarCellHeaderProps> = ({children}) => {packages/components/calendar/__tests__/calendar.test.tsx (1)
470-489: Consider adding more test cases for custom cell content.While the basic test case is good, consider adding tests for:
- Empty/null custom content
- Complex content with nested components
- Content that changes based on cell state (selected, disabled, etc.)
packages/components/calendar/__tests__/range-calendar.test.tsx (1)
750-774: Consider adding range-specific test cases for custom cell content.While the basic test is good, consider adding tests for:
- Custom content in range selection mode
- Content rendering during range dragging
- Content updates when range endpoints change
apps/docs/content/docs/components/calendar.mdx (2)
138-146: Clarify Customization Methods in the DocumentationThe new "Custom Cell Content" section states that the Calendar component supports customizing cell content in two ways; however, only a single CodeDemo is provided. For clarity and to mirror the detailed examples provided in the RangeCalendar docs, consider adding an explicit example for both approaches (using a render function and using component composition). This would help users quickly grasp the alternative methods available.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~146-~146: Loose punctuation mark.
Context: ... customization: -CalendarCellContent: The wrapper component for the cell cont...(UNLIKELY_OPENING_PUNCTUATION)
169-172: Verify Consistency in Slots DescriptionsThe new slots—cellContent, cellHeaderWrapper, cellHeader, and cellBody—are added as part of the custom cell content feature. The descriptions are clear, but please double-check that the punctuation and formatting are consistent with the rest of the Slots section. A minor review for loose punctuation (as flagged by static analysis) would improve overall readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
.changeset/tame-students-tan.md(1 hunks)apps/docs/content/components/calendar/custom-cell-content.raw.jsx(1 hunks)apps/docs/content/components/calendar/custom-cell-content.tsx(1 hunks)apps/docs/content/components/calendar/index.ts(2 hunks)apps/docs/content/components/range-calendar/custom-cell-content.raw.jsx(1 hunks)apps/docs/content/components/range-calendar/custom-cell-content.tsx(1 hunks)apps/docs/content/components/range-calendar/index.ts(2 hunks)apps/docs/content/docs/components/calendar.mdx(2 hunks)apps/docs/content/docs/components/range-calendar.mdx(5 hunks)packages/components/calendar/__tests__/calendar.test.tsx(3 hunks)packages/components/calendar/__tests__/range-calendar.test.tsx(2 hunks)packages/components/calendar/src/calendar-cell-body.tsx(1 hunks)packages/components/calendar/src/calendar-cell-content-default.tsx(1 hunks)packages/components/calendar/src/calendar-cell-content.tsx(1 hunks)packages/components/calendar/src/calendar-cell-context.tsx(1 hunks)packages/components/calendar/src/calendar-cell-header.tsx(1 hunks)packages/components/calendar/src/calendar-cell.tsx(3 hunks)packages/components/calendar/src/calendar.tsx(2 hunks)packages/components/calendar/src/index.ts(2 hunks)packages/components/calendar/src/range-calendar.tsx(2 hunks)packages/components/calendar/src/use-calendar-base.ts(2 hunks)packages/components/calendar/src/use-calendar.ts(3 hunks)packages/components/calendar/src/use-range-calendar.ts(4 hunks)packages/components/calendar/stories/calendar.stories.tsx(3 hunks)packages/components/calendar/stories/range-calendar.stories.tsx(5 hunks)packages/core/theme/src/components/calendar.ts(18 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/calendar.mdx
[uncategorized] ~146-~146: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
apps/docs/content/docs/components/range-calendar.mdx
[uncategorized] ~205-~205: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (41)
apps/docs/content/components/range-calendar/custom-cell-content.tsx (1)
1-1: LGTM! Using raw import for documentation.The import correctly uses Vite's raw import feature to load the example code as a string, which is ideal for documentation purposes.
packages/components/calendar/src/calendar-cell.tsx (3)
13-15: Imports align with new cell context approach
These imports cleanly introduce context-based functionality and a default cell content component, which improves the flexibility for custom rendering.
31-31: Validate custom cell content usage
AccessingcellContentfromuseCalendarContext()is a straightforward solution for injecting custom cell content. Ensure that calling components or consumers are aware of the new prop to avoid undefined usage.
77-109: Comprehensive cell context
Storing the entire spectrum of date and interaction states incellContextValueis a robust approach that centralizes all relevant logic for child consumers. This should neatly support both default and custom cell rendering strategies.packages/core/theme/src/components/calendar.ts (6)
31-37: Consistency in layout changes
Switching tow-full h-fullfor the grid cells and adding new slot classes (cellContent,cellHeaderWrapper) can improve layout flexibility. Confirm that these expansions don’t cause unintended overflow or layout shifts in smaller containers.Also applies to: 39-39
78-120: Range styling logic
These updated pseudo-classes and data attributes for range-bound styling enable more precise theming. Verify that each pseudo-element properly appears or disappears as intended, especially when dealing with disabled or outside-month states.
157-171: Shadow and animation variants
The introducedshowShadowanddisableAnimationvariants add helpful toggles for theming. Consider verifying potential performance overhead from shadows on large calendars or slow devices.
295-299: Foreground range highlight
Applying background tints and text color changes for range selection is a clear approach. Verify that the color token (foreground/10) is consistent across multiple themes (light/dark), ensuring adequate contrast.
383-390: Potential overlap with disabled states
When layering multiple pseudo-elements for range highlights, confirm thatdata-[unavailable=true]ordata-[disabled=true]does not clash visually, especially with partial or fully disabled days.Also applies to: 406-411
428-471: Shadow logic for range edges
Allowing shadows only on the start/end cells in a selected range is a neat effect. Double-check that partial selections spanning months handle these edges gracefully, without abrupt styling transitions.apps/docs/content/components/calendar/custom-cell-content.tsx (1)
1-9: Modular example integration
Importing the raw JSX forAppand exporting it under"/App.jsx"is a clean pattern for showcasing custom cell content. This approach nicely compartmentalizes the docs example.packages/components/calendar/src/calendar-cell-content-default.tsx (2)
1-9: LGTM! Clean and well-typed interface definition.The imports are appropriate and the interface is well-defined with proper type safety.
19-20: LGTM! Proper component identification.Setting displayName follows React best practices for better debugging experience.
packages/components/calendar/src/calendar-cell-content.tsx (2)
1-7: LGTM! Well-structured type definitions.The interface properly extends HTML div props while enforcing the required children prop.
23-26: LGTM! Proper component identification and export.The displayName and export follow React best practices.
packages/components/calendar/src/calendar-cell-body.tsx (1)
27-30: LGTM! Proper component identification and export.The displayName and export follow React best practices.
apps/docs/content/components/calendar/index.ts (1)
16-16: LGTM! Clean addition of custom cell content feature.The import and export of
customCellContentare correctly implemented, aligning with the PR's objective of introducing custom cell content support.Also applies to: 34-34
packages/components/calendar/src/calendar.tsx (2)
10-13: LGTM! Well-structured Props interface.The Props interface correctly extends UseCalendarProps while properly handling the children prop type.
21-26: LGTM! Clean implementation of custom cell content.The children prop is correctly destructured and passed as cellContent to useCalendar.
apps/docs/content/components/range-calendar/index.ts (1)
16-16: LGTM! Consistent implementation of custom cell content feature.The import and export of
customCellContentmirror the changes in the calendar component, maintaining consistency across the codebase.Also applies to: 35-35
packages/components/calendar/src/range-calendar.tsx (2)
13-16: LGTM! Well-structured Props interface.The Props interface correctly extends UseRangeCalendarProps while properly handling the children prop type.
24-29: LGTM! Clean implementation of custom cell content.The children prop is correctly destructured and passed as cellContent to useRangeCalendar, maintaining consistency with the Calendar component implementation.
packages/components/calendar/src/use-calendar.ts (2)
21-21: LGTM! Addition ofcellContentparameter.The new parameter enables custom cell content rendering, aligning with the PR objectives.
112-112: LGTM! Proper context integration ofcellContent.The
cellContentproperty is correctly added to both the context object and its dependency array, ensuring proper memoization.Also applies to: 124-124
packages/components/calendar/src/use-range-calendar.ts (3)
19-19: LGTM! Updated type definition.The type definition correctly excludes
childrenfromAriaRangeCalendarProps, maintaining proper type safety.
29-29: LGTM! Addition ofcellContentparameter.The parameter addition maintains consistency with the calendar component's customization capabilities.
120-120: LGTM! Proper context integration ofcellContent.The
cellContentproperty is correctly added to both the context object and its dependency array, ensuring proper memoization.Also applies to: 131-131
packages/components/calendar/src/use-calendar-base.ts (2)
88-93: LGTM! Well-documented Props interface update.The
cellContentproperty is properly documented with JSDoc and has an appropriate type definition supporting both function and direct ReactNode values.
194-194: LGTM! Consistent ContextType update.The
cellContentproperty is correctly added to the ContextType interface with the same type definition as in Props.packages/components/calendar/stories/calendar.stories.tsx (3)
11-11: LGTM! Appropriate imports for cell customization.The imports include necessary utilities and components for demonstrating cell content customization.
Also applies to: 23-25
277-341: LGTM! Excellent example implementation.The template provides two clear examples of cell content customization:
- Event indicators with different colors based on day conditions
- Custom day number styling for weekends
The implementation effectively demonstrates the flexibility of the new feature.
487-492: LGTM! Proper story export.The story is correctly exported with default props, making it accessible in Storybook.
packages/components/calendar/stories/range-calendar.stories.tsx (3)
89-93: LGTM! Improved error handling in onChange handlers.The addition of null checks before updating state prevents potential runtime errors when the calendar value is cleared.
Also applies to: 174-178, 300-304
312-376: Well-structured custom cell implementations with good accessibility!The custom cell renderers demonstrate:
- Event-based cell content with color-coded indicators
- Day-of-week based text coloring
Both implementations maintain accessibility with proper ARIA attributes.
515-520: Good addition of a story showcasing custom cell content.The story effectively demonstrates the custom cell content feature, providing a valuable reference for other developers.
packages/components/calendar/__tests__/calendar.test.tsx (1)
14-18: Improved type safety with proper generic type parameters.The updated type annotations enhance type checking and provide better TypeScript support.
Also applies to: 22-24
packages/components/calendar/__tests__/range-calendar.test.tsx (1)
19-21: LGTM! Consistent type annotation improvement.The simplified forwardRef type annotation maintains type safety while improving code readability.
.changeset/tame-students-tan.md (1)
1-19: Well-documented feature addition with clear example.The changeset properly describes the new custom cell content feature and provides a helpful example of its usage. The patch version bump is appropriate for this backward-compatible feature addition.
apps/docs/content/docs/components/range-calendar.mdx (3)
16-16: Review the Addition of the CarbonAd ComponentThe insertion of the
<CarbonAd/>component near the top refreshes the document’s visual layout. Please verify that this is intended for production documentation rather than solely for demonstration purposes.
36-38: Update Import Examples with New Cell ComponentsThe updated import examples now include
CalendarCellContent,CalendarCellHeader, andCalendarCellBodyfrom both"@heroui/react"and"@heroui/calendar". This update is correct and aligns well with the new customization features.
49-97: Comprehensive Custom Cell Content SectionThe detailed "Custom Cell Content" section (lines 49-97) is excellent. It clearly explains two methods for customizing cell content—using a render function and component composition—complete with code examples. This provides developers with a clear and practical guide to using the new features.
| ### Custom Cell Content | ||
|
|
||
| The Calendar component supports customizing the cell content in two ways: | ||
|
|
||
| <CodeDemo title="Custom Cell" files={rangeCalendarContent.customCellContent} /> | ||
|
|
||
| The Calendar provides three components for cell customization: | ||
|
|
||
| - `CalendarCellContent`: The wrapper component for the cell content |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate Duplicate Custom Cell Content Section
A nearly identical "Custom Cell Content" section appears again at lines 197-205. This duplication may confuse readers and increase maintenance overhead. Consider consolidating these sections into a single, comprehensive explanation. Additionally, revisiting the punctuation (as noted by static analysis) could further enhance clarity.
Suggested diff to remove duplicate block:
-### Custom Cell Content
-The Calendar component supports customizing the cell content in two ways:
-<CodeDemo title="Custom Cell" files={rangeCalendarContent.customCellContent} />
-The Calendar provides three components for cell customization:
- - `CalendarCellContent`: The wrapper component for the cell content
- - `CalendarCellHeader`: The interactive header element that handles selection
- - `CalendarCellBody`: Additional content container below the button
-These components inherit all calendar states (selected, disabled, etc.) and maintain proper accessibility.Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~205-~205: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
@heroui/accordion
@heroui/alert
@heroui/autocomplete
@heroui/avatar
@heroui/badge
@heroui/breadcrumbs
@heroui/button
@heroui/calendar
@heroui/card
@heroui/checkbox
@heroui/chip
@heroui/date-input
@heroui/code
@heroui/date-picker
@heroui/divider
@heroui/drawer
@heroui/dropdown
@heroui/form
@heroui/image
@heroui/input
@heroui/input-otp
@heroui/kbd
@heroui/link
@heroui/listbox
@heroui/menu
@heroui/modal
@heroui/navbar
@heroui/number-input
@heroui/pagination
@heroui/popover
@heroui/progress
@heroui/radio
@heroui/ripple
@heroui/scroll-shadow
@heroui/select
@heroui/skeleton
@heroui/slider
@heroui/snippet
@heroui/spacer
@heroui/spinner
@heroui/switch
@heroui/table
@heroui/tabs
@heroui/toast
@heroui/tooltip
@heroui/user
@heroui/react
@heroui/system
@heroui/system-rsc
@heroui/theme
@heroui/use-aria-accordion
@heroui/use-aria-accordion-item
@heroui/use-aria-button
@heroui/use-aria-link
@heroui/use-aria-modal-overlay
@heroui/use-aria-multiselect
@heroui/use-callback-ref
@heroui/use-clipboard
@heroui/use-data-scroll-overflow
@heroui/use-disclosure
@heroui/use-draggable
@heroui/use-image
@heroui/use-infinite-scroll
@heroui/use-intersection-observer
@heroui/use-is-mobile
@heroui/use-is-mounted
@heroui/use-measure
@heroui/use-pagination
@heroui/use-real-shape
@heroui/use-ref-state
@heroui/use-resize
@heroui/use-safe-layout-effect
@heroui/use-scroll-position
@heroui/use-ssr
@heroui/use-theme
@heroui/use-update-effect
@heroui/aria-utils
@heroui/dom-animation
@heroui/framer-utils
@heroui/react-rsc-utils
@heroui/react-utils
@heroui/shared-icons
@heroui/shared-utils
@heroui/stories-utils
@heroui/test-utils
commit: |
Summary by CodeRabbit
New Features
Documentation
Tests
Style