feat(calendar): add custom cell content to Calendar #3554
Conversation
… annotations for clarity
|
|
@1amageek is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces several new components and enhancements to the Calendar and RangeCalendar components, including the addition of custom cell content rendering capabilities. Key changes involve the creation of new components such as Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Calendar
participant CalendarCellContent
participant CalendarCellBody
participant CalendarCellHeader
User->>Calendar: Render calendar
Calendar->>CalendarCellContent: Pass cell content
CalendarCellContent->>CalendarCellBody: Render body content
CalendarCellContent->>CalendarCellHeader: Render header content
CalendarCellBody-->>Calendar: Return body
CalendarCellHeader-->>Calendar: Return header
Calendar-->>User: Display calendar with custom content
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
|
|
#3313 was closed and the PR was remade. |
![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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (12)
- .changeset/popular-seals-appear.md (1 hunks)
- packages/components/calendar/tests/calendar.test.tsx (1 hunks)
- packages/components/calendar/tests/range-calendar.test.tsx (1 hunks)
- packages/components/calendar/src/calendar-base.tsx (4 hunks)
- packages/components/calendar/src/calendar-cell.tsx (5 hunks)
- packages/components/calendar/src/calendar-month.tsx (2 hunks)
- packages/components/calendar/src/calendar.tsx (2 hunks)
- packages/components/calendar/src/range-calendar.tsx (2 hunks)
- packages/components/calendar/src/use-calendar.ts (3 hunks)
- packages/components/calendar/src/use-range-calendar.ts (3 hunks)
- packages/components/calendar/stories/calendar.stories.tsx (2 hunks)
- packages/components/calendar/stories/range-calendar.stories.tsx (2 hunks)
Additional comments not posted (28)
.changeset/popular-seals-appear.md (1)
1-5: LGTM!The changeset correctly summarizes the new feature and updates made in the PR.
packages/components/calendar/src/calendar.tsx (2)
1-1: Import statement update looks good.The import statement now includes
CalendarDate, which is necessary for the newrenderCellContentprop.
10-12: Props interface update looks good.The
Propsinterface now includes the optionalrenderCellContentprop, allowing for custom cell content rendering.packages/components/calendar/src/range-calendar.tsx (2)
1-1: Import statement update looks good.The import statement now includes
CalendarDate, which is necessary for the newrenderCellContentprop.
17-19: Props interface update looks good.The
Propsinterface now includes the optionalrenderCellContentprop, allowing for custom cell content rendering.packages/components/calendar/src/use-range-calendar.ts (3)
20-22: LGTM!The addition of the
renderCellContentproperty to thePropsinterface is correct and aligns with the PR objectives.
27-28: LGTM!The updated function signature for
useRangeCalendarcorrectly integrates therenderCellContentproperty.
92-92: LGTM!The
renderCellContentproperty is correctly passed to thegetBaseCalendarPropsfunction, ensuring it is used within the calendar component.packages/components/calendar/src/calendar-month.tsx (3)
17-17: LGTM!The addition of the
renderCellContentproperty to theCalendarMonthPropsinterface is correct and aligns with the PR objectives.
21-21: LGTM!The updated function signature for
CalendarMonthcorrectly integrates therenderCellContentproperty.
58-58: LGTM!The
renderCellContentproperty is correctly passed to theCalendarCellcomponent, ensuring it is used within the calendar cells.packages/components/calendar/src/use-calendar.ts (3)
21-21: LGTM!The addition of the
renderCellContentproperty to thePropsinterface is correct and aligns with the PR objectives.
29-29: LGTM!The updated function signature for
useCalendarcorrectly integrates therenderCellContentproperty.
100-100: LGTM!The
renderCellContentproperty is correctly passed to thegetBaseCalendarPropsfunction, ensuring it is used within the calendar component.packages/components/calendar/src/calendar-cell.tsx (5)
20-20: Prop addition approved.The
renderCellContentprop is a valuable addition, enhancing the flexibility of theCalendarCellcomponent.
24-31: Destructuring approved.The destructuring of
originalPropsto includerenderCellContentis correctly implemented.
47-48: Usage ofotherPropsapproved.Forwarding
otherPropstouseCalendarCellensures that all remaining props are handled correctly.
54-72: Usage ofotherProps.dateapproved.Using
otherProps.datemaintains consistency with the destructured props and ensures correct determination of cell states.
101-101: Rendering logic approved.The rendering logic correctly uses
renderCellContentif provided, and falls back to the default formatted date otherwise.packages/components/calendar/src/calendar-base.tsx (3)
36-36: Prop addition approved.The
renderCellContentprop is a valuable addition, enhancing the flexibility of theCalendarBasecomponent.
52-52: Destructuring approved.The destructuring of
propsto includerenderCellContentis correctly implemented.
104-104: Usage ofrenderCellContentapproved.Passing the
renderCellContentprop toCalendarMonthensures that custom cell content rendering is propagated correctly.packages/components/calendar/stories/calendar.stories.tsx (2)
260-287: Template function approved.The
CustomCellTemplatefunction enhances the flexibility of the calendar display by allowing for distinct styling and width settings.
408-419: New export approved.The
CustomCellContentexport expands the functionality of the calendar component by providing more options for rendering and styling.packages/components/calendar/stories/range-calendar.stories.tsx (2)
291-318: Verify consistency and maintainability of styles and class names.Ensure that the inline styles and class names used in the
RangeCalendarcomponents are consistent with the rest of the application and maintainable in the long term.
441-452: LGTM! Verify the correctness of therenderCellContentfunction.The code changes are approved.
However, ensure that the
renderCellContentfunction correctly formats the date and integrates well with the rest of the application.packages/components/calendar/__tests__/calendar.test.tsx (1)
467-489: LGTM! The test logic is clear and effective.The test suite correctly verifies that the
Calendarcomponent can render custom content in its cells.packages/components/calendar/__tests__/range-calendar.test.tsx (1)
752-777: LGTM! The test logic is clear and effective.The test suite correctly verifies that the
RangeCalendarcomponent can render custom content in its cells.
There was a problem hiding this comment.
Thanks for the PR. Sorry for the late review. Overall it looks good and it could be much cleaner. Here's my comments.
- in
use-calendar.ts, please includerenderCellContentincontextso that you can retrieve from it instead of passing it from different places. - please add
renderCellContentinuse-calendar-base.ts. - since you've introduced a new prop, please also update the documentation.
- I think we may also show the dot when a cell is selected to make it look better
If you have any questions, free feel to ping me at discord. My ID is same as my github one.
| @@ -18,13 +18,15 @@ interface Props extends UseCalendarBaseProps { | |||
| * Props for the button picker, which is used to select the month, year and expand the header. | |||
| */ | |||
| buttonPickerProps?: ButtonProps; | |||
| renderCellContent?: (date: CalendarDate) => React.ReactNode; | |||
There was a problem hiding this comment.
remove this line and retrieve renderCellContent from useCalendarBase
There was a problem hiding this comment.
<Calendar {...args} calendarWidth={340}>
{(date) => (
<CalendarCellContent>
<CalendarCellHeader />
<CalendarCellBody>
<div className="flex flex-col w-full text-tiny gap-0.5 px-0.5">
// Cell
</div>
</CalendarCellBody>
</CalendarCellContent>
)}
</Calendar>
Changed renderCellContent to children.
|
Hey @1amageek please update this PR so we can include this into the 2.5.0 release |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (22)
packages/components/calendar/src/calendar-cell-content-default.tsx (2)
7-9: Consider adding JSDoc documentation and accessibility props.The interface is clean but could benefit from:
- JSDoc documentation explaining the purpose and usage
- Additional props for accessibility (e.g.,
aria-label)+/** + * Props for the default calendar cell content component. + * @property date - The date to be displayed in the cell + */ export interface CalendarCellContentDefaultProps { date: CalendarDate; + /** Accessible label for the cell content. If not provided, date will be used */ + 'aria-label'?: string; }
11-17: Consider modern React patterns and type safety improvements.While the implementation is clean, consider these improvements:
- Avoid using
React.FCas it's not recommended in modern React- Add type safety for the children prop
-export const CalendarCellContentDefault: React.FC<CalendarCellContentDefaultProps> = ({date}) => { +export function CalendarCellContentDefault({date}: CalendarCellContentDefaultProps) { return ( <CalendarCellContent> <CalendarCellButton>{date.day}</CalendarCellButton> </CalendarCellContent> ); -}; +}packages/components/calendar/src/calendar-cell-content.tsx (2)
9-21: Consider adding prop validation and fallback for slots.While the implementation is clean, consider these improvements:
- Add validation for required props
- Provide a fallback for when slots.cellContent is undefined
Here's a suggested improvement:
export const CalendarCellContent = ({children, ...props}: CalendarCellContentProps) => { const {slots, classNames} = useCalendarContext(); + + if (!children) { + console.warn('CalendarCellContent: children prop is required'); + } + + const cellContentClass = slots?.cellContent?.({class: classNames?.cellContent}) || ''; return ( <div - className={slots?.cellContent({class: classNames?.cellContent})} + className={cellContentClass} data-slot="cell-content" {...props} >
1-25: Well-designed foundation for calendar cell customization.This component provides a solid foundation for the custom cell content feature:
- Clean separation of concerns
- Flexible content rendering
- Proper integration with NextUI's styling system
Consider documenting usage examples in the component's comments to help developers understand how to leverage this component effectively.
packages/components/calendar/src/calendar-cell-body.tsx (2)
7-11: Consider adding JSDoc comments for better documentationThe type definitions are well-structured and properly extend HTMLNextUIProps. Consider adding JSDoc comments to document the Props interface and exported type for better API documentation.
+/** + * Props for the CalendarCellBody component + */ interface Props extends HTMLNextUIProps<"div"> { children: React.ReactNode; } +/** + * Type alias for CalendarCellBody component props + */ export type CalendarCellBodyProps = Props;
13-25: Consider adding type safety for slots usageThe component implementation is solid, but the slots usage could benefit from type safety improvements. Consider adding type checking for the
cellBodyslot to prevent runtime errors.- const {slots, classNames} = useCalendarContext(); + const {slots, classNames} = useCalendarContext(); + + if (!slots?.cellBody) { + console.warn('CalendarCellBody: slots.cellBody is undefined'); + } + const bodyProps = { ...props, ref: ref, className: slots?.cellBody({class: classNames?.cellBody}), "data-slot": "cell-body", };packages/components/calendar/src/calendar-cell-context.tsx (2)
7-24: Consider adding JSDoc documentationAdding JSDoc comments would improve the developer experience by providing detailed descriptions of each property's purpose and expected values.
Example improvement:
+/** + * Context type for calendar cell state and properties + */ export interface CalendarCellContextType { + /** The date represented by this cell */ date: CalendarDate; + /** Calendar state containing selection and navigation info */ state: CalendarState | RangeCalendarState; // ... add documentation for other properties }
1-31: Consider performance implications of context updatesSince this context will be used across multiple calendar cells, consider implementing memoization strategies in consumer components to prevent unnecessary re-renders when context values change.
packages/components/calendar/src/range-calendar.tsx (1)
18-23: Enhance JSDoc documentation for the children propWhile the type definition is correct, the JSDoc could be more descriptive about the usage and parameters.
Consider updating the documentation:
/** - * The calendar cell render function + * Custom render function or content for calendar cells. + * When provided as a function, it receives the cell's date and should return the content to render. + * @param date - The date object representing the calendar cell + * @returns ReactNode to render within the cell */packages/components/calendar/src/calendar-cell-button.tsx (4)
40-41: Remove unnecessary comment.The comment about extracting pressed state doesn't add value as the code is self-explanatory.
- // Extract pressed from buttonProps using optional chaining const isPressed = buttonProps["aria-pressed"] === true;
43-43: Consider consolidating state extraction.Move the date extraction up with the other useCalendarCell destructuring for better code organization.
const { state, buttonProps, isSelected, isDisabled, isInvalid, isOutsideMonth, isToday, isUnavailable, isRangeSelection, isRangeStart, isRangeEnd, isSelectionStart, isSelectionEnd, + date, } = useCalendarCell(); - const {date} = useCalendarCell();
12-14: Add JSDoc documentation for the props interface.Consider adding comprehensive documentation for the props interface to improve developer experience.
+/** + * Props for the CalendarCellButton component + * @property {React.ReactNode} children - Custom content to render inside the cell + */ export interface CalendarCellButtonProps extends HTMLNextUIProps<"div"> { children?: React.ReactNode; }
16-70: Consider enhancing component reusability.To improve the component's flexibility and reusability:
- Consider adding a
renderDayprop for custom day number rendering- Add support for cell-specific className/style props
- Consider implementing a component composition pattern to allow more granular customization
This would align well with the PR's objective of supporting custom cell content while maintaining flexibility.
packages/components/calendar/src/use-range-calendar.ts (1)
Line range hint
108-119: Consider retrieving cellContent from useCalendarBaseBased on the previous review comment and the component's architecture,
cellContentshould be retrieved fromuseCalendarBaseinstead of being passed directly to the context. This ensures consistent behavior between Calendar and RangeCalendar components.Apply this diff:
const context = useMemo<ContextType<RangeCalendarState>>( () => ({ state, slots, headerRef, weekdayStyle, isHeaderExpanded, setIsHeaderExpanded, visibleMonths, classNames, disableAnimation, - cellContent, }), [ state, slots, classNames, weekdayStyle, isHeaderExpanded, setIsHeaderExpanded, visibleMonths, disableAnimation, - cellContent, ], );packages/components/calendar/src/use-calendar.ts (1)
Line range hint
1-144: Consider implementing a default cell rendererTo enhance the component's flexibility while maintaining good defaults, consider implementing a default cell renderer that could be composed with custom content.
Example approach:
interface CellContent { date: DateValue; defaultContent: React.ReactNode; isSelected?: boolean; } type CellContentRenderer = (props: CellContent) => React.ReactNode;This would allow users to:
- Access default rendering logic
- Compose custom content with default behavior
- Maintain consistent styling
packages/components/calendar/stories/calendar.stories.tsx (2)
289-293: Consider extracting weekday styling logicThe weekday styling logic could be extracted into a reusable helper function to improve maintainability and reusability.
+const getWeekdayStyle = (date: DateValue, locale: string) => { + const dayOfWeek = getDayOfWeek(date, locale); + return dayOfWeek === 0 ? "text-red-500" : dayOfWeek === 6 ? "text-blue-500" : "inherit"; +}; {(date) => { - const dayOfWeek = getDayOfWeek(date, locale); - const style = - dayOfWeek === 0 ? "text-red-500" : dayOfWeek === 6 ? "text-blue-500" : "inherit"; + const style = getWeekdayStyle(date, locale);
427-432: Add documentation for the custom cell content storyConsider adding JSDoc comments to document:
- The purpose of this story
- Example usage scenarios
- Available customization options
+/** + * Demonstrates two approaches to customizing calendar cell content: + * 1. Static content with decorative bars + * 2. Dynamic content with weekday-based styling + */ export const CustomCellContent = { render: CustomCellTemplate, args: { ...defaultProps, }, };packages/components/calendar/stories/range-calendar.stories.tsx (1)
298-336: Well-structured implementation demonstrating custom cell content capabilitiesThe template effectively showcases both static and dynamic cell content approaches, with proper internationalization support.
Consider the following improvements:
- Move inline styles to CSS classes for better maintainability:
-const style = - dayOfWeek === 0 ? "text-red-500" : dayOfWeek === 6 ? "text-blue-500" : "inherit"; +const weekendStyles = { + sunday: "text-red-500", + saturday: "text-blue-500", + weekday: "text-inherit" +}; +const style = dayOfWeek === 0 ? weekendStyles.sunday : + dayOfWeek === 6 ? weekendStyles.saturday : + weekendStyles.weekday;
- Ensure sufficient color contrast for weekend dates (red/blue) against all possible background states (hover, selected, etc.)
packages/components/calendar/src/use-calendar-base.ts (1)
218-218: Consider adding validation for cellContent function calls.While the implementation correctly passes the
cellContentthrough the hook, consider adding runtime validation when it's a function to prevent potential null pointer exceptions.return { // ...other properties - cellContent, + cellContent: typeof cellContent === 'function' + ? (date: CalendarDate) => { + try { + return cellContent(date); + } catch (error) { + console.warn('Error rendering cell content:', error); + return null; + } + } + : cellContent, // ...other properties };Also applies to: 339-339
packages/components/calendar/__tests__/calendar.test.tsx (1)
468-490: Enhance test coverage for custom cell contentWhile the current test verifies basic rendering, consider adding the following test cases for comprehensive coverage:
- Default behavior when renderCellContent is not provided
- Handling of null/undefined renderCellContent
- Complex custom content (with events, styling)
- Accessibility validation for custom content
Here's a suggested implementation:
describe("Custom cell content", () => { it("should render custom content in the calendar cells", () => { const renderCellContent = (date: CalendarDate) => ( <div> {date.day} <span>*</span> </div> ); const wrapper = render( <Calendar defaultValue={new CalendarDate(2024, 3, 31)} renderCellContent={renderCellContent} />, ); const gridCells = wrapper.getAllByRole("gridcell"); const customContentCell = gridCells.find((cell) => cell.textContent === "31*"); expect(customContentCell).not.toBeNull(); expect(customContentCell).toHaveTextContent("31*"); }); + + it("should render default content when renderCellContent is not provided", () => { + const wrapper = render( + <Calendar defaultValue={new CalendarDate(2024, 3, 31)} /> + ); + + const gridCells = wrapper.getAllByRole("gridcell"); + const defaultCell = gridCells.find((cell) => cell.textContent === "31"); + + expect(defaultCell).not.toBeNull(); + expect(defaultCell).toHaveTextContent("31"); + }); + + it("should handle interactive custom content", () => { + const handleClick = jest.fn(); + const renderCellContent = (date: CalendarDate) => ( + <div onClick={handleClick} data-testid={`cell-${date.day}`}> + {date.day} + <button>Click me</button> + </div> + ); + + const wrapper = render( + <Calendar + defaultValue={new CalendarDate(2024, 3, 31)} + renderCellContent={renderCellContent} + /> + ); + + const customCell = wrapper.getByTestId("cell-31"); + const button = customCell.querySelector("button"); + + expect(button).not.toBeNull(); + fireEvent.click(button!); + expect(handleClick).toHaveBeenCalled(); + }); + + it("should maintain accessibility with custom content", () => { + const renderCellContent = (date: CalendarDate) => ( + <div role="presentation" aria-label={`Custom cell ${date.day}`}> + {date.day} + </div> + ); + + const wrapper = render( + <Calendar + defaultValue={new CalendarDate(2024, 3, 31)} + renderCellContent={renderCellContent} + /> + ); + + const customCell = wrapper.getByLabelText("Custom cell 31"); + expect(customCell).toBeInTheDocument(); + }); });packages/core/theme/src/components/calendar.ts (1)
Line range hint
35-49: Consider enhancing slot customization optionsThe new slots could benefit from additional customization options:
- Color variants for theming consistency
- Animation variants for smooth transitions
- Dark mode specific styles for better visibility
Example addition to variants:
variants: { disableAnimation: { true: { cellButton: "transition-none", }, false: { headerWrapper: ["[&_.chevron-icon]:transition-transform", "after:transition-height"], grid: "transition-opacity", cellButton: ["origin-center transition-[transform,background-color,color] !duration-150"], pickerWrapper: "transition-opacity !duration-250", pickerItem: "transition-opacity", + cellContent: "transition-opacity duration-150", }, }, },packages/components/calendar/src/calendar-cell.tsx (1)
23-23: Consider adding unit tests for thecellContentfunctionalityTo ensure the new
cellContentprop functions as expected in various scenarios, it is advisable to add unit tests covering cases wherecellContentis a function, aReactNode, or undefined.Would you like assistance in creating these unit tests or setting up test cases?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (16)
packages/components/calendar/__tests__/calendar.test.tsx(1 hunks)packages/components/calendar/src/calendar-cell-body.tsx(1 hunks)packages/components/calendar/src/calendar-cell-button.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.tsx(1 hunks)packages/components/calendar/src/calendar.tsx(1 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(4 hunks)packages/components/calendar/src/use-calendar.ts(3 hunks)packages/components/calendar/src/use-range-calendar.ts(3 hunks)packages/components/calendar/stories/calendar.stories.tsx(3 hunks)packages/components/calendar/stories/range-calendar.stories.tsx(3 hunks)packages/core/theme/src/components/calendar.ts(2 hunks)
🔇 Additional comments (34)
packages/components/calendar/src/calendar-cell-content-default.tsx (3)
1-5: LGTM! Well-structured imports.
Good choice of using @internationalized/date for date handling, and proper component composition with separate CalendarCellContent and CalendarCellButton components.
19-19: LGTM! Good debugging support.
Setting a descriptive display name is a good practice for debugging and follows NextUI naming conventions.
1-19: Verify integration with the Calendar component.
Let's verify how this default content integrates with the main Calendar component.
✅ Verification successful
Integration with Calendar component is properly implemented
The CalendarCellContentDefault component is correctly integrated within the calendar system:
- It's used as the default cell content renderer in CalendarCell component when no custom content is provided
- The integration is verified through comprehensive test coverage in both calendar.test.tsx and range-calendar.test.tsx
- The component properly handles date display and accessibility attributes
- It's compatible with both single-date and range selection modes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of CalendarCellContentDefault in the codebase
ast-grep --pattern 'import { $_ } from "$_/calendar-cell-content-default"'
# Search for tests covering this component
rg -l "CalendarCellContentDefault" --type=test
Length of output: 152
Script:
#!/bin/bash
# Search for usage of CalendarCellContentDefault in the codebase
rg -l "CalendarCellContentDefault"
# Search for calendar component implementation
ast-grep --pattern 'export const Calendar = $_'
# Search for any test files containing Calendar
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts
Length of output: 3041
Script:
#!/bin/bash
# Let's check the calendar-cell.tsx implementation to see how CalendarCellContentDefault is used
cat packages/components/calendar/src/calendar-cell.tsx
# Check the calendar test file for coverage
cat packages/components/calendar/__tests__/calendar.test.tsx
# Check the range calendar test for additional coverage
cat packages/components/calendar/__tests__/range-calendar.test.tsx
Length of output: 48761
packages/components/calendar/src/calendar-cell-content.tsx (2)
1-7: LGTM! Clean type definitions and imports.
The type definitions are well-structured with proper extension of HTMLNextUIProps and explicit children requirement.
23-25: LGTM! Proper component metadata.
The displayName follows NextUI conventions and the export strategy is appropriate.
packages/components/calendar/src/calendar-cell-body.tsx (2)
1-5: LGTM! Well-organized imports
The imports follow good practices with clear separation between types, runtime dependencies, and local imports.
27-29: LGTM! Proper component metadata
The displayName is correctly set for debugging purposes, and the default export is appropriate for this main component.
packages/components/calendar/src/index.ts (1)
3-5: LGTM! New imports follow consistent naming conventions.
The imports for the new cell-related components are well-organized and follow the established naming patterns.
packages/components/calendar/src/calendar-cell-context.tsx (3)
1-5: LGTM! Well-organized type imports
The imports are properly typed and clearly organized, importing only the necessary types from their respective packages.
7-24: LGTM! Comprehensive interface definition
The interface covers all necessary states and properties for a calendar cell, including selection, range, and validation states.
26-31: LGTM! Well-configured context creation
The context is properly configured with strict mode and a helpful error message. However, let's verify that this context is properly consumed across the codebase.
✅ Verification successful
Context usage pattern is correct and well-implemented
The verification shows proper implementation where:
CalendarCellProvideris correctly used inCalendarCellcomponent to wrap the cell contentuseCalendarCellhook is properly consumed withinCalendarCellButtoncomponent, which is a child ofCalendarCell- No instances of using the context hook without the provider were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify proper usage of CalendarCellProvider and useCalendarCell
# Look for any direct context usage without the provider
# Check for useCalendarCell usage
echo "Checking useCalendarCell usage patterns:"
rg "useCalendarCell" -A 5
# Check for CalendarCellProvider usage
echo "Checking CalendarCellProvider implementation:"
rg "CalendarCellProvider" -A 5
Length of output: 5128
packages/components/calendar/src/calendar.tsx (3)
3-3: LGTM: Import addition is appropriate
The CalendarDate type import is necessary for typing the new children prop parameter.
20-26: LGTM: Clean implementation of cell content customization
The changes maintain type safety and follow the component's existing patterns while adding the new functionality.
11-26: Verify test coverage and documentation
The changes look good, but let's ensure proper test coverage and documentation exist.
✅ Verification successful
Test coverage and documentation are sufficient
The changes are well covered:
- Test coverage exists for custom cell content functionality in
calendar.test.tsxwith dedicated test cases - Documentation and examples are present in
calendar.stories.tsxwith a dedicated "CustomCellContent" story that demonstrates both children prop usage patterns:- Declarative JSX composition using CalendarCell components
- Function-as-children pattern with date parameter
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for test coverage and documentation updates
# Look for related test files
echo "Checking for test coverage..."
rg -l "Custom cell content" "packages/components/calendar"
# Look for documentation updates
echo "Checking for documentation updates..."
fd -e mdx -e md . packages/components/calendar | xargs rg -l "cell.*content|renderCell"
Length of output: 422
Script:
#!/bin/bash
# Let's check the actual test content and documentation more thoroughly
# Check test implementation details
echo "Checking test implementations..."
rg -A 10 "children|cellContent" "packages/components/calendar/__tests__/calendar.test.tsx"
rg -A 10 "children|cellContent" "packages/components/calendar/__tests__/range-calendar.test.tsx"
# Look for documentation in more locations and with broader terms
echo "Checking documentation..."
fd . packages/components/calendar -e md -e mdx | xargs cat
Length of output: 8677
Script:
#!/bin/bash
# Let's check for test coverage specifically for the children/cellContent prop
echo "Checking test coverage for children/cellContent prop..."
rg -B 5 -A 15 "it\(.*cell.*content" packages/components/calendar/__tests__/calendar.test.tsx
rg -B 5 -A 15 "it\(.*children" packages/components/calendar/__tests__/calendar.test.tsx
# Check for documentation in storybook files
echo "Checking storybook documentation..."
fd . packages/components/calendar -e stories.tsx | xargs rg -l "children|cellContent"
Length of output: 596
Script:
#!/bin/bash
# Let's check the actual storybook examples and test content
echo "Checking storybook examples..."
cat packages/components/calendar/stories/calendar.stories.tsx
echo "Checking test file content..."
cat packages/components/calendar/__tests__/calendar.test.tsx
Length of output: 27591
packages/components/calendar/src/range-calendar.tsx (2)
3-3: LGTM: Import addition is appropriate
The CalendarDate type import is correctly placed and necessary for the new cell content customization feature.
27-33: Verify integration with CalendarBase component
The implementation looks correct, but let's verify that the cellContent prop is properly handled by the base calendar component.
✅ Verification successful
Let me gather more information about how cellContent is handled in the base calendar component.
Let me check the test files specifically to verify the cellContent functionality.
Based on the previous results, I can now provide a final response as we have sufficient evidence about the cellContent implementation:
✅ Integration with CalendarBase is properly implemented
The code analysis shows that:
cellContentprop is properly defined in the base interfaces and hooks- It's correctly passed through the component hierarchy:
- From RangeCalendar to useRangeCalendar
- Through useCalendarBase
- Finally rendered in CalendarCell component
- CalendarCell handles all cases:
- Function rendering:
typeof cellContent === "function" ? cellContent(date) - Direct content:
: cellContent - Default fallback:
?? <CalendarCellContentDefault date={date} />
- Function rendering:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if CalendarBase properly handles cellContent prop
ast-grep --pattern 'function CalendarBase($props) {
$$$
cellContent
$$$
}'
# Check for any test coverage of custom cell content
rg -l "test|describe|it.*cell.*content" packages/components/calendar/
Length of output: 411
Script:
#!/bin/bash
# Check implementation details of cellContent in calendar components
ast-grep --pattern 'interface $_ {
$$$
cellContent
$$$
}'
# Check how cellContent is used in CalendarBase
rg "cellContent" packages/components/calendar/src/ -A 3 -B 3
# Check test cases for cellContent
rg "cellContent" packages/components/calendar/__tests__/ -A 3 -B 3
Length of output: 9316
Script:
#!/bin/bash
# Check test cases specifically for cellContent functionality
rg "cellContent|children.*prop" packages/components/calendar/__tests__/range-calendar.test.tsx -B 3 -A 3
# Check CalendarCell tests
rg "cellContent|children.*prop" packages/components/calendar/__tests__/calendar-cell.test.tsx -B 3 -A 3
# Check if there are any snapshots containing cellContent
fd -e snap -x cat {} \; | rg "cellContent"
Length of output: 360
packages/components/calendar/src/calendar-cell-button.tsx (2)
1-14: LGTM! Clean imports and well-defined interface.
The imports are well organized and the props interface is appropriately defined with proper type extension.
72-74: LGTM! Proper component metadata.
The display name and export are correctly implemented following React best practices.
packages/components/calendar/src/use-calendar.ts (2)
Line range hint 118-130: LGTM: Proper context and dependency handling
The cellContent prop is correctly added to both the context object and its dependency array, ensuring proper updates when the prop changes.
28-28: 🛠️ Refactor suggestion
Add type definition and documentation for the cellContent prop
The cellContent prop lacks type definition and JSDoc documentation. This is essential for maintaining API clarity and developer experience.
Let's verify the prop location and type usage:
packages/components/calendar/stories/calendar.stories.tsx (2)
11-11: LGTM: Imports are well-organized and necessary for the new feature.
The new imports support the component composition approach for custom cell content.
Also applies to: 18-25
268-306: Implementation differs from PR objectives
The PR objectives mention adding a renderCellContent prop, but the implementation uses component composition with CalendarCellContent, CalendarCellButton, and CalendarCellBody. While this approach is more flexible, it should be documented in the PR description.
Let's check if there are any references to the originally proposed renderCellContent prop:
packages/components/calendar/stories/range-calendar.stories.tsx (2)
15-15: LGTM! New imports are properly organized
The new imports are well-organized and correctly support the custom cell content feature.
Also applies to: 22-28
459-464: LGTM! Story export follows established patterns
The CustomCellContent story is properly exported and maintains consistency with other stories in the file.
packages/components/calendar/src/use-calendar-base.ts (3)
84-90: LGTM! Well-documented prop definition.
The cellContent prop is properly typed and documented with JSDoc comments. The union type ((date: CalendarDate) => React.ReactNode) | React.ReactNode provides good flexibility for both function-based and direct content rendering.
186-186: LGTM! Consistent type definition in context.
The cellContent property in ContextType maintains type consistency with the Props interface.
84-90: Verify Calendar component integration.
Let's verify that the Calendar component properly consumes and renders the cellContent prop.
✅ Verification successful
Let me check how the cellContent prop is being used in the Calendar components.
Calendar component properly integrates the cellContent prop
The verification shows that the cellContent prop is correctly integrated:
- It's properly typed in the base props interface
- It's consumed through the Calendar context in
calendar-cell.tsx - It's correctly rendered with proper fallback to
CalendarCellContentDefault - It supports both function and ReactNode variants as specified in the type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Calendar component implementation for cellContent usage
# Search for cellContent usage in Calendar component files
echo "Searching for cellContent usage in Calendar components..."
rg -l "cellContent" packages/components/calendar/src/
# Check if cellContent is properly typed in Calendar props
echo "Checking Calendar props interface..."
ast-grep --pattern 'interface $PROPS extends $_ {
$$$
cellContent?: $_;
$$$
}'
Length of output: 14993
Script:
#!/bin/bash
# Check how cellContent is used in calendar-cell.tsx and calendar-cell-content.tsx
echo "Checking cellContent implementation in calendar-cell.tsx..."
rg "cellContent" packages/components/calendar/src/calendar-cell.tsx -B 2 -A 2
echo "\nChecking cellContent implementation in calendar-cell-content.tsx..."
rg "cellContent" packages/components/calendar/src/calendar-cell-content.tsx -B 2 -A 2
Length of output: 1290
packages/core/theme/src/components/calendar.ts (1)
35-35: LGTM: cellContent slot implementation
The flex column layout with gap is well-suited for stacking custom content vertically within calendar cells.
packages/components/calendar/src/calendar-cell.tsx (6)
23-23: Addition of cellContent prop enhances customization
The new optional cellContent prop allows developers to provide custom content for calendar cells, improving flexibility. The typing is appropriate and aligns with React standards.
37-37: Proper extraction of cellContent from context
Destructuring cellContent from useCalendarContext ensures that the component accesses context values correctly.
42-51: Effective usage of useAriaCalendarCell
Renaming useCalendarCell to useAriaCalendarCell avoids naming conflicts and improves code clarity. The useAriaCalendarCell hook is correctly utilized to obtain necessary props and state.
98-101: Conditional rendering of cell content is correctly implemented
The logic to render cellContent appropriately handles all cases: when it's a function, a ReactNode, or undefined, defaulting to CalendarCellContentDefault when necessary.
76-93: Comprehensive context provided to CalendarCellProvider
All necessary state and computed properties are included in cellContextValue, ensuring child components receive all required data.
47-47: Verify the disabling logic when isPickerVisible is true
Setting isDisabled to true when isPickerVisible is true may lead to unintended behavior by disabling calendar cells while the picker is visible. Please confirm if this logic aligns with the intended user experience.
| @@ -16,4 +19,4 @@ export {useRangeCalendar} from "./use-range-calendar"; | |||
| export {CalendarProvider, useCalendarContext} from "./calendar-context"; | |||
|
|
|||
| // export component | |||
| export {Calendar, RangeCalendar}; | |||
| export {Calendar, RangeCalendar, CalendarCellContent, CalendarCellButton, CalendarCellBody}; | |||
There was a problem hiding this comment.
💡 Codebase verification
Calendar cell components need proper documentation before being exposed as public API
Based on the verification results:
- The cell components (
CalendarCellContent,CalendarCellButton,CalendarCellBody) are properly implemented with TypeScript interfaces and React components - They are used internally by the Calendar component for cell rendering
- However, there's no documentation or examples showing their intended public usage
- The README.md only contains basic installation instructions and refers to documentation that may not cover these components
Recommendations:
- Document the purpose and usage of each cell component
- Add examples demonstrating customization scenarios
- Consider marking these components as internal if they're not meant for public consumption
🔗 Analysis chain
Verify the public API exposure of new cell components.
While the exports maintain backward compatibility, let's verify that these cell components are intended to be part of the public API and are properly documented.
Let me gather more information about these components to better understand their implementation and documentation status.
Let me check the README content and look for any examples to complete our verification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for documentation and usage examples of the new cell components
# Check for documentation of the new components
echo "Checking for documentation..."
rg -l "CalendarCell(Content|Button|Body)" docs/
# Check for story/example files
echo "Checking for stories/examples..."
fd -e stories.tsx -e stories.ts calendar-cell
# Check for exported types
echo "Checking for exported types..."
rg -l "export.*type.*(CalendarCell(Content|Button|Body)Props)" packages/components/calendar/src/
# Check for tests
echo "Checking for test coverage..."
fd -e test.tsx -e test.ts calendar-cell packages/components/calendar/src/
Length of output: 658
Script:
#!/bin/bash
# Check the implementation and exports of these components
echo "Checking component implementations..."
ast-grep --pattern 'export function CalendarCell$_($$$) { $$$ }'
# Look for any documentation in the source files
echo "Checking source file documentation..."
rg -B 5 "export (function|const) CalendarCell(Content|Button|Body)" packages/components/calendar/src/
# Check if these components are used internally
echo "Checking internal usage..."
rg "CalendarCell(Content|Button|Body)" packages/components/calendar/src/
# Check for README or documentation in the calendar package
echo "Checking package documentation..."
fd README.md packages/components/calendar/
Length of output: 5932
Script:
#!/bin/bash
# Check README content
echo "Checking README content..."
cat packages/components/calendar/README.md
# Look for example usage in the Calendar component
echo "Checking Calendar component implementation..."
rg -A 10 "function Calendar" packages/components/calendar/src/
# Check for examples in stories if they exist
echo "Checking examples directory..."
fd -e tsx -e ts example packages/components/calendar/
Length of output: 5219
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (9)
apps/docs/content/components/calendar/custom-cell-content.ts (2)
20-37: Add TypeScript type annotationsThe TypeScript version (
AppTs) is identical to the JavaScript version and doesn't leverage TypeScript's type system.Consider adding type annotations to demonstrate TypeScript usage:
-export default function App() { +export default function App(): JSX.Element { + const renderCellContent = (date: Date): JSX.Element => ( <div className="flex flex-col w-full gap-0.5 justify-center items-center p-0.5"> // ... content </div> + ); return ( - <Calendar> + <Calendar renderCellContent={renderCellContent}>
39-46: Consider DRYing up the example codeThe
AppandAppTstemplates contain identical code. This duplication makes maintenance harder.Consider using a single template and exporting it with different extensions:
-const App = `...`; -const AppTs = `...`; +const sharedCode = `...`; const react = { - "/App.jsx": App, - "/App.tsx": AppTs, + "/App.jsx": sharedCode, + "/App.tsx": sharedCode, };packages/core/theme/src/components/calendar.ts (3)
35-35: Well-structured implementation of the cellContent slot!The minimal styling with flex layout provides good flexibility for custom content while maintaining consistent spacing.
Consider documenting the following in the component's API docs:
- The vertical flex layout behavior
- How custom content should handle overflow scenarios
49-49: Consider adding overflow handling to cellBodyWhile the full width and height implementation is good, it might need overflow handling for custom content.
Consider adding overflow control:
- cellBody: "w-full h-full", + cellBody: "w-full h-full overflow-hidden",
Line range hint
35-49: Ensure accessibility for custom cell contentThe new slots integrate well with existing styles, but custom content might need additional accessibility considerations.
Consider documenting these accessibility guidelines for custom content:
- Preserve the semantic structure of dates
- Maintain sufficient color contrast for custom content
- Ensure custom content doesn't interfere with keyboard navigation
- Add appropriate ARIA attributes when rendering complex custom content
apps/docs/content/docs/components/calendar.mdx (3)
132-136: Enhance the feature introduction with use casesWhile the introduction is clear, it would be more helpful to include examples of practical use cases for custom cell content, such as displaying events, appointments, or highlighting special dates.
Consider expanding the introduction with something like:
### Custom Cell Content The Calendar component supports customizing the cell content in two ways: + +Common use cases for custom cell content include: +- Displaying events or appointments +- Highlighting special dates with badges or icons +- Adding custom styling for specific date ranges
138-144: Fix punctuation and enhance component descriptionsThe component descriptions could be more detailed to help developers understand when to use each component.
Consider this improved version:
-The Calendar provides three components for cell customization: +The Calendar provides three components for cell customization: -- `CalendarCellContent`: The wrapper component for the cell content -- `CalendarCellButton`: The interactive button element that handles selection -- `CalendarCellBody`: Additional content container below the button +- `CalendarCellContent`: The wrapper component that manages layout and spacing of custom cell content +- `CalendarCellButton`: The interactive button element that handles selection and keyboard navigation +- `CalendarCellBody`: Additional content container below the button, perfect for badges or supplementary information🧰 Tools
🪛 LanguageTool
[uncategorized] ~140-~140: Loose punctuation mark.
Context: ... customization: -CalendarCellContent: The wrapper component for the cell cont...(UNLIKELY_OPENING_PUNCTUATION)
162-164: Add relationships between slotsThe new slots documentation would benefit from explaining how these slots relate to each other in the component hierarchy.
Consider enhancing the documentation:
-- **cellContent**: The wrapper for custom cell content. -- **cellButton**: The button element within the cell. -- **cellBody**: The container for additional cell content. +- **cellContent**: The wrapper for custom cell content. Parent of `cellButton` and `cellBody`. +- **cellButton**: The button element within the cell. Must be a child of `cellContent`. +- **cellBody**: The container for additional cell content. Must be a child of `cellContent`, rendered after `cellButton`.apps/docs/content/docs/components/range-calendar.mdx (1)
46-95: Consider enhancing the documentation with accessibility details.While the custom cell content documentation is comprehensive, it would be beneficial to add specific accessibility guidelines for custom content implementation, such as:
- ARIA attributes to maintain
- Color contrast requirements
- Screen reader considerations
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
apps/docs/content/components/calendar/custom-cell-content.ts(1 hunks)apps/docs/content/components/calendar/index.ts(2 hunks)apps/docs/content/components/range-calendar/custom-cell-content.ts(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(4 hunks)packages/core/theme/src/components/calendar.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/docs/content/components/range-calendar/custom-cell-content.ts
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/calendar.mdx
[uncategorized] ~140-~140: 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] ~190-~190: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (4)
apps/docs/content/components/range-calendar/index.ts (1)
15-15: LGTM! Clean implementation of the custom cell content feature.
The changes follow the established patterns in the codebase and properly integrate the new custom cell content feature into the range calendar documentation.
Also applies to: 32-32
apps/docs/content/components/calendar/index.ts (1)
15-15: LGTM! Documentation changes align with the new feature.
The addition of the customCellContent import and its inclusion in the calendarContent object follows the established pattern and maintains consistency with other example imports. These changes appropriately support the documentation of the new custom cell content feature.
Also applies to: 32-32
apps/docs/content/docs/components/range-calendar.mdx (2)
33-34: LGTM! Import statements are properly updated.
The new cell customization components are correctly added to both main and individual import statements.
213-215: LGTM! Slots documentation is properly updated.
The new cell customization slots are well-documented and consistent with the existing documentation style.
| ### 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 | ||
| - `CalendarCellButton`: The interactive button element that handles selection | ||
| - `CalendarCellBody`: Additional content container below the button | ||
|
|
||
| These components inherit all calendar states (selected, disabled, etc.) and maintain proper accessibility. | ||
|
|
There was a problem hiding this comment.
Remove duplicate custom cell content section.
This section duplicates the content already covered in lines 46-95. Having duplicate documentation can lead to maintenance issues and user confusion. Consider removing this section and ensuring all relevant information is consolidated in the earlier section.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
apps/docs/content/components/range-calendar/custom-cell-content.ts (2)
1-18: Enhance example with date-specific content and accessibilityThe example could be more comprehensive and accessible:
- Show how content can vary based on the date
- Add aria-labels to explain what the colored bars represent
- Use CSS variables for colors to respect theme preferences
Consider updating the example:
const App = `import {RangeCalendar, CalendarCellContent, CalendarCellButton, CalendarCellBody} from "@nextui-org/react"; export default function App() { return ( <RangeCalendar> - <CalendarCellContent> + <CalendarCellContent renderCellContent={(date) => ( <CalendarCellButton /> <CalendarCellBody> <div className="flex flex-col w-full gap-0.5 justify-center items-center p-0.5"> - <span className="bg-red-600 h-1 w-full rounded-full" /> - <span className="bg-green-600 h-1 w-full rounded-full" /> - <span className="bg-yellow-600 h-1 w-full rounded-full" /> + <span + aria-label="High priority tasks" + className="bg-danger h-1 w-full rounded-full" + /> + <span + aria-label="Medium priority tasks" + className="bg-success h-1 w-full rounded-full" + /> + <span + aria-label="Low priority tasks" + className="bg-warning h-1 w-full rounded-full" + /> </div> </CalendarCellBody> - </CalendarCellContent> + )}> + </CalendarCellContent> </RangeCalendar> ); }`;
20-37: Enhance TypeScript example to showcase type safetyThe TypeScript example could better demonstrate:
- Type safety for the date parameter
- Interface for custom cell content props
- Differences between Header and Button variants
Consider updating the example:
const AppTs = `import {RangeCalendar, CalendarCellContent, CalendarCellHeader, CalendarCellBody} from "@nextui-org/react"; +interface TaskIndicator { + priority: 'high' | 'medium' | 'low'; + count: number; +} + +interface CellContentProps { + date: Date; + tasks: TaskIndicator[]; +} + export default function App() { + const renderContent = (date: Date) => { + // Example of date-specific content + const tasks: TaskIndicator[] = [ + { priority: 'high', count: 2 }, + { priority: 'medium', count: 1 }, + { priority: 'low', count: 3 } + ]; + + return ( + <> + <CalendarCellHeader /> + <CalendarCellBody> + <div className="flex flex-col w-full gap-0.5 justify-center items-center p-0.5"> + {tasks.map(({priority, count}) => ( + <span + key={priority} + aria-label={\`\${count} \${priority} priority tasks\`} + className={\`bg-\${priority} h-1 w-full rounded-full\`} + /> + ))} + </div> + </CalendarCellBody> + </> + ); + }; + return ( <RangeCalendar> - <CalendarCellContent> - <CalendarCellHeader /> - <CalendarCellBody> - <div className="flex flex-col w-full gap-0.5 justify-center items-center p-0.5"> - <span className="bg-red-600 h-1 w-full rounded-full" /> - <span className="bg-green-600 h-1 w-full rounded-full" /> - <span className="bg-yellow-600 h-1 w-full rounded-full" /> - </div> - </CalendarCellBody> + <CalendarCellContent renderCellContent={renderContent}> </CalendarCellContent> </RangeCalendar> ); }`;apps/docs/content/docs/components/calendar.mdx (1)
140-140: Fix formatting: Remove extra space after colon-The Calendar provides three components for cell customization: +The Calendar provides three components for cell customization:🧰 Tools
🪛 LanguageTool
[uncategorized] ~140-~140: Loose punctuation mark.
Context: ... customization: -CalendarCellContent: The wrapper component for the cell cont...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/docs/content/components/calendar/custom-cell-content.ts(1 hunks)apps/docs/content/components/range-calendar/custom-cell-content.ts(1 hunks)apps/docs/content/docs/components/calendar.mdx(2 hunks)apps/docs/content/docs/components/range-calendar.mdx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/docs/content/components/calendar/custom-cell-content.ts
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/calendar.mdx
[uncategorized] ~140-~140: 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] ~199-~199: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (5)
apps/docs/content/components/range-calendar/custom-cell-content.ts (1)
39-46: LGTM!
The export configuration correctly maps the JSX and TypeScript examples to their respective paths.
apps/docs/content/docs/components/calendar.mdx (1)
138-145: Enhance component documentation with interfaces and usage details
The existing review comment about improving documentation for custom cell components is still valid. The current descriptions lack:
- Detailed explanation of each component's purpose and usage
- Props/interfaces documentation
- Common use cases and examples
🧰 Tools
🪛 LanguageTool
[uncategorized] ~140-~140: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
apps/docs/content/docs/components/range-calendar.mdx (3)
36-37: LGTM! Import statements updated correctly.
The import statements properly include the new cell customization components.
49-98: LGTM! Well-documented cell customization section.
The section provides comprehensive coverage of cell customization options with clear examples for both render function and component composition approaches.
191-204:
Remove duplicate section and fix component naming inconsistencies.
This section duplicates the content from lines 49-98 and introduces inconsistencies in component naming:
CalendarCellHeadervsCalendarCellButton- The earlier section provides more comprehensive documentation
Please remove this duplicate section to maintain documentation clarity and prevent confusion.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~199-~199: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
| ### Custom Cell Content | ||
|
|
||
| The Calendar component supports customizing the cell content in two ways: | ||
|
|
||
| <CodeDemo title="Custom Cell" files={calendarContent.customCellContent} /> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Documentation needs examples for custom cell content usage
The introduction of custom cell content feature needs more detailed examples showing:
- Basic usage with simple custom content
- Complex scenarios with conditional rendering
- Integration with other UI components
Consider adding example code blocks like this:
// Basic custom content
<Calendar
renderCellContent={(date) => (
<div>
<span>{date.day}</span>
{isHoliday(date) && <HolidayIcon />}
</div>
)}
/>
// Complex conditional rendering
<Calendar
renderCellContent={(date) => (
<CalendarCellContent>
<CalendarCellHeader>{date.day}</CalendarCellHeader>
<CalendarCellBody>
{events[date.toISOString()]?.map(event => (
<EventIndicator key={event.id} type={event.type} />
))}
</CalendarCellBody>
</CalendarCellContent>
)}
/>| - **cellContent**: The wrapper for custom cell content. | ||
| - **cellHeaderWrapper**: The wrapper for the cell header content. | ||
| - **cellHeader**: The header element within the cell that handles selection. | ||
| - **cellBody**: The container for additional cell content. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add prop documentation for new slots
The new slots need detailed documentation about their props and usage patterns.
Add prop documentation for each slot:
- **cellContent**: The wrapper for custom cell content.
+- **cellContent**: The wrapper for custom cell content. Props:
+ - `children`: ReactNode - The content to render
+ - `isSelected`: boolean - Whether the cell is selected
+ - `isDisabled`: boolean - Whether the cell is disabled
+ - Additional aria and data attributes for accessibility
+
- **cellHeaderWrapper**: The wrapper for the cell header content.
+- **cellHeaderWrapper**: The wrapper for the cell header content. Props:
+ - `children`: ReactNode - The header content
+ - Additional styling and layout props
+
- **cellHeader**: The header element within the cell that handles selection.
+- **cellHeader**: The header element within the cell that handles selection. Props:
+ - `children`: ReactNode - The header content
+ - `onSelect`: () => void - Selection handler
+ - Additional interaction props
+
- **cellBody**: The container for additional cell content.
+- **cellBody**: The container for additional cell content. Props:
+ - `children`: ReactNode - The additional content
+ - Additional styling and layout props📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **cellContent**: The wrapper for custom cell content. | |
| - **cellHeaderWrapper**: The wrapper for the cell header content. | |
| - **cellHeader**: The header element within the cell that handles selection. | |
| - **cellBody**: The container for additional cell content. | |
| - **cellContent**: The wrapper for custom cell content. Props: | |
| - `children`: ReactNode - The content to render | |
| - `isSelected`: boolean - Whether the cell is selected | |
| - `isDisabled`: boolean - Whether the cell is disabled | |
| - Additional aria and data attributes for accessibility | |
| - **cellHeaderWrapper**: The wrapper for the cell header content. Props: | |
| - `children`: ReactNode - The header content | |
| - Additional styling and layout props | |
| - **cellHeader**: The header element within the cell that handles selection. Props: | |
| - `children`: ReactNode - The header content | |
| - `onSelect`: () => void - Selection handler | |
| - Additional interaction props | |
| - **cellBody**: The container for additional cell content. Props: | |
| - `children`: ReactNode - The additional content | |
| - Additional styling and layout props |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
packages/core/theme/src/components/calendar.ts (4)
Line range hint
31-50: Enhance accessibility with focus-visible styles for cellButtonThe new slots look good, but the cellButton could benefit from more explicit focus-visible styles for better keyboard navigation accessibility.
Consider adding these focus-visible styles:
- cellButton: ["relative w-full h-full justify-start", ...dataFocusVisibleClasses], + cellButton: [ + "relative w-full h-full justify-start", + "focus-visible:z-10", + "focus-visible:outline-2", + "focus-visible:outline-focus", + ...dataFocusVisibleClasses + ],
77-119: Optimize range selection stylingThe range selection styling uses complex pseudo-elements and multiple data attributes. While functional, it could be simplified for better maintainability.
Consider consolidating similar styles:
- "data-[range-start=true]:before:w-8", - "data-[selection-start=true]:before:w-8", - "data-[range-start=true]:before:justify-self-center", - "data-[selection-start=true]:before:justify-self-center", + "[data-range-start=true]:before:w-8,[data-selection-start=true]:before:w-8", + "[data-range-start=true]:before:justify-self-center,[data-selection-start=true]:before:justify-self-center",
164-169: Optimize animation duration for better UXThe current transition duration might feel slightly slow. Consider reducing it for a snappier feel.
- cellHeader: ["origin-center transition-[transform,background-color,color] !duration-150"], + cellHeader: ["origin-center transition-[transform,background-color,color] !duration-100"],
348-356: Optimize dark mode stylesThe dark mode styles for success variant could be consolidated using CSS variables for better maintainability.
- "data-[selected=true]:data-[range-selection=true]:before:bg-success-100", - "dark:data-[selected=true]:data-[range-selection=true]:before:bg-success-50", + "data-[selected=true]:data-[range-selection=true]:before:bg-[--success-bg]", + "[--success-bg:theme(colors.success.100)] dark:[--success-bg:theme(colors.success.50)]",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
packages/core/theme/src/components/calendar.ts(18 hunks)
🔇 Additional comments (1)
packages/core/theme/src/components/calendar.ts (1)
Line range hint 419-470: LGTM! Shadow implementation is clean and consistent
The shadow variants are well-implemented with consistent opacity values and proper handling of range selection states.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
apps/docs/content/components/calendar/custom-cell-content.ts (2)
9-13: Add semantic meaning to the colored indicatorsThe colored spans appear to represent different states or priorities, but their meaning isn't clear from the code alone.
Consider adding a tooltip or title attribute to provide context:
<div className="flex flex-col w-full gap-0.5 justify-center items-center p-0.5"> - <span className="bg-red-600 h-1 w-full rounded-full" /> - <span className="bg-green-600 h-1 w-full rounded-full" /> - <span className="bg-yellow-600 h-1 w-full rounded-full" /> + <span className="bg-red-600 h-1 w-full rounded-full" title="High Priority Events" /> + <span className="bg-green-600 h-1 w-full rounded-full" title="Available Slots" /> + <span className="bg-yellow-600 h-1 w-full rounded-full" title="Pending Events" /> </div>
39-46: Simplify the export configurationThe current export configuration uses unnecessary spread syntax for a single object.
Consider simplifying:
-const react = { - "/App.jsx": App, - "/App.tsx": AppTs, -}; - -export default { - ...react, -}; +export default { + "/App.jsx": App, + "/App.tsx": AppTs, +};apps/docs/content/docs/components/range-calendar.mdx (1)
49-95: Add explanatory comments to code examples.The documentation clearly explains the customization options, but the code examples could be more self-documenting.
Consider adding comments to explain the customization options:
<RangeCalendar> + {/* Method 1: Using render function to customize date display */} {(date, cellState) => ( <CalendarCellContent> <CalendarCellHeader> <span className={ getDayOfWeek(date, locale) === 0 ? "text-red-500" : "text-default-500" } > {date.day} </span> </CalendarCellHeader> </CalendarCellContent> )} </RangeCalendar> <RangeCalendar> + {/* Method 2: Using component composition for rich content */} <CalendarCellContent> <CalendarCellHeader /> <CalendarCellBody> <div className="flex flex-col w-full gap-0.5 justify-center items-center p-0.5"> <span className="bg-red-600 h-1 w-full rounded-full" /> <span className="bg-green-600 h-1 w-full rounded-full" /> <span className="bg-yellow-600 h-1 w-full rounded-full" /> </div> </CalendarCellBody> </CalendarCellContent> </RangeCalendar>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/docs/content/components/calendar/custom-cell-content.ts(1 hunks)apps/docs/content/components/range-calendar/custom-cell-content.ts(1 hunks)apps/docs/content/docs/components/range-calendar.mdx(5 hunks)packages/components/calendar/src/calendar-cell-header.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/docs/content/components/range-calendar/custom-cell-content.ts
- packages/components/calendar/src/calendar-cell-header.tsx
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/range-calendar.mdx
[uncategorized] ~199-~199: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (3)
apps/docs/content/docs/components/range-calendar.mdx (3)
36-37: LGTM! Import statements updated correctly.
The import statements have been properly updated to include the new cell customization components.
191-204: Remove duplicate section.
This section duplicates the content already covered in lines 49-95.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~199-~199: Loose punctuation mark.
Context: ... customization: - CalendarCellContent: The wrapper component for the cell cont...
(UNLIKELY_OPENING_PUNCTUATION)
223-226: Update slot names to match component examples.
The slot names are inconsistent with the component examples used in the documentation.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
apps/docs/content/components/range-calendar/custom-cell-content.ts (2)
1-32: Consider improving the example's maintainability and accessibilityA few suggestions to enhance the example:
- The text "MTG" is duplicated in different conditions with different colors, which might be confusing
- Consider extracting magic numbers (3, 5, 7) into named constants
- Add ARIA labels or tooltips for better accessibility of colored indicators
Here's a suggested improvement:
const App = `import {RangeCalendar, CalendarCellContent, CalendarCellHeader, CalendarCellBody} from "@nextui-org/react"; +const CALENDAR_EVENTS = { + BIRTHDAY: { mod: 7, text: "Birth day", bgColor: "red" }, + MTG_PRIMARY: { mod: 5, text: "MTG Primary", bgColor: "green" }, + MTG_SECONDARY: { mod: 3, text: "MTG Secondary", bgColor: "yellow" }, +}; export default function App() { return ( <RangeCalendar calendarWidth={400}> {(date) => ( <CalendarCellContent> <CalendarCellHeader /> <CalendarCellBody> <div className="flex flex-col w-full text-tiny gap-0.5 px-0.5"> - {date.day % 7 === 0 && ( - <span className="bg-red-500/20 w-full rounded-md px-1 text-red-400 line-clamp-1"> - Birth day - </span> - )} - {date.day % 5 === 0 && ( - <span className="bg-green-500/20 w-full rounded-md px-1 text-green-400 line-clamp-1"> - MTG - </span> - )} - {date.day % 3 === 0 && ( - <span className="bg-yellow-500/20 w-full rounded-md px-1 text-yellow-400 line-clamp-1"> - MTG - </span> - )} + {Object.entries(CALENDAR_EVENTS).map(([key, {mod, text, bgColor}]) => ( + date.day % mod === 0 && ( + <span + key={key} + className={\`bg-\${bgColor}-500/20 w-full rounded-md px-1 text-\${bgColor}-400 line-clamp-1\`} + aria-label={\`Event: \${text}\`} + > + {text} + </span> + ) + ))} </div> </CalendarCellBody> </CalendarCellContent> )} </RangeCalendar> ); }`;
34-51: Add TypeScript types and improve accessibilityThe TypeScript example could be enhanced with proper types and accessibility improvements.
Here's a suggested improvement:
-const AppTs = `import {RangeCalendar, CalendarCellContent, CalendarCellHeader, CalendarCellBody} from "@nextui-org/react"; +const AppTs = `import {RangeCalendar, CalendarCellContent, CalendarCellHeader, CalendarCellBody, CalendarProps} from "@nextui-org/react"; +type ColorBar = { + color: 'red' | 'green' | 'yellow'; + label: string; +}; +const colorBars: ColorBar[] = [ + { color: 'red', label: 'High priority' }, + { color: 'green', label: 'Medium priority' }, + { color: 'yellow', label: 'Low priority' } +]; -export default function App() { +export default function App(): JSX.Element { return ( <RangeCalendar> <CalendarCellContent> <CalendarCellHeader /> <CalendarCellBody> <div className="flex flex-col w-full gap-0.5 justify-center items-center p-0.5"> - <span className="bg-red-600 h-1 w-full rounded-full" /> - <span className="bg-green-600 h-1 w-full rounded-full" /> - <span className="bg-yellow-600 h-1 w-full rounded-full" /> + {colorBars.map(({color, label}) => ( + <span + key={color} + className={\`bg-\${color}-600 h-1 w-full rounded-full\`} + role="presentation" + aria-label={label} + /> + ))} </div> </CalendarCellBody> </CalendarCellContent> </RangeCalendar> ); }`;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/docs/content/components/calendar/custom-cell-content.ts(1 hunks)apps/docs/content/components/range-calendar/custom-cell-content.ts(1 hunks)
🔇 Additional comments (2)
apps/docs/content/components/calendar/custom-cell-content.ts (1)
53-60: LGTM!
The export configuration is clean and follows best practices.
apps/docs/content/components/range-calendar/custom-cell-content.ts (1)
53-60: LGTM!
The export configuration is clean and follows best practices.
|
@wingkwong @jrgarciadev I made further improvements to the PR, added support for the Storyboard, documentation, and tests. Please review it again. |
|
can you check those comments made by coderabbitai and click Resolve Conversation if resolved |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/components/calendar/stories/calendar.stories.tsx (1)
268-332: LGTM: Well-structured custom cell implementations with minor improvement suggestionsThe implementation provides two clear examples of custom cell content:
- Event indicators with different colors
- Weekend day highlighting
Consider these improvements:
- The aria-label "Birthday event" is duplicated across different event types
- The modulo-based event conditions (date.day % 7, % 5, % 3) might be confusing for users
Consider this improvement for better accessibility:
- aria-label="Birthday event" + aria-label={date.day % 7 === 0 ? "Birthday celebration" : date.day % 5 === 0 ? "Team meeting" : "Project sync"}packages/components/calendar/stories/range-calendar.stories.tsx (1)
319-347: Consider extracting event rendering logicThe event rendering logic in the cell body could be extracted into a separate component to improve readability and reusability.
Consider creating a new component like this:
type CalendarEventProps = { date: DateValue; className: string; label: string; children: React.ReactNode; }; const CalendarEvent = ({date, className, label, children}: CalendarEventProps) => ( <span aria-label={label} className={className} role="status" > {children} </span> );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/docs/content/components/calendar/custom-cell-content.ts(1 hunks)apps/docs/content/components/range-calendar/custom-cell-content.ts(1 hunks)packages/components/calendar/stories/calendar.stories.tsx(3 hunks)packages/components/calendar/stories/range-calendar.stories.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/docs/content/components/calendar/custom-cell-content.ts
- apps/docs/content/components/range-calendar/custom-cell-content.ts
🔇 Additional comments (5)
packages/components/calendar/stories/calendar.stories.tsx (2)
11-11: LGTM: Import statements are properly organized
The new imports for Calendar components and date utilities are correctly structured and necessary for the custom cell content feature.
Also applies to: 18-25
453-458: LGTM: Story export follows established patterns
The CustomCellContent story export maintains consistency with other stories in the file, properly using the default props.
packages/components/calendar/stories/range-calendar.stories.tsx (3)
15-15: LGTM: Imports are properly structured
The new imports for cell-related components and utilities are correctly organized and sourced from appropriate locations.
Also applies to: 22-28
85-89: LGTM: Consistent null checks in onChange handlers
The addition of null checks in onChange handlers is a good defensive programming practice, ensuring type safety and preventing potential runtime errors.
Also applies to: 170-174, 296-300
503-508: LGTM: Story export follows standard pattern
The CustomCellContent story is properly exported and maintains consistency with other stories in the file.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
ci(changesets): 📦 version packages
📝 Description
This PR adds the ability to render custom content in calendar cells. It introduces a new
renderCellContentprop that allows developers to specify custom content for each calendar cell.⛳️ Current behavior (updates)
Currently, the calendar cells render only the default formatted date content, and there is no way to customize the content within each cell.
🚀 New behavior
With this PR, developers can pass a
renderCellContentfunction as a prop to theCalendarcomponent. This function receives the date of the cell and allows custom content to be rendered within the cell. IfrenderCellContentis not provided, the cell will fall back to rendering the default formatted date.💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
This update enhances the flexibility and customizability of the calendar component, allowing for a wide range of use cases where specific content needs to be displayed within calendar cells.
Summary by CodeRabbit
Release Notes
New Features
CalendarCellContent,CalendarCellBody,CalendarCellHeader.Tests
Documentation