Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(calendar): add custom cell content to Calendar #3554

Open
wants to merge 29 commits into
base: canary
Choose a base branch
from

Conversation

1amageek
Copy link

@1amageek 1amageek commented Jul 25, 2024

📝 Description

This PR adds the ability to render custom content in calendar cells. It introduces a new renderCellContent prop 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 renderCellContent function as a prop to the Calendar component. This function receives the date of the cell and allows custom content to be rendered within the cell. If renderCellContent is 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

    • Introduced customizable cell content rendering in both Calendar and RangeCalendar components.
    • Added new components: CalendarCellContent, CalendarCellBody, CalendarCellHeader.
    • Enhanced Storybook examples for Calendar and RangeCalendar, showcasing custom cell content rendering.
  • Tests

    • Added new test suites to verify custom cell content rendering for Calendar and RangeCalendar components.
  • Documentation

    • Expanded documentation sections for both Calendar and RangeCalendar, detailing new customization options and components.

@1amageek 1amageek requested a review from jrgarciadev as a code owner July 25, 2024 03:53
Copy link

changeset-bot bot commented Jul 25, 2024

⚠️ No Changeset found

Latest commit: fdc5337

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Jul 25, 2024

@1amageek is attempting to deploy a commit to the NextUI Inc Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Jul 25, 2024

Walkthrough

This 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 CalendarCellBody, CalendarCellContent, CalendarCellHeader, and their integration into the existing calendar structure. Additionally, new test cases have been added to verify the rendering of custom content in both Calendar and RangeCalendar components. Documentation has also been updated to reflect these changes, improving clarity on customization options.

Changes

File Path Change Summary
packages/components/calendar/__tests__/calendar.test.tsx Added a new test suite "Custom cell content" with a test case for rendering custom content in calendar cells.
packages/components/calendar/src/calendar-cell-body.tsx Introduced CalendarCellBody component with props extending HTML attributes and context integration.
packages/components/calendar/src/calendar-cell-content-default.tsx Added CalendarCellContentDefault component for displaying the day of the month in a header format.
packages/components/calendar/src/calendar-cell-content.tsx Introduced CalendarCellContent component for rendering calendar cell content with context integration.
packages/components/calendar/src/calendar-cell-context.tsx Created context for managing calendar cell state, including properties like date and selection status.
packages/components/calendar/src/calendar-cell.tsx Updated CalendarCell to use new context and rendering logic, allowing for flexible cell content display.
packages/components/calendar/src/calendar.tsx Modified Props interface to include an optional children property for custom rendering of calendar cells.
packages/components/calendar/src/index.ts Expanded exports to include new components: CalendarCellContent, CalendarCellHeader, and CalendarCellBody.
packages/components/calendar/src/range-calendar.tsx Updated RangeCalendar to support custom rendering of calendar cells via a new children prop.
packages/components/calendar/src/use-calendar-base.ts Added cellContent property to Props interface for custom cell rendering.
packages/components/calendar/src/use-calendar.ts Introduced cellContent as an optional property in the useCalendar function.
packages/components/calendar/src/use-range-calendar.ts Added cellContent to useRangeCalendar parameters for custom rendering.
packages/components/calendar/stories/calendar.stories.tsx Enhanced Storybook with CustomCellTemplate for demonstrating custom cell content.
packages/components/calendar/stories/range-calendar.stories.tsx Updated CustomCellTemplate in Storybook for RangeCalendar to showcase custom cell rendering.
packages/core/theme/src/components/calendar.ts Modified calendar component to include new slots for cellContent and cellBody.
apps/docs/content/components/calendar/custom-cell-content.ts Added new file exporting App and AppTs components for rendering calendar interfaces.
apps/docs/content/components/calendar/index.ts Updated to include customCellContent in the calendarContent object.
apps/docs/content/components/range-calendar/custom-cell-content.ts Introduced new file for RangeCalendar with App and AppTs components.
apps/docs/content/components/range-calendar/index.ts Added customCellContent to the rangeCalendarContent object.
apps/docs/content/docs/components/calendar.mdx Enhanced documentation with a new section on custom cell content and updated slots.
apps/docs/content/docs/components/range-calendar.mdx Improved documentation for RangeCalendar, including examples of new components and props.
packages/components/calendar/__tests__/range-calendar.test.tsx Added a test suite "Custom cell content" to verify rendering of custom content in RangeCalendar cells.
packages/components/calendar/src/calendar-cell-header.tsx Introduced CalendarCellHeader component for rendering headers in calendar cells.

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
Loading

Possibly related PRs

Suggested reviewers

  • jrgarciadev
  • wingkwong

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@1amageek
Copy link
Author

#3313 was closed and the PR was remade.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6d9995b and 448ab55.

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 new renderCellContent prop.


10-12: Props interface update looks good.

The Props interface now includes the optional renderCellContent prop, 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 new renderCellContent prop.


17-19: Props interface update looks good.

The Props interface now includes the optional renderCellContent prop, allowing for custom cell content rendering.

packages/components/calendar/src/use-range-calendar.ts (3)

20-22: LGTM!

The addition of the renderCellContent property to the Props interface is correct and aligns with the PR objectives.


27-28: LGTM!

The updated function signature for useRangeCalendar correctly integrates the renderCellContent property.


92-92: LGTM!

The renderCellContent property is correctly passed to the getBaseCalendarProps function, ensuring it is used within the calendar component.

packages/components/calendar/src/calendar-month.tsx (3)

17-17: LGTM!

The addition of the renderCellContent property to the CalendarMonthProps interface is correct and aligns with the PR objectives.


21-21: LGTM!

The updated function signature for CalendarMonth correctly integrates the renderCellContent property.


58-58: LGTM!

The renderCellContent property is correctly passed to the CalendarCell component, ensuring it is used within the calendar cells.

packages/components/calendar/src/use-calendar.ts (3)

21-21: LGTM!

The addition of the renderCellContent property to the Props interface is correct and aligns with the PR objectives.


29-29: LGTM!

The updated function signature for useCalendar correctly integrates the renderCellContent property.


100-100: LGTM!

The renderCellContent property is correctly passed to the getBaseCalendarProps function, ensuring it is used within the calendar component.

packages/components/calendar/src/calendar-cell.tsx (5)

20-20: Prop addition approved.

The renderCellContent prop is a valuable addition, enhancing the flexibility of the CalendarCell component.


24-31: Destructuring approved.

The destructuring of originalProps to include renderCellContent is correctly implemented.


47-48: Usage of otherProps approved.

Forwarding otherProps to useCalendarCell ensures that all remaining props are handled correctly.


54-72: Usage of otherProps.date approved.

Using otherProps.date maintains consistency with the destructured props and ensures correct determination of cell states.


101-101: Rendering logic approved.

The rendering logic correctly uses renderCellContent if provided, and falls back to the default formatted date otherwise.

packages/components/calendar/src/calendar-base.tsx (3)

36-36: Prop addition approved.

The renderCellContent prop is a valuable addition, enhancing the flexibility of the CalendarBase component.


52-52: Destructuring approved.

The destructuring of props to include renderCellContent is correctly implemented.


104-104: Usage of renderCellContent approved.

Passing the renderCellContent prop to CalendarMonth ensures that custom cell content rendering is propagated correctly.

packages/components/calendar/stories/calendar.stories.tsx (2)

260-287: Template function approved.

The CustomCellTemplate function enhances the flexibility of the calendar display by allowing for distinct styling and width settings.


408-419: New export approved.

The CustomCellContent export 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 RangeCalendar components are consistent with the rest of the application and maintainable in the long term.


441-452: LGTM! Verify the correctness of the renderCellContent function.

The code changes are approved.

However, ensure that the renderCellContent function 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 Calendar component 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 RangeCalendar component can render custom content in its cells.

@wingkwong wingkwong added this to the v2.5.0 milestone Jul 25, 2024
@wingkwong wingkwong self-assigned this Aug 4, 2024
Copy link
Member

@wingkwong wingkwong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Sorry for the late review. Overall it looks good and it could be much cleaner. Here's my comments.

  1. in use-calendar.ts, please include renderCellContent in context so that you can retrieve from it instead of passing it from different places.
  2. please add renderCellContent in use-calendar-base.ts.
  3. since you've introduced a new prop, please also update the documentation.
  4. I think we may also show the dot when a cell is selected to make it look better
    image

If you have any questions, free feel to ping me at discord. My ID is same as my github one.

packages/components/calendar/src/use-calendar.ts Outdated Show resolved Hide resolved
packages/components/calendar/src/use-calendar.ts Outdated Show resolved Hide resolved
packages/components/calendar/src/calendar-month.tsx Outdated Show resolved Hide resolved
packages/components/calendar/src/calendar-month.tsx Outdated Show resolved Hide resolved
packages/components/calendar/src/calendar-base.tsx Outdated Show resolved Hide resolved
packages/components/calendar/src/calendar-base.tsx Outdated Show resolved Hide resolved
packages/components/calendar/src/calendar-month.tsx Outdated Show resolved Hide resolved
packages/components/calendar/src/calendar.tsx Outdated Show resolved Hide resolved
packages/components/calendar/src/use-calendar.ts Outdated Show resolved Hide resolved
@jrgarciadev
Copy link
Member

Hey @1amageek please update this PR so we can include this into the 2.5.0 release

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. JSDoc documentation explaining the purpose and usage
  2. 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:

  1. Avoid using React.FC as it's not recommended in modern React
  2. 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:

  1. Add validation for required props
  2. 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 documentation

The 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 usage

The component implementation is solid, but the slots usage could benefit from type safety improvements. Consider adding type checking for the cellBody slot 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 documentation

Adding 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 updates

Since 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 prop

While 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:

  1. Consider adding a renderDay prop for custom day number rendering
  2. Add support for cell-specific className/style props
  3. 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 useCalendarBase

Based on the previous review comment and the component's architecture, cellContent should be retrieved from useCalendarBase instead 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 renderer

To 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:

  1. Access default rendering logic
  2. Compose custom content with default behavior
  3. Maintain consistent styling
packages/components/calendar/stories/calendar.stories.tsx (2)

289-293: Consider extracting weekday styling logic

The 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 story

Consider 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 capabilities

The template effectively showcases both static and dynamic cell content approaches, with proper internationalization support.

Consider the following improvements:

  1. 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;
  1. 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 cellContent through 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 content

While the current test verifies basic rendering, consider adding the following test cases for comprehensive coverage:

  1. Default behavior when renderCellContent is not provided
  2. Handling of null/undefined renderCellContent
  3. Complex custom content (with events, styling)
  4. 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 options

The new slots could benefit from additional customization options:

  1. Color variants for theming consistency
  2. Animation variants for smooth transitions
  3. 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 the cellContent functionality

To ensure the new cellContent prop functions as expected in various scenarios, it is advisable to add unit tests covering cases where cellContent is a function, a ReactNode, 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

📥 Commits

Reviewing files that changed from the base of the PR and between 448ab55 and 9fc7840.

📒 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:

  • CalendarCellProvider is correctly used in CalendarCell component to wrap the cell content
  • useCalendarCell hook is properly consumed within CalendarCellButton component, which is a child of CalendarCell
  • 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.tsx with dedicated test cases
  • Documentation and examples are present in calendar.stories.tsx with 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:

  • cellContent prop 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} />
🏁 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};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

packages/components/calendar/src/calendar-cell-button.tsx Outdated Show resolved Hide resolved
packages/core/theme/src/components/calendar.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 annotations

The 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 code

The App and AppTs templates 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 cellBody

While 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 content

The new slots integrate well with existing styles, but custom content might need additional accessibility considerations.

Consider documenting these accessibility guidelines for custom content:

  1. Preserve the semantic structure of dates
  2. Maintain sufficient color contrast for custom content
  3. Ensure custom content doesn't interfere with keyboard navigation
  4. 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 cases

While 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 descriptions

The 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 slots

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9fc7840 and b2fcf12.

📒 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.

Comment on lines 182 to 195
### 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 cellButton

The 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 styling

The 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 UX

The 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 styles

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 01b3ba2 and 874af0b.

📒 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 indicators

The 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 configuration

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 874af0b and e814634.

📒 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 accessibility

A few suggestions to enhance the example:

  1. The text "MTG" is duplicated in different conditions with different colors, which might be confusing
  2. Consider extracting magic numbers (3, 5, 7) into named constants
  3. 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 accessibility

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between e814634 and 24ccded.

📒 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.

@1amageek
Copy link
Author

@wingkwong @jrgarciadev I made further improvements to the PR, added support for the Storyboard, documentation, and tests. Please review it again.

@1amageek 1amageek requested a review from wingkwong November 25, 2024 05:42
@wingkwong
Copy link
Member

can you check those comments made by coderabbitai and click Resolve Conversation if resolved

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 suggestions

The implementation provides two clear examples of custom cell content:

  1. Event indicators with different colors
  2. Weekend day highlighting

Consider these improvements:

  1. The aria-label "Birthday event" is duplicated across different event types
  2. 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 logic

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 24ccded and 1cd1d21.

📒 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.

@jrgarciadev jrgarciadev modified the milestones: v2.5.0, v2.6.0 Nov 29, 2024
Copy link

vercel bot commented Dec 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nextui-docs-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 12, 2024 11:34am
nextui-storybook-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 12, 2024 11:34am

@1amageek
Copy link
Author

@jrgarciadev Is there any additional work to be done? Please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants