-
-
Notifications
You must be signed in to change notification settings - Fork 69
chore(calendar) update to new Shadcn calendar #222
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Calendar
participant DayPicker
participant CalendarDayButton
participant Button
User->>Calendar: Render with props (captionLayout, buttonVariant, components, formatters)
Calendar->>DayPicker: Pass customized props, formatters, components (including Chevron, CalendarDayButton)
DayPicker->>CalendarDayButton: Render each day using CalendarDayButton
CalendarDayButton->>Button: Render day button with custom styling and focus management
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this 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
🧹 Nitpick comments (1)
components/ui/calendar.tsx (1)
28-29: Minor: avoid recomputing default class map
getDefaultClassNames()is pure; calling it twice per render (Calendar & CalendarDayButton) is unnecessary.
Consider lifting it outside the components or memoising to avoid redundant work.Also applies to: 180-181
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
components/ui/calendar.tsx(1 hunks)package.json(1 hunks)
🔇 Additional comments (3)
package.json (1)
50-51: Major-version bump – verify integration & build pipeline
react-day-pickerjumped from 8 → 9 (breaking).
Please run the full build/test suite (including SSR pages) and confirm:
- No ESM/TS import resolution issues.
- The required v9 CSS (or
getDefaultClassNames) is actually bundled.- All calendar usages elsewhere were migrated.
components/ui/calendar.tsx (2)
40-44: Possible wrong formatter key
formatMonthDropdownis not listed in the v9 formatter API (formatCaption,formatDay, …).
Double-check this – an unknown key will be ignored and the dropdown may fallback to default English labels.
182-186: LGTM – focus handling is correctEffect scopes to
modifiers.focused; straightforward and side-effect free.
| <Button | ||
| ref={ref} | ||
| variant="ghost" | ||
| size="icon" | ||
| data-day={day.date.toLocaleDateString()} | ||
| data-selected-single={ | ||
| modifiers.selected && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use locale-independent day identifiers
data-day={day.date.toLocaleDateString()} varies with the user’s locale (e.g. 04/07/2025 vs 7.4.2025).
Prefer an ISO value to keep DOM queries deterministic:
- data-day={day.date.toLocaleDateString()}
+ data-day={day.date.toISOString().slice(0,10)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button | |
| ref={ref} | |
| variant="ghost" | |
| size="icon" | |
| data-day={day.date.toLocaleDateString()} | |
| data-selected-single={ | |
| modifiers.selected && | |
| <Button | |
| ref={ref} | |
| variant="ghost" | |
| size="icon" | |
| data-day={day.date.toISOString().slice(0,10)} | |
| data-selected-single={ | |
| modifiers.selected && |
🤖 Prompt for AI Agents
In components/ui/calendar.tsx around lines 188 to 194, the data-day attribute
uses day.date.toLocaleDateString(), which produces locale-dependent date
strings. Replace this with an ISO 8601 formatted date string (e.g.,
day.date.toISOString().split('T')[0]) to ensure consistent, locale-independent
day identifiers for reliable DOM queries.
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/ui/8bit/calendar.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/ui/8bit/calendar.tsx (1)
lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (1)
components/ui/8bit/calendar.tsx (1)
114-135:❓ Verification inconclusive
Handle the
"down"orientation (and future-proof the fallback)The new Shadcn calendar emits three orientations:
"left","right"and"down".
With the current implementation any non-"left"value falls through to the right-chevron, so the month/year selector will incorrectly render a right arrow instead of a downward one.Quick fix:
- if (orientation === "left") { … } - return ( // right + if (orientation === "left") { … } + if (orientation === "right") { … } + // ↓ fallback for "down" (rotate the right chevron 90° as a placeholder) + return ( + <svg + viewBox="0 0 256 256" + className={cn("size-4 shrink-0 -rotate-90", className)} + aria-label="chevron-down" + {...props} + > + {/* existing right-chevron rects */} + </svg> + );At minimum, add an explicit
if (orientation === "down")guard so the intent is clear and avoid silently rendering the wrong icon.
🏁 Script executed:
#!/usr/bin/env bash # Locate the calendar component file FILE=$(fd calendar.tsx -t f | head -n1) echo "Using file: $FILE" # Show occurrences of orientation prop and related logic rg -n "orientation" "$FILE" # Extract lines around the SVG return (approx. lines 90–160) sed -n '90,160p' "$FILE"Length of output: 277
Explicitly handle the “down” orientation in the chevron icon
Please verify in
components/ui/8bit/calendar.tsx(lines 114–135) that non-"left"values aren’t silently falling back to the right-chevron. You should add explicit guards for both"right"and"down"so the down-arrow doesn’t render as a right-arrow when the calendar emits"down"(and to future-proof against any new orientations).Example snippet:
- if (orientation === "left") { … } - return ( // right + if (orientation === "left") { + … // left-chevron + } else if (orientation === "right") { + … // right-chevron + } else if (orientation === "down") { + return ( + <svg + viewBox="0 0 256 256" + className={cn("size-4 shrink-0 -rotate-90", className)} + aria-label="chevron-down" + {...props} + > + {/* existing rects */} + </svg> + ); + } + // fallback for any other orientation + return ( + <svg + viewBox="0 0 256 256" + className={cn("size-4 shrink-0", className)} + aria-label="chevron-right" + {...props} + > + {/* existing rects */} + </svg> + );
| Chevron: ({ className, ...props }) => { | ||
| if (props.orientation === "left") { | ||
| return ( | ||
| <svg | ||
| viewBox="0 0 256 256" | ||
| fill="currentColor" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| stroke="currentColor" | ||
| strokeWidth="0.25" | ||
| color="" | ||
| className={cn("size-4 shrink-0", className)} | ||
| aria-label="chevron-left" | ||
| {...props} | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid leaking orientation onto the rendered <svg> element
orientation is currently buried inside props, then spread onto the <svg>.
React will pass it through as an unknown DOM attribute (orientation="left" / "right"), which is invalid HTML and needlessly inflates the markup.
- Chevron: ({ className, ...props }) => {
- if (props.orientation === "left") {
+ Chevron: ({ className, orientation, ...props }) => {
+ if (orientation === "left") {
…
- {...props}
+ {...props}
>Extracting the variable keeps the public signature intact while preventing the stray attribute from ever reaching the DOM.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Chevron: ({ className, ...props }) => { | |
| if (props.orientation === "left") { | |
| return ( | |
| <svg | |
| viewBox="0 0 256 256" | |
| fill="currentColor" | |
| xmlns="http://www.w3.org/2000/svg" | |
| stroke="currentColor" | |
| strokeWidth="0.25" | |
| color="" | |
| className={cn("size-4 shrink-0", className)} | |
| aria-label="chevron-left" | |
| {...props} | |
| > | |
| Chevron: ({ className, orientation, ...props }) => { | |
| if (orientation === "left") { | |
| return ( | |
| <svg | |
| viewBox="0 0 256 256" | |
| fill="currentColor" | |
| xmlns="http://www.w3.org/2000/svg" | |
| stroke="currentColor" | |
| strokeWidth="0.25" | |
| color="" | |
| className={cn("size-4 shrink-0", className)} | |
| aria-label="chevron-left" | |
| {...props} | |
| > |
🤖 Prompt for AI Agents
In components/ui/8bit/calendar.tsx around lines 54 to 67, the orientation prop
is being spread onto the SVG element, causing an invalid HTML attribute to
appear in the DOM. To fix this, destructure orientation from props before
spreading the remaining props onto the SVG, so orientation is used internally
but not passed as an attribute to the SVG element.
Summary by CodeRabbit
New Features
Refactor
Chores