feat(calendar): add custom cell content to Calendar #3554
Conversation
… annotations for clarity
🦋 Changeset detectedLatest commit: e6f5f33 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 |
|
@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 comprehensive enhancements to the Calendar component, focusing on improving customization capabilities, type safety, and component flexibility. The changes span multiple files across the calendar package, introducing new components like Changes
Sequence DiagramsequenceDiagram
participant Developer
participant Calendar
participant CalendarCell
participant CellContent
Developer->>Calendar: Provide custom cell rendering function
Calendar->>CalendarCell: Pass rendering context
CalendarCell->>CellContent: Render custom content
CellContent-->>CalendarCell: Return rendered cell
CalendarCell-->>Calendar: Display customized cell
Possibly related PRs
Suggested Labels
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.
|
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.
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.
|
@wingkwong fixed |
|
@1amageek you may run the build, test, typecheck etc to double check. they are still failing. |
|
@wingkwong fixed |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/components/calendar/src/calendar-cell-content.tsx (1)
9-21: Consider adding ARIA attributes for better accessibility.While the implementation is clean and follows React best practices, consider enhancing accessibility by adding appropriate ARIA attributes.
<div className={slots?.cellContent({class: classNames?.cellContent})} data-slot="cell-content" + role="presentation" {...props} > {children} </div>packages/components/calendar/src/calendar-cell-body.tsx (2)
7-11: Consider using more descriptive interface name.While the interface is well-defined, using a generic name like
Propsmight cause confusion. Consider renaming it to be more specific.-interface Props extends HTMLHeroUIProps<"div"> { +interface CalendarCellBodyProps extends HTMLHeroUIProps<"div"> { children: React.ReactNode; } -export type CalendarCellBodyProps = Props;
13-25: LGTM! Clean implementation with proper ref forwarding.The component correctly implements ref forwarding and follows React best practices. The props spreading and context usage are handled appropriately.
However, consider adding accessibility attributes:
return ( - <div {...bodyProps}>{children}</div> + <div {...bodyProps} role="presentation">{children}</div> );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/components/calendar/src/calendar-cell-body.tsx(1 hunks)packages/components/calendar/src/calendar-cell-content.tsx(1 hunks)packages/components/calendar/src/calendar-cell-context.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/calendar/src/calendar-cell-context.tsx
🔇 Additional comments (3)
packages/components/calendar/src/calendar-cell-content.tsx (2)
5-7: LGTM! Well-defined interface with proper type safety.The interface correctly extends HTMLHeroUIProps and requires children prop, ensuring type safety and proper component usage.
23-25: LGTM! Proper component identification.Setting the display name helps with debugging and component identification in React DevTools.
packages/components/calendar/src/calendar-cell-body.tsx (1)
27-29: LGTM! Proper component identification.Setting the display name helps with debugging and component identification in React DevTools.
|
🤷🏻 |
apps/docs/content/components/range-calendar/custom-cell-content.raw.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/docs/content/components/calendar/custom-cell-content.raw.jsx (1)
12-38: 🛠️ Refactor suggestionConsolidate repeated conditions.
The event indicator logic and styling are duplicated across three conditions. Consider creating a helper function to reduce duplication.
+const EVENT_STYLES = { + 7: { bg: "bg-red-500/20", text: "text-red-400" }, + 5: { bg: "bg-green-500/20", text: "text-green-400" }, + 3: { bg: "bg-yellow-500/20", text: "text-yellow-400" }, +}; + +const renderEventIndicator = (day, styles) => ( + <span + aria-label="Calendar event" + className={`${styles.bg} ${styles.text} w-full rounded-md px-1 line-clamp-1`} + role="status" + > + MTG + </span> +); + {date.day % 7 === 0 && ( - <span - aria-label="Calendar event" - className="bg-red-500/20 w-full rounded-md px-1 text-red-400 line-clamp-1" - role="status" - > - MTG - </span> + renderEventIndicator(date.day, EVENT_STYLES[7]) )}
🧹 Nitpick comments (1)
apps/docs/content/components/calendar/custom-cell-content.raw.jsx (1)
6-6: Consider making the calendar width responsive.Using a fixed width of 400px might cause issues on smaller screens. Consider using responsive units or making the width configurable.
- <Calendar calendarWidth={400}> + <Calendar className="w-full max-w-[400px]">
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/docs/content/components/calendar/custom-cell-content.raw.jsx(1 hunks)
🔇 Additional comments (1)
apps/docs/content/components/calendar/custom-cell-content.raw.jsx (1)
8-11: Well-structured calendar cell layout!Good use of the Calendar component hierarchy with proper content organization using flex layout.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core/theme/src/components/calendar.ts (2)
78-127: Consider improving pseudo-element naming for better maintainability.The range selection styling uses
beforeandafterpseudo-elements for background styling, which might be confusing. Consider using more semantic class names or CSS custom properties.- "before:content-[''] before:absolute before:inset-0 before:z-[-1] before:rounded-none", - "after:content-[''] after:absolute after:inset-0 after:z-[-1] after:rounded-none", + "[--range-bg:theme(colors.primary.50)] [--range-start-bg:theme(colors.primary.500)]", + "before:content-[''] before:absolute before:inset-0 before:z-[-1] before:rounded-none before:bg-[var(--range-bg)]", + "after:content-[''] after:absolute after:inset-0 after:z-[-1] after:rounded-none after:bg-[var(--range-bg)]",
315-321: Consolidate duplicate range selection styles.The range selection background styles are repeated across multiple color variants. Consider extracting common styles into a shared class or using CSS custom properties for better maintainability.
+const rangeSelectionBase = [ + "data-[selected=true]:data-[range-selection=true]:before:bg-[var(--range-bg)]", + "data-[selected=true]:data-[range-selection=true]:text-[var(--range-text)]", + "data-[selected=true]:data-[range-selection=true]:after:bg-[var(--range-bg)]", +]; class: { cellHeaderWrapper: [ - "data-[selected=true]:data-[range-selection=true]:before:bg-primary-50", - "data-[selected=true]:data-[range-selection=true]:text-primary", - "data-[selected=true]:data-[range-selection=true]:after:bg-primary-50", + ...rangeSelectionBase, + "[--range-bg:theme(colors.primary.50)] [--range-text:theme(colors.primary.500)]", ], }Also applies to: 336-342, 357-366, 383-391, 406-412
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/theme/src/components/calendar.ts(17 hunks)
🔇 Additional comments (2)
packages/core/theme/src/components/calendar.ts (2)
31-51: LGTM! Well-structured slot definitions for the new cell architecture.The new slots (
cellContent,cellBody,cellHeaderWrapper,cellHeader) and updated grid styles provide a solid foundation for custom cell content rendering while maintaining accessibility and proper layout.
175-181: LGTM! Proper RTL support implementation.The new
isRTLvariant correctly handles button order for right-to-left layouts. The default value is appropriately set tofalse.Also applies to: 188-189
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/docs/content/components/range-calendar/custom-cell-content.raw.jsx (1)
1-6:⚠️ Potential issueUpdate the package import from "@heroui/react" to "@nextui-org/react".
The import statement is using an incorrect package name.
import { RangeCalendar, CalendarCellContent, CalendarCellHeader, CalendarCellBody, -} from "@heroui/react"; +} from "@nextui-org/react";
🧹 Nitpick comments (4)
packages/core/theme/src/components/calendar.ts (2)
78-128: Consider optimizing pseudo-element usage in range selection.The range selection styling uses both
beforeandafterpseudo-elements with identical properties, which might be redundant.Consider consolidating the styling to use a single pseudo-element:
- "before:content-[''] before:absolute before:inset-0 before:z-[-1] before:rounded-none", - "after:content-[''] after:absolute after:inset-0 after:z-[-1] after:rounded-none", + "before:content-[''] before:absolute before:inset-0 before:z-[-1] before:rounded-none",
191-481: Consider extracting common color variant styles.The compound variants for each color (foreground, primary, secondary, etc.) contain repetitive patterns that could be extracted into reusable utility functions.
Consider creating a utility function to generate color variants:
const createColorVariant = (color: string) => ({ [`data-[selected=true]:bg-${color}`]: true, [`data-[selected=true]:text-${color}-foreground`]: true, [`data-[hover=true]:bg-${color}-50`]: true, [`data-[hover=true]:text-${color}-400`]: true, [`data-[selected=true]:data-[hover=true]:bg-${color}`]: true, [`data-[selected=true]:data-[hover=true]:text-${color}-foreground`]: true, });apps/docs/content/components/range-calendar/custom-cell-content.raw.jsx (2)
8-10: Consider making the calendar width responsive.Using a fixed width of 400px might not work well across different screen sizes. Consider using responsive width or container-based sizing.
- <RangeCalendar calendarWidth={400}> + <RangeCalendar className="w-full max-w-[400px]">
12-14: Document the purpose of empty CalendarCellHeader.Add a comment explaining that CalendarCellHeader renders the default date number, helping other developers understand the component's purpose.
<CalendarCellContent> + {/* CalendarCellHeader renders the date number */} <CalendarCellHeader /> <CalendarCellBody>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/docs/content/components/range-calendar/custom-cell-content.raw.jsx(1 hunks)packages/components/calendar/src/use-calendar-base.ts(2 hunks)packages/core/theme/src/components/calendar.ts(18 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/calendar/src/use-calendar-base.ts
🔇 Additional comments (2)
packages/core/theme/src/components/calendar.ts (2)
31-51: LGTM! Well-structured slot definitions for the new calendar cell architecture.The new slots (
cellContent,cellBody,cellHeaderWrapper,cellHeader) are well-organized and follow a clear hierarchy, improving the component's flexibility for custom content rendering.
176-182: LGTM! Proper RTL support implementation.The RTL support is correctly implemented by swapping the order of navigation buttons.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core/theme/src/components/calendar.ts (2)
78-120: Enhance range selection styling with better pseudo-element handling.The range selection styling has been improved with:
- Better handling of outside month dates
- Consistent width for range start/end indicators
- Proper justification for range indicators
Consider adding RTL support for range selection pseudo-elements to ensure proper alignment in right-to-left layouts.
196-464: Consistent color variants with proper hover and selected states.The compound variants for different colors (foreground, primary, secondary, etc.) maintain consistency in:
- Hover states
- Selected states
- Shadow effects
- Range selection styling
Consider extracting common color-specific styles into a utility function to reduce repetition. Example:
const getColorStyles = (color: string) => ({ cellHeader: [ `data-[selected=true]:bg-${color}`, `data-[selected=true]:text-${color}-foreground`, `data-[hover=true]:bg-${color}-50`, `data-[hover=true]:text-${color}-400`, `data-[selected=true]:data-[hover=true]:bg-${color}`, `data-[selected=true]:data-[hover=true]:text-${color}-foreground`, ] });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/theme/src/components/calendar.ts(18 hunks)
🔇 Additional comments (2)
packages/core/theme/src/components/calendar.ts (2)
31-51: Improved calendar cell structure with better organization.The new slots (
cellContent,cellHeaderWrapper,cellHeader,cellBody) provide better separation of concerns and more granular styling control. The updated grid styles ensure proper alignment and responsiveness.
175-181: Added RTL support for navigation buttons.The new
isRTLvariant properly handles right-to-left layouts by reordering the navigation buttons.✅ Verification successful
🌐 Web query:
What is the current best practice for handling RTL in Tailwind CSS?💡 Result:
Tailwind CSS has evolved its RTL (right-to-left) support through native features and community solutions. Here are current best practices based on recent implementations: ## Core Implementation Strategies 1. **Logical Properties** (Tailwind v3.3+) - Use next-gen CSS logical properties: - `ms-4` instead of `ml-4` (margin-start) - `pe-4` instead of `pr-4` (padding-end) - Supports automatic mirroring when direction changes[2][3][8] 2. **Direction Attribute** ```html <html dir="rtl"> <!-- or dir="ltr" --> ``` This triggers automatic layout adjustments in supported components[2][3][8] ## Technical Approaches **Native Tailwind Features:** ```html <div class="ltr:pl-4 rtl:pr-4"> Content adapts to direction </div> ``` (Using built-in RTL/LTR variants)[9] **Plugin Solution (tailwindcss-rtl):** ```javascript // tailwind.config.js plugins: [require('tailwindcss-rtl')] ``` Replaces directional utilities with start/end semantics: - `ps-4` instead of `pl-4` (padding-start) - `me-4` instead of `mr-4` (margin-end)[8] ## Recommended Workflow 1. Set base direction in HTML 2. Use logical properties for spacing/borders 3. Reserve RTL variants (`rtl:*`) for edge cases 4. For complex layouts: ```html <nav class="flex space-x-4 rtl:space-x-reverse"> <!-- Auto-reversed spacing in RTL --> </nav> ``` ## Component Library Integration Flowbite demonstrates full RTL compatibility through: ```html <a href="#" class="flex items-center space-x-3 rtl:space-x-reverse"> <img src="logo.svg"/> <span>Text</span> </a> ``` (Components automatically mirror layouts)[2][3] ## Production Considerations - Purge unused styles with: ```bash npx tailwindcss -o build.css --minify ``` - Maintain <10kB CSS size even with RTL/LTR support[7] Current consensus favors native logical properties over variant-based solutions[1][2][8]. For legacy projects, the tailwindcss-rtl plugin remains a viable option while transitioning to modern practices[8]. Citations: - 1: https://github.com/tailwindlabs/tailwindcss/discussions/1492 - 2: https://flowbite.com/docs/customize/rtl/ - 3: https://dev.to/themesberg/tailwind-rtl-support-for-ui-components-flowbite-9h - 4: https://benjamincrozat.com/tailwind-css - 5: https://ryanschiang.com/tailwindcss-direction-rtl - 6: https://ishadeed.com/article/on-tailwindcss/ - 7: https://tailwindcss.com/docs/optimizing-for-production - 8: https://github.com/20lives/tailwindcss-rtl - 9: https://tailwindcss.com/docs/hover-focus-and-other-states - 10: https://www.uxpin.com/studio/blog/tailwind-best-practices/
RTL Variant Implementation Verified:
The newisRTLvariant applies specific Tailwind order classes ("order-1" for the next button and "order-3" for the previous button) when RTL is enabled, effectively reordering the navigation buttons. This approach aligns with current Tailwind best practices—reordering elements via utility classes is a valid technique when managing RTL layouts. Just ensure the HTMLdirattribute or equivalent logical properties are set appropriately across the project for full RTL consistency.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/docs/content/docs/components/range-calendar.mdx (1)
191-204: 🛠️ Refactor suggestionDuplicate "Custom Cell Content" Section:
There appears to be redundant documentation for custom cell content: one detailed example is shown above (lines 49–70) and a near-identical section is repeated here. To avoid confusion and reduce maintenance overhead, please consider consolidating these sections by removing the duplicate.🧰 Tools
🪛 LanguageTool
[uncategorized] ~199-~199: Loose punctuation mark.
Context: ... customization: -CalendarCellContent: The wrapper component for the cell cont...(UNLIKELY_OPENING_PUNCTUATION)
🧹 Nitpick comments (1)
apps/docs/content/docs/components/range-calendar.mdx (1)
49-70: Custom Cell Content Example (Render Function):
The example demonstrating how to customize cell content via a render function is clear and well-structured. Consider briefly noting the role of the second parameter (cellState) in the callback for added clarity to developers.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/docs/content/docs/components/range-calendar.mdx(5 hunks)
🧰 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)
16-17: New CarbonAd Insertion:
The addition of the component appears intentional to enrich the documentation interface. Please double-check that its placement and styling align with the overall docs guidelines and do not distract from the primary content.
36-38: Updated Import Statements:
The import commands now include CalendarCellContent, CalendarCellHeader, and CalendarCellBody from both "@heroui/react" and "@heroui/calendar". This update correctly reflects the introduction of the new custom cell content functionality.
223-226: Inconsistent Slot Naming in API Documentation:
The Slots list refers to "cellContent", "cellHeaderWrapper", "cellHeader", and "cellBody", which do not match the component names used in the examples (CalendarCellContent, CalendarCellHeader, CalendarCellBody). Based on previous feedback, aligning these names—perhaps by renaming the interactive element slot to "cellButton" or updating the documentation to reflect the new component names—would improve clarity.
|
@1amageek is it ready fore review? saw you still pushed some changes after re-requesting review. |
|
@jrgarciadev @wingkwong No need. I'll never commit again. |
📝 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
New Features
CalendarCellContent,CalendarCellHeader,CalendarCellBody, andCalendarCellDefault.Improvements
Breaking Changes
cellButtontocellHeader).Documentation