feat(calendar): add custom cell content to Calendar #3554
Conversation
… annotations for clarity
🦋 Changeset detectedLatest commit: 0d443a0 The changes in this PR will be included in the next version bump. 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.
|
@jrgarciadev Is there any additional work to be done? Please review. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (12)
apps/docs/components/marketing/marquee.tsx (1)
62-69: Consider adding a user preference check to support reduced motion.Everything works as expected, but it’s good practice to accommodate users who prefer minimized motion. For instance, you can conditionally disable or reduce animation if the user has enabled a reduced motion setting in their OS or browser to further enhance accessibility.
apps/docs/components/docs/toc.tsx (3)
68-79: Consider viewport height variations and accessibility.The fixed height calculation and scroll behavior implementation could be improved:
- The fixed height calculation might not work well across different viewport sizes and devices.
- The mask image implementation could be simplified using Tailwind's built-in gradient utilities.
- Consider adding
aria-labelfor better screen reader support.Consider this improvement:
<div ref={tocRef} + aria-label="Table of contents" className={clsx( - "fixed w-full max-w-[12rem] flex flex-col gap-4 text-left pb-28 h-[calc(100vh-121px)] scrollbar-hide overflow-y-scroll", + "fixed w-full max-w-[12rem] flex flex-col gap-4 text-left pb-28 min-h-0 max-h-[calc(100vh-121px)] scrollbar-hide overflow-y-auto", isProBannerVisible ? "top-32" : "top-20", + "bg-gradient-to-b from-background via-background to-transparent" )} - style={{ - WebkitMaskImage: `linear-gradient(to top, transparent 0%, #000 100px, #000 ${ - scrollPosition > 30 ? "90%" : "100%" - }, transparent 100%)`, - }} >
85-108: Simplify complex styling logic.The list item styling uses multiple conditional classes and pseudo-elements, which could be simplified for better maintainability.
Consider extracting the styles into a custom Tailwind component:
+ // Add to tailwind.config.js + theme: { + extend: { + components: { + 'toc-item': { + base: 'transition-colors font-normal flex items-center text-sm text-default-500 dark:text-default-300', + active: 'text-foreground dark:text-foreground before:opacity-100', + indicator: 'before:content-[""] before:opacity-0 before:-ml-3 before:absolute before:bg-default-400 before:w-1 before:h-1 before:rounded-full before:transition-opacity' + } + } + } + } // Then in the component: - className={clsx( - "transition-colors", - "font-normal", - "flex items-center text-sm font-normal text-default-500 dark:text-default-300", - // ... more classes - )} + className={clsx( + "toc-item-base toc-item-indicator", + {'toc-item-active': activeId === heading.id}, + paddingLeftByLevel[heading.level] + )}
111-124: Enhance back to top functionality.The back to top link could benefit from improved accessibility and user experience.
Consider these improvements:
<li className="mt-2 opacity-0 data-[visible=true]:opacity-100 transition-opacity" data-visible={activeIndex >= 2} > <Divider /> <Spacer y={2} /> <a + role="button" + aria-label="Scroll back to top" className="flex gap-2 items-center text-small text-default-500 dark:text-foreground/30 hover:text-foreground/80 pl-4 transition-opacity" href={`#${firstId}`} + onClick={(e) => { + e.preventDefault(); + window.scrollTo({ top: 0, behavior: 'smooth' }); + }} > Back to top <ChevronCircleTopLinearIcon /> </a> </li>apps/docs/components/docs/components/package-managers.tsx (2)
102-102: Ensure text remains accessible for all users.Using
"text-small"may reduce legibility for some. Consider verifying accessibility standards to ensure sufficient contrast and readability.
104-105: Maintain clarity and readability.Placing the phrase on the same line helps brevity, but be mindful of preserving clarity for users scanning the text. It's recommended to check if any line breaks or additional context would improve comprehension.
apps/docs/components/mdx-components.tsx (1)
208-210: Preserve extensibility by considering a customizable className.Removing the
classNameprop simplifies the component, but it also prevents consumers from adding their own CSS classes. Consider re-adding an optionalclassNameprop to keep the custom styling extensible.apps/docs/components/navbar.tsx (1)
132-139: Consider the trade-off of removinguseCallback.
IfhandleVersionChangeis passed down as a prop to multiple components, memoization might still be beneficial for performance. However, if it’s only used here, removinguseCallbackis reasonable for simpler code.apps/docs/app/providers.tsx (1)
24-25: Consider the reusability ofProviderWrapper.
Currently,ProviderWrapperis defined inline withinProviders. If there's a possibility of reusing this wrapper in other parts of the application, consider placing it in its own file or top-level scope to improve discoverability and maintainability.apps/docs/components/marketing/nextui-pro-section.tsx (2)
26-34: Consider making background image resolution more explicit.Switching between dark and light images based on
isDarkModeandisMobileis done correctly. However, you might want to store these URLs in a separate constants file or provide fallback images for any missing scenario.
46-56: Move CheckmarkIcon to a separate file if reused.Defining
CheckmarkIconinline is reasonable here, but if used across multiple components, consider extracting it into a shared icons folder for better discoverability and reusability.apps/docs/components/docs/sidebar.tsx (1)
320-320: Includeslugandtagin the Memo Dependencies
slugandtagare referenced in your memoized block, yet they aren’t included in the dependency array. If these props change,treeContentwon’t update. Consider adding them to ensure the UI updates correctly.-}, [routes]); +}, [routes, slug, tag]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/workflows/release.yaml(2 hunks)apps/docs/app/docs/layout.tsx(1 hunks)apps/docs/app/page.tsx(1 hunks)apps/docs/app/providers.tsx(1 hunks)apps/docs/components/docs/components/codeblock.tsx(0 hunks)apps/docs/components/docs/components/package-managers.tsx(1 hunks)apps/docs/components/docs/sidebar.tsx(3 hunks)apps/docs/components/docs/toc.tsx(1 hunks)apps/docs/components/marketing/install-banner.tsx(2 hunks)apps/docs/components/marketing/marquee.tsx(1 hunks)apps/docs/components/marketing/nextui-pro-section.tsx(1 hunks)apps/docs/components/mdx-components.tsx(2 hunks)apps/docs/components/navbar.tsx(3 hunks)apps/docs/components/sandpack/copy-button.tsx(1 hunks)apps/docs/config/github-info.json(1 hunks)apps/docs/content/blog/v2.6.0.mdx(4 hunks)
💤 Files with no reviewable changes (1)
- apps/docs/components/docs/components/codeblock.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/docs/config/github-info.json
🔇 Additional comments (32)
apps/docs/components/sandpack/copy-button.tsx (1)
26-26: Use consistent event naming for improved clarity.Switching from
onPresstoonClickaligns with the standard DOM event naming convention, promoting code consistency. The change appears correct; just ensure all references and documentation reflect this new convention.apps/docs/components/marketing/install-banner.tsx (2)
75-75: Confirm consistency in analytics event labels.Noting that the event’s
actionis still"press"despite switching toonClick. Ensure the analytics event name remains consistent across components to prevent confusion or inaccurate reporting.
94-94: Maintain consistent event naming in analytics.Same concern as above. The
actionhere is"press". Verify whether adjusting it to"click"(or another term) would be more accurate and consistent with the new event handler name..github/workflows/release.yaml (2)
30-30: LGTM! Good simplification of the build step.The removal of custom error handling is a good practice as it relies on GitHub Actions' built-in behavior to fail the job if any step fails.
46-46: LGTM! Correct condition for canary release.The simplified condition
steps.changesets.outputs.published != 'true'is the right approach as:
- Build failures are already handled by GitHub Actions
- Canary release should only run when the regular release didn't publish
apps/docs/content/blog/v2.6.0.mdx (4)
90-94: LGTM! Form documentation link and code demo updates.The changes correctly update the form documentation path and add the editor visibility control.
316-316: LGTM! Consistent form documentation link.The documentation link maintains consistency with the earlier update.
569-569: LGTM! Updated routing documentation link.The routing documentation path has been correctly updated to follow the new structure.
752-752: LGTM! Well-structured acknowledgments section.The acknowledgments appropriately recognize team members and contributors with proper GitHub profile links.
apps/docs/components/mdx-components.tsx (2)
201-201: Adopt React standard event naming convention.Switching from
onPresstoonClickis a welcome refinement. This aligns with common React patterns and can help reduce confusion for maintainers and users who expect the standardonClick.
240-240: Consistent usage of InlineCodeChip across API table.Rendering attribute names via
<InlineCodeChip>improves clarity and interface consistency. This pattern unifies the styling of code-like text in your documentation. Looks good!apps/docs/components/navbar.tsx (2)
127-131: Good use of utility class composition.
Leveragingclsxandlink({color: "foreground"})together is a clean approach to keep styling modular and maintainable.
140-148: Check for potential null references.
Ensureposthogis always defined. Otherwise, invokingposthog.captureon a null or undefined object could cause runtime errors. You might want to add a safeguard or fallback ifposthogis not initialized.apps/docs/app/providers.tsx (1)
35-40: Ensure consistent behavior for non-production environments.
Wrapping children withPostHogProvideronly in production is good for performance and privacy in development. This is generally a best practice, so no concerns here.apps/docs/app/docs/layout.tsx (4)
24-24: Check responsive layout for this background image positioning.
The new bottom and left offsets (-bottom-[40%],-left-[20%]) can noticeably shift the image’s appearance on various screens. Ensure that it remains visible and consistent across different viewports.
26-26: Local image reference is a good change.
Switching thesrcfrom an external URL to a local asset can reduce load time variations and improve reliability.
30-30: Examine the top offset for overlapping or hidden content.
The-top-[80%]positioning might risk pushing the image further out of view. Verify that it doesn’t overlap or cause hidden DOM elements on different screen sizes.
32-32: Local gradient image source confirmed.
Using a locally hosted gradient improves control over the asset and consistency in styling.apps/docs/app/page.tsx (1)
16-16: Use the simplified import path consistently.This change to a shorter import path is an improvement for clarity and maintainability. Double-check other references to the old path to avoid confusion or potential import errors.
✅ Verification successful
Import path is consistently used across the codebase
The verification shows that:
- The component is correctly exported from
apps/docs/components/marketing/nextui-pro-section.tsx- The import in
apps/docs/app/page.tsxuses the simplified path@/components/marketing/nextui-pro-section- There are no references to the old path pattern anywhere in the codebase
- The import and export paths are properly aligned
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verifies there's no remaining references to the old NextUIProSection path. rg "@/components/marketing/nextui-pro-section/nextui-pro-section"Length of output: 65
Script:
#!/bin/bash # Let's check for any imports of NextUIProSection component with various path patterns rg -l "import.*NextUIProSection.*from.*components/marketing" # Also check for any references to nextui-pro-section directory fd "nextui-pro-section"Length of output: 163
Script:
#!/bin/bash # Let's examine the actual import statements in both files to ensure consistency echo "=== Content of apps/docs/app/page.tsx ===" rg "import.*NextUIProSection" apps/docs/app/page.tsx echo -e "\n=== Content of apps/docs/components/marketing/nextui-pro-section.tsx ===" rg "export.*NextUIProSection" apps/docs/components/marketing/nextui-pro-section.tsxLength of output: 510
apps/docs/components/marketing/nextui-pro-section.tsx (9)
1-2: Confirm "use client" usage.Ensuring
"use client";is declared at the start is crucial for client-side rendering in Next.js 13. This is correct for using hooks likeuseEffectanduseState.
3-6: Imports appear correct and relevant.The imported hooks and components are appropriate. There's no sign of extraneous imports.
13-19: Check for potential SSR hydration issues.Using a
mountedstate is a good practice to avoid SSR mismatch issues when relying on client-only features (like theme detection). This approach helps ensure that UI elements render properly after hydration.
36-43: Gradient class names are well-defined.The complex gradient definitions are well-structured and properly scoped via utility classes. This approach clearly segregates dark vs. light mode styling.
58-84: Layout structure is clear and expressive.The composition of headings and subheadings, including usage of
titleandsubtitleclass utilities, is well organized. The approach effectively conveys hierarchy for the user.
85-129: Tailwind classes effectively manage text styling.Combining conditional classes for large text, color emphasis, and shadows is well executed, highlighting key text segments with ease.
130-145: SVG structure for arrow icon is fine.The external link arrow is defined here. Confirm with designers that the sizing and stroke align with your design guidelines, but functionally it's good.
156-181: Efficient approach to background images.Wrapping the large background images within Marquee meets the design requirement without overhead. The fallback or layered approach for mobile (
mobileClassName) and web (webClassName) is logically separated.
9-11: Validate the usage of Marquee and custom hooks.Importing the
Marqueecomponent and the customuseIsMobilehook is logically consistent with the feature set. Ensure these references are defined and tested.✅ Verification successful
Both imports are valid and properly defined in the codebase
The verification confirms that:
- The
Marqueecomponent is properly defined inapps/docs/components/marketing/marquee.tsxas a React functional component with proper props and implementation.- The
useIsMobilehook is defined inpackages/hooks/use-is-mobile/src/index.tsand provides the expected mobile detection functionality.Both imports are logically consistent with their implementations and are safe to use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searching for the definitions of 'Marquee' and 'useIsMobile' to confirm their existence and usage. rg "function Marquee" -A 10 rg "function useIsMobile" -A 10Length of output: 635
Script:
#!/bin/bash # Let's search for Marquee component definition and its exports ast-grep --pattern 'export default function Marquee' ast-grep --pattern 'function Marquee' ast-grep --pattern 'const Marquee' # Also search for any file named Marquee fd marquee -t f -iLength of output: 4236
apps/docs/components/docs/sidebar.tsx (4)
3-3: UseEffect Implementation is AlignedNo issues found with using
useEffecthere in lieu ofuseLayoutEffect. This aligns well with the current approach, as there's no direct need to measure or manipulate layout.
8-8: Updated React Imports Look GoodIt’s good practice to import React features directly, and removing unused imports helps keep the codebase clean.
13-13: Import Statement is Properly OrganizedThe added
clsxanddataAttrimports are straightforward, and there’s no apparent issue in how they are used in this file.
22-22: ImportingisEmptyis Logical and CorrectThe added import for
isEmptyis successfully utilized to detect child nodes. This method improves clarity instead of manual checks.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/components/calendar/stories/range-calendar.stories.tsx (1)
322-322:⚠️ Potential issueFix aria-labels for MTG events
The aria-labels should be more specific about the event type (MTG) for better accessibility.
Apply this diff to improve the aria-labels:
- aria-label="calendar event" + aria-label="MTG event"Also applies to: 331-331, 340-340
🧹 Nitpick comments (9)
packages/components/calendar/stories/calendar.stories.tsx (1)
273-337: Well-structured implementation with room for improvementThe custom cell content implementation effectively demonstrates the feature with good accessibility practices and semantic styling. However, there are a few suggestions for improvement:
- Consider extracting the repeated event markup into a reusable component to reduce code duplication.
- The magic numbers for event conditions (7, 5, 3) could be more meaningful as named constants.
Here's a suggested refactor to improve code maintainability:
const CustomCellTemplate = (args: CalendarProps) => { const {locale} = useLocale(); + + const EVENT_TYPES = { + WEEKLY: { divisor: 7, color: 'red' }, + SPRINT: { divisor: 5, color: 'green' }, + DAILY: { divisor: 3, color: 'yellow' } + }; + + const CalendarEvent = ({ color, label = "MTG" }) => ( + <span + aria-label="calendar event" + className={`bg-${color}-500/20 w-full rounded-md px-1 text-${color}-400 line-clamp-1`} + role="status" + > + {label} + </span> + ); return ( <div className="flex gap-4"> <div className="flex flex-col items-center gap-4"> <Calendar {...args} calendarWidth={340}> {(date) => ( <CalendarCellContent> <CalendarCellHeader /> <CalendarCellBody> <div className="flex flex-col w-full text-tiny gap-0.5 px-0.5"> - {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> - )} - {date.day % 5 === 0 && ( - <span - aria-label="calendar event" - className="bg-green-500/20 w-full rounded-md px-1 text-green-400 line-clamp-1" - role="status" - > - MTG - </span> - )} - {date.day % 3 === 0 && ( - <span - aria-label="calendar event" - className="bg-yellow-500/20 w-full rounded-md px-1 text-yellow-400 line-clamp-1" - role="status" - > - MTG - </span> - )} + {Object.entries(EVENT_TYPES).map(([type, {divisor, color}]) => ( + date.day % divisor === 0 && ( + <CalendarEvent key={type} color={color} /> + ) + ))} </div> </CalendarCellBody> </CalendarCellContent> )} </Calendar> </div> // ... rest of the componentapps/docs/content/components/range-calendar/custom-cell-content.raw.jsx (2)
10-11: Consider making the width responsive.Using a fixed width of 400px may limit responsiveness. Consider using responsive units (e.g.,
max-width,width: 100%, etc.) or a container-based approach to enhance layout adaptability.
16-42: Differentiatearia-labelorroleattributes for events.Currently, each event is labeled as “calendar event” with
role="status". Consider providing more descriptive labels (e.g., “weekly meeting”, “budget check”, etc.) to improve accessibility and clarity for screen readers.packages/core/theme/src/components/calendar.ts (3)
Line range hint
35-51: Excellent accessibility considerations in the cellHeader implementation.The cellHeader implementation includes proper data attributes for various states (disabled, readonly, unavailable) and includes focus-visible classes for keyboard navigation.
However, consider adding ARIA attributes to improve screen reader support.
cellHeader: [ "w-8 h-8 flex items-center text-foreground justify-center rounded-full shrink-0", "box-border appearance-none select-none whitespace-nowrap font-normal", "subpixel-antialiased overflow-hidden tap-highlight-transparent", + "aria-disabled:cursor-not-allowed", "data-[disabled=true]:text-default-300", "data-[disabled=true]:cursor-default", "data-[readonly=true]:cursor-default", "data-[disabled=true]:transition-none", "data-[unavailable=true]:text-default-300", "data-[unavailable=true]:cursor-default", "data-[unavailable=true]:line-through", ...dataFocusVisibleClasses, ],
78-120: Consider reorganizing range selection styles for better maintainability.The range selection styles are comprehensive but could be better organized. Consider grouping the styles by their purpose:
- Base styles
- Outside month handling
- Range start styles
- Range end styles
cellHeaderWrapper: [ // Base styles "relative", "overflow-visible", "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", // Outside month handling + "/* Outside month handling */", "data-[outside-month=true]:before:hidden", "data-[selected=true]:data-[range-selection=true]:data-[outside-month=true]:bg-transparent", "data-[selected=true]:data-[range-selection=true]:data-[outside-month=true]:text-default-300", "data-[outside-month=true]:after:hidden", "data-[selected=true]:data-[range-selection=true]:data-[outside-month=true]:bg-transparent", "data-[selected=true]:data-[range-selection=true]:data-[outside-month=true]:text-default-300", // Range start styles + "/* Range start styles */", "data-[range-start=true]:before:rounded-l-full", // ... (remaining range start styles) // Range end styles + "/* Range end styles */", "data-[range-end=true]:before:rounded-r-full", // ... (remaining range end styles) ],
157-170: Enhance shadow customization with CSS custom properties.Consider using CSS custom properties for shadow values to improve maintainability and allow for easier theme customization.
showShadow: { true: { - cellHeader: "data-[selected=true]:shadow-md", + cellHeader: "data-[selected=true]:shadow-[var(--calendar-cell-shadow,theme(boxShadow.md))]", }, false: { cellHeader: "shadow-none data-[selected=true]:shadow-none", }, },apps/docs/content/components/calendar/custom-cell-content.raw.jsx (3)
9-10: Consider renaming the default export for clarity.
Naming the componentAppmay be confusing for library consumers. A more descriptive name likeCustomCalendarExampleorCustomCellCalendarcould improve clarity.
17-25: Use consistent aria-label naming for better accessibility.
Here, we use"Calendar event". In later conditions, we use"calendar event". Consider harmonizing the capitalization to maintain consistency, for example"Calendar event".
26-34: Avoid repetitive code for "MTG" content.
The code snippet for displaying "MTG" is repeated in each condition. You could refactor to map the day mod values to the appropriate color or text, making the logic more concise and easier to maintain.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/docs/content/components/calendar/custom-cell-content.raw.jsx(1 hunks)apps/docs/content/components/calendar/custom-cell-content.tsx(1 hunks)apps/docs/content/components/range-calendar/custom-cell-content.raw.jsx(1 hunks)apps/docs/content/components/range-calendar/custom-cell-content.tsx(1 hunks)packages/components/calendar/src/calendar-cell-header.tsx(1 hunks)packages/components/calendar/stories/calendar.stories.tsx(3 hunks)packages/components/calendar/stories/range-calendar.stories.tsx(5 hunks)packages/core/theme/src/components/calendar.ts(18 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/docs/content/components/range-calendar/custom-cell-content.tsx
- apps/docs/content/components/calendar/custom-cell-content.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/calendar/src/calendar-cell-header.tsx
🔇 Additional comments (12)
packages/components/calendar/stories/range-calendar.stories.tsx (4)
15-15: LGTM: Clean import organizationThe new imports are well-organized and properly grouped based on their sources.
Also applies to: 22-28
85-89: LGTM: Consistent null-safety patternThe onChange handlers consistently check for null values before updating state, preventing potential runtime errors.
Also applies to: 170-174, 296-300
319-347: LGTM: Excellent color-coding system for eventsThe implementation uses distinct colors with proper contrast ratios (using opacity) and consistent styling for different types of events. The line-clamp ensures content fits within the cell.
503-508: LGTM: Consistent story export patternThe CustomCellContent story export follows the established pattern in the file, maintaining consistency with other stories.
packages/components/calendar/stories/calendar.stories.tsx (2)
11-11: LGTM: Import statements are properly organizedThe new imports are correctly added to support the custom cell content feature.
Also applies to: 19-26
483-488: LGTM: Story export follows established patternsThe CustomCellContent story export is properly implemented with consistent structure and default props.
apps/docs/content/components/range-calendar/custom-cell-content.raw.jsx (2)
1-6: Great import organization and usage of NextUI’s new Calendar components.No issues detected here. Good job importing multiple components from a single library module for easy maintenance.
15-43: Reduce repetition in conditional event rendering.You are repeating similar blocks for multiples of 7, 5, and 3. Consider creating a helper function or mapping logic to handle this efficiently, and to avoid unintentional multiple events for a single date.
packages/core/theme/src/components/calendar.ts (2)
31-34: LGTM! Grid layout improvements enhance the calendar structure.The updated grid-related classes provide better structure and alignment for the calendar layout.
Line range hint
188-470: Verify color contrast ratios and standardize dark mode handling.While the color variants are comprehensive, there are a few considerations:
- Some color combinations might not meet WCAG contrast requirements
- Dark mode handling is inconsistent across variants (e.g., present in success but missing in danger)
Add dark mode variants consistently across all color variants:
{ isRange: true, color: "danger", class: { cellHeaderWrapper: [ "data-[selected=true]:data-[range-selection=true]:before:bg-danger-50", "data-[selected=true]:data-[range-selection=true]:text-danger-500", + "dark:data-[selected=true]:data-[range-selection=true]:before:bg-danger-100", + "dark:data-[selected=true]:data-[range-selection=true]:text-danger-600", "data-[selected=true]:data-[range-selection=true]:after:bg-danger-50", + "dark:data-[selected=true]:data-[range-selection=true]:after:bg-danger-100", ], }, },apps/docs/content/components/calendar/custom-cell-content.raw.jsx (2)
1-2: Imports look good.
All necessary dependencies are correctly imported from React and@nextui-org/react.
11-16: Great demonstration of a custom cell content callback.
Using the inline function for each cell inside<Calendar>effectively showcases how to pass a date to the child components. This pattern is clear and flexible for customization.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.changeset/tame-students-tan.md (3)
5-8: Consider expanding the changes documentation.While the changes are documented, it would be helpful to include more details about:
- The specific props and types for the new customization features
- Any limitations or constraints on the customization capabilities
- Performance implications, if any
10-17: Enhance migration guide for breaking changes.The breaking changes are well documented, but consider adding:
- A complete mapping of all renamed class names
- Code snippets showing before/after examples for common use cases
- Any deprecated APIs that are being removed
20-28: Improve the example implementation.While the example is helpful, consider enhancing it to:
- Show more complex customization scenarios
- Include TypeScript types for better type safety
- Demonstrate handling of different date states (selected, disabled, today)
Here's a suggested enhancement:
<Calendar> - {(date) => ( + {(date: Date, state: CalendarCellState) => ( - <div style={{ backgroundColor: "lightblue" }}> + <div + style={{ + backgroundColor: state.isSelected ? "lightblue" : + state.isToday ? "lightyellow" : "transparent", + opacity: state.isDisabled ? 0.5 : 1 + }} + > <span>{date.day}</span> + {state.isToday && <span>(Today)</span>} </div> )} </Calendar>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/tame-students-tan.md(1 hunks)
🔇 Additional comments (1)
.changeset/tame-students-tan.md (1)
1-3: LGTM! Version bump is appropriate.The major version bump is correctly specified given the breaking changes being introduced.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
a.txt (3)
90-110: Consider memoizing the CalendarCellContent component.Since this component will be rendered for each cell in the calendar, memoizing it could improve performance.
-export const CalendarCellContent = ({children, ...props}: CalendarCellContentProps) => { +export const CalendarCellContent = React.memo(({children, ...props}: CalendarCellContentProps) => { const {slots, classNames} = useCalendarContext(); return ( <div className={slots?.cellContent({class: classNames?.cellContent})} data-slot="cell-content" {...props} > {children} </div> ); -}; +});🧰 Tools
🪛 LanguageTool
[uncategorized] ~105-~105: Loose punctuation mark.
Context: ...+ > + {children} + + ); +}; + +CalendarCellContent.displayName...(UNLIKELY_OPENING_PUNCTUATION)
179-235: Consider extracting data attributes to a separate function.The CalendarCellHeader component has many data attributes that could be extracted to improve readability.
+const getHeaderDataAttributes = (props: { + isDisabled: boolean; + isInvalid: boolean; + isFocused: boolean; + isFocusVisible: boolean; + isHovered: boolean; + isPressed: boolean; + isRangeEnd: boolean; + isRangeSelection: boolean; + isRangeStart: boolean; + isReadOnly: boolean; + isSelected: boolean; + isSelectionEnd: boolean; + isSelectionStart: boolean; + isToday: boolean; + isUnavailable: boolean; +}) => ({ + "data-disabled": dataAttr(props.isDisabled && !props.isInvalid), + "data-focus-visible": dataAttr(props.isFocused && props.isFocusVisible), + "data-hover": dataAttr(props.isHovered), + // ... rest of the attributes +});🧰 Tools
🪛 LanguageTool
[uncategorized] ~234-~234: Loose punctuation mark.
Context: ...date.day} + + + ); +}; + +CalendarCellHeader.displayName ...(UNLIKELY_OPENING_PUNCTUATION)
432-437: Add examples to the JSDoc comment.The cellContent prop documentation would benefit from usage examples.
/** * Function to custom render the content of the calendar cell * @param date The date to render * @returns ReactNode + * @example + * ```tsx + * <Calendar + * cellContent={(date) => ( + * <div> + * {date.day} + * {isHoliday(date) && <HolidayIcon />} + * </div> + * )} + * /> + * ``` */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
a.txt(1 hunks)packages/components/calendar/__tests__/calendar.test.tsx(3 hunks)packages/components/calendar/__tests__/range-calendar.test.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/components/calendar/tests/range-calendar.test.tsx
- packages/components/calendar/tests/calendar.test.tsx
🧰 Additional context used
🪛 LanguageTool
a.txt
[typographical] ~2-~2: Two consecutive dots
Context: ...ar/src/calendar-base.tsx index d202ebf46..0836bfb24 100644 --- a/packages/componen...
(DOUBLE_PUNCTUATION)
[typographical] ~22-~22: Two consecutive dots
Context: ...tsx new file mode 100644 index 000000000..55c734fdf --- /dev/null +++ b/packages/c...
(DOUBLE_PUNCTUATION)
[uncategorized] ~46-~46: Loose punctuation mark.
Context: ..., + "data-slot": "cell-body", + }; + + return <div {...bodyProps}>{chi...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~57-~57: Two consecutive dots
Context: ...tsx new file mode 100644 index 000000000..b0cfecd32 --- /dev/null +++ b/packages/c...
(DOUBLE_PUNCTUATION)
[uncategorized] ~76-~76: Loose punctuation mark.
Context: ...lHeader> + + ); +}; + +CalendarCellContentDefault.disp...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~82-~82: Two consecutive dots
Context: ...tsx new file mode 100644 index 000000000..948a84cbc --- /dev/null +++ b/packages/c...
(DOUBLE_PUNCTUATION)
[uncategorized] ~105-~105: Loose punctuation mark.
Context: ...+ > + {children} + + ); +}; + +CalendarCellContent.displayName...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~113-~113: Two consecutive dots
Context: ...tsx new file mode 100644 index 000000000..6b4ed8447 --- /dev/null +++ b/packages/c...
(DOUBLE_PUNCTUATION)
[typographical] ~164-~164: Two consecutive dots
Context: ...tsx new file mode 100644 index 000000000..6caebfc27 --- /dev/null +++ b/packages/c...
(DOUBLE_PUNCTUATION)
[uncategorized] ~234-~234: Loose punctuation mark.
Context: ...date.day} + + + ); +}; + +CalendarCellHeader.displayName ...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~241-~241: Two consecutive dots
Context: ...ar/src/calendar-cell.tsx index b2e742c58..0a426b1c9 100644 --- a/packages/componen...
(DOUBLE_PUNCTUATION)
[typographical] ~333-~333: Two consecutive dots
Context: ...alendar/src/calendar.tsx index 30891d3c7..ce5ef2e51 100644 --- a/packages/componen...
(DOUBLE_PUNCTUATION)
[typographical] ~369-~369: Two consecutive dots
Context: ...ts/calendar/src/index.ts index f34634627..a74bebb66 100644 --- a/packages/componen...
(DOUBLE_PUNCTUATION)
[typographical] ~388-~388: Two consecutive dots
Context: ...r/src/range-calendar.tsx index 913f2604c..eda6ba24f 100644 --- a/packages/componen...
(DOUBLE_PUNCTUATION)
[typographical] ~425-~425: Two consecutive dots
Context: ...src/use-calendar-base.ts index e87be7c48..7cfaaefc2 100644 --- a/packages/componen...
(DOUBLE_PUNCTUATION)
[typographical] ~466-~466: Two consecutive dots
Context: ...ndar/src/use-calendar.ts index def46e973..bd4ae7e86 100644 --- a/packages/componen...
(DOUBLE_PUNCTUATION)
[uncategorized] ~488-~488: Loose punctuation mark.
Context: ...ableAnimation, + cellContent, }), [ state, @@ -118,12 +119,...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~496-~496: Loose punctuation mark.
Context: ...ndYearPickers, + cellContent, ], ); return { Component, - ...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~506-~506: Two consecutive dots
Context: ...rc/use-range-calendar.ts index 08d9c321e..635f0054a 100644 --- a/packages/componen...
(DOUBLE_PUNCTUATION)
[uncategorized] ~537-~537: Loose punctuation mark.
Context: ...ableAnimation, + cellContent, }), [ state, @@ -125,13 +126,...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~546-~546: Loose punctuation mark.
Context: ...t, showMonthAndYearPickers, ], ); return { Component, - ...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (3)
a.txt (3)
12-16: LGTM! Fragment wrapper added for proper component structure.The addition of fragment wrapper maintains proper React component structure while preserving the animation functionality.
32-54: Well-structured component with proper typing and context usage.The CalendarCellBody component follows React best practices with proper ref forwarding and type definitions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~46-~46: Loose punctuation mark.
Context: ..., + "data-slot": "cell-body", + }; + + return <div {...bodyProps}>{chi...(UNLIKELY_OPENING_PUNCTUATION)
123-161: Well-defined context with comprehensive type definitions.The CalendarCellContextType interface provides a clear contract for cell-related state and properties.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/components/calendar/src/calendar-cell.tsx (1)
114-118: 🛠️ Refactor suggestionAdd type safety for custom cell content rendering
The custom cell content rendering implementation could benefit from improved type safety and error handling.
This is a duplicate of a previous review comment. The suggestion to add type safety and error boundaries is still valid and should be implemented.
🧹 Nitpick comments (2)
packages/components/calendar/__tests__/calendar.test.tsx (1)
470-489: Enhance test coverage for custom cell content.While the basic functionality is tested, consider adding the following test cases:
- Test accessibility attributes when custom content is rendered
- Test keyboard navigation and selection with custom content
- Test edge cases (e.g., disabled dates, selected dates)
describe("Custom cell content", () => { it("should render custom content in the calendar cells", () => { const wrapper = render( <Calendar defaultValue={new CalendarDate(2024, 3, 31)}> {(date) => ( <div> {date.day} <span>*</span> </div> )} </Calendar>, ); const gridCells = wrapper.getAllByRole("gridcell"); const customContentCell = gridCells.find((cell) => cell.textContent === "31*"); expect(customContentCell).not.toBeNull(); expect(customContentCell).toHaveTextContent("31*"); }); + + it("should maintain accessibility with custom content", () => { + const wrapper = render( + <Calendar defaultValue={new CalendarDate(2024, 3, 31)}> + {(date) => ( + <div> + {date.day} + <span aria-label="highlighted">*</span> + </div> + )} + </Calendar>, + ); + + const selectedDate = wrapper.getByLabelText("Selected", {exact: false}); + expect(selectedDate).toHaveAttribute("aria-selected", "true"); + }); + + it("should handle keyboard navigation with custom content", () => { + const wrapper = render( + <Calendar autoFocus defaultValue={new CalendarDate(2024, 3, 31)}> + {(date) => ( + <div> + {date.day} + <span>*</span> + </div> + )} + </Calendar>, + ); + + const grid = wrapper.getByRole("grid"); + fireEvent.keyDown(grid, {key: "ArrowLeft"}); + + const newFocusedDate = wrapper.getByText("30*"); + expect(newFocusedDate).toHaveFocus(); + }); });packages/components/calendar/src/calendar-cell.tsx (1)
29-29: Add JSDoc documentation for the cell context value object.The cell context value object is well-structured but would benefit from JSDoc documentation to improve maintainability and developer experience.
Add documentation for the context value groups:
+ /** + * Context value for calendar cell state and interactions. + */ const cellContextValue: CalendarCellContextType = { + // Core date and state properties date: props.date, state, // ... rest of the properties };Also applies to: 74-105
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/docs/content/docs/components/range-calendar.mdx(4 hunks)packages/components/calendar/__tests__/calendar.test.tsx(2 hunks)packages/components/calendar/src/calendar-cell.tsx(3 hunks)packages/components/calendar/src/use-range-calendar.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/calendar/src/use-range-calendar.ts
🧰 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 (4)
apps/docs/content/docs/components/range-calendar.mdx (2)
223-226: 🛠️ Refactor suggestionAlign slot names with component examples.
The slot names are inconsistent with the component names used in the examples:
cellHeaderWrapperandcellHeaderslots correspond toCalendarCellButtonin the examplesUpdate the slot names to match the component names used in the examples:
- **cellContent**: The wrapper for custom cell content. - **cellHeaderWrapper**: The wrapper for the cell header content. - **cellHeader**: The header element within the cell that handles selection. - **cellBody**: The container for additional cell content. + **cellContent**: The wrapper for custom cell content (CalendarCellContent). + **cellButton**: The button element that handles selection (CalendarCellHeader). + **cellBody**: The container for additional cell content (CalendarCellBody).Likely invalid or redundant comment.
24-28: Verify package versions and dependencies.The package names have been updated from
@nextui-orgto@heroui. Let's verify the package versions and their availability.Also applies to: 36-37
✅ Verification successful
Package versions and dependencies verified successfully
All referenced packages (@heroui/calendar v2.2.10, @heroui/react v2.6.14, and heroui-cli v1.0.0) are available on npm registry. The installation commands are correctly formatted and follow package management best practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the packages exist and get their latest versions # Check if packages exist on npm echo "Checking @heroui/calendar package..." npm view @heroui/calendar version echo "Checking @heroui/react package..." npm view @heroui/react version # Check if CLI package exists echo "Checking heroui-cli package..." npm view heroui-cli versionLength of output: 349
packages/components/calendar/src/calendar-cell.tsx (2)
Line range hint
17-24: Well-structured props interface with good type safety.The props interface maintains backward compatibility while adding new functionality for picker visibility and slots customization.
2-2: Verify the impact of NextUI to HeroUI migration.The change from NextUI to HeroUI dependencies could affect existing implementations. Please ensure all consumers of this component are updated accordingly.
Also applies to: 6-6, 13-15
✅ Verification successful
Migration change is isolated to calendar component implementation
The change from NextUI to HeroUI types in calendar-cell.tsx is part of a controlled, component-by-component migration. The impact is contained within the calendar component implementation and doesn't require immediate updates to other components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining NextUI imports that might need updating rg --type ts --type tsx "@nextui" -l # Search for existing usage of CalendarCell to identify potential breaking changes ast-grep --pattern 'CalendarCell $$$'Length of output: 104
Script:
#!/bin/bash # Search for NextUI imports rg -t ts -t tsx "@nextui" -l # Search for CalendarCell imports rg -t ts -t tsx "import.*CalendarCell.*from" -l # Search for CalendarCell component usage with props ast-grep --pattern '<CalendarCell $$$>' # Search for remaining NextUI calendar-related types ast-grep --pattern 'import { $$ } from "@nextui-org/theme"'Length of output: 238
Script:
#!/bin/bash # Search for any files with NextUI imports find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "@nextui" {} \; # Search for CalendarCell usage in tsx files find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "CalendarCell" {} \; # Search for NextUI theme imports specifically find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "@nextui-org/theme" {} \;Length of output: 24288
There was a problem hiding this comment.
Checks are failing. Please double check. Probably it is related to the recent package rebrand. Apart from that, here's my comments.
- we only need one changeset and you may set it as
patch(to avoid double bump the version when we set the react one to minor.) - please elaborate more why we need such breaking change
cellButtontocellHeader. It's confusing as I can still seecellButtonis in use while it is mentioned that it has been renamed. - Since this is marked as 2.7.0, we'll first prioritise the bug fix release first. We'll have several rounds of reviews after that. Thank you for your patient.
|
<span
{...mergeProps(buttonProps, hoverProps, focusProps)}
ref={ref}
className={slots?.cellButton({class: classNames?.cellButton})}
>
{/* All cell content goes here */}
</span>
<div className={slots?.cellHeaderWrapper({class: classNames?.cellHeaderWrapper})}>
<span
className={slots?.cellHeader({class: classNames?.cellHeader})}
data-slot="cell-header"
>
{date.day}
</span>
</div>The relationship between them: Key differences:
This is not a breaking change but rather a clearer separation of concerns where:
|
📝 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, andCalendarCellBodyImprovements
Breaking Changes
cellButtontocellHeader)Documentation