feat(datepicker): add outside days styling support#1674
feat(datepicker): add outside days styling support#1674Fejjal94 wants to merge 4 commits intothemesberg:mainfrom
Conversation
|
|
Someone is attempting to deploy a commit to the Bergside Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe Datepicker now marks days outside the viewed month with a new ChangesOut-of-Month Day Styling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/Datepicker/Views/Days.tsx (1)
16-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the new theme key backward-compatible.
Making
outsiderequired on the exportedDatepickerViewsDaysThemeis a breaking type change for any consumer that provides a fully typed custom datepicker theme. Make it optional, or keep a default fallback so existing overrides continue to compile.♻️ Suggested fix
export interface DatepickerViewsDaysTheme { header: { base: string; title: string; }; items: { base: string; item: { base: string; selected: string; disabled: string; today: string; - outside: string; + outside?: string; }; }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Datepicker/Views/Days.tsx` around lines 16 - 29, The new required "outside" key on the exported DatepickerViewsDaysTheme is a breaking change; make the property optional on DatepickerViewsDaysTheme (outside?: string) and ensure the Days view uses a fallback when reading theme.items.item.outside (e.g., default to the existing base/today/disabled class) so existing custom themes that don't include outside continue to compile and behave predictably; update any code that destructures or accesses theme.items.item.outside in the Days component to use the fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/ui/src/components/Datepicker/Views/Days.tsx`:
- Around line 16-29: The new required "outside" key on the exported
DatepickerViewsDaysTheme is a breaking change; make the property optional on
DatepickerViewsDaysTheme (outside?: string) and ensure the Days view uses a
fallback when reading theme.items.item.outside (e.g., default to the existing
base/today/disabled class) so existing custom themes that don't include outside
continue to compile and behave predictably; update any code that destructures or
accesses theme.items.item.outside in the Days component to use the fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc17130f-4982-4805-9000-2d41b94e07d1
📒 Files selected for processing (2)
packages/ui/src/components/Datepicker/Views/Days.tsxpackages/ui/src/components/Datepicker/theme.ts
|
Hi team! All CI checks are now passing |
SutuSebastian
left a comment
SutuSebastian
left a comment
There was a problem hiding this comment.
remove ajouté comments
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/components/Datepicker/Views/Days.tsx (1)
69-69: 💤 Low valueOptional: include year in the
isOutsidecheck for completeness.Month-only comparison is safe within a 42-cell grid (a same-month-number date from a different year can never appear in the window), but the intent of "not in the same calendar month" is more precisely expressed with both components.
♻️ Proposed fix
- const isOutside = currentDate.getMonth() !== viewDate.getMonth(); + const isOutside = + currentDate.getMonth() !== viewDate.getMonth() || + currentDate.getFullYear() !== viewDate.getFullYear();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/Datepicker/Views/Days.tsx` at line 69, The isOutside boolean in Days.tsx currently only compares currentDate.getMonth() to viewDate.getMonth(); update this to also compare years so it truly reflects "not in the same calendar month" (e.g., set isOutside using currentDate.getMonth() !== viewDate.getMonth() || currentDate.getFullYear() !== viewDate.getFullYear()). Locate the const isOutside that uses currentDate and viewDate and change the expression accordingly so same-month-number from a different year is treated as outside.packages/ui/src/components/Datepicker/theme.ts (1)
53-53: 💤 Low valueOptional: add a dark-mode variant to
outsidefor explicit dark mode styling.
text-gray-400sits in the same Tailwind variant bucket astext-gray-900frombase, sotwMergewill correctly override it in light mode. However,dark:text-whitefrombaselives in a separate variant bucket and is not overridden bytext-gray-400. The net result in dark mode is white text at 40% opacity rather than the intended muted-gray tone.🎨 Proposed fix
- outside: "text-gray-400 opacity-40", + outside: "text-gray-400 opacity-40 dark:text-gray-500",(Note: the existing
disabledkey has the same gap — this can be addressed separately.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/Datepicker/theme.ts` at line 53, Update the Datepicker theme's outside key to include an explicit dark-mode variant so dark-mode uses a muted gray instead of white at reduced opacity; replace or extend the current value ("text-gray-400 opacity-40") with something that adds a dark variant such as "text-gray-400 opacity-40 dark:text-gray-400/40" (adjust classes to match your Tailwind config) in the theme object where the outside key is defined; (note: the disabled key has the same issue and can be fixed similarly).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/ui/src/components/Datepicker/theme.ts`:
- Line 53: Update the Datepicker theme's outside key to include an explicit
dark-mode variant so dark-mode uses a muted gray instead of white at reduced
opacity; replace or extend the current value ("text-gray-400 opacity-40") with
something that adds a dark variant such as "text-gray-400 opacity-40
dark:text-gray-400/40" (adjust classes to match your Tailwind config) in the
theme object where the outside key is defined; (note: the disabled key has the
same issue and can be fixed similarly).
In `@packages/ui/src/components/Datepicker/Views/Days.tsx`:
- Line 69: The isOutside boolean in Days.tsx currently only compares
currentDate.getMonth() to viewDate.getMonth(); update this to also compare years
so it truly reflects "not in the same calendar month" (e.g., set isOutside using
currentDate.getMonth() !== viewDate.getMonth() || currentDate.getFullYear() !==
viewDate.getFullYear()). Locate the const isOutside that uses currentDate and
viewDate and change the expression accordingly so same-month-number from a
different year is treated as outside.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7aaf4457-44df-4e32-82ea-88c35976edbb
📒 Files selected for processing (2)
packages/ui/src/components/Datepicker/Views/Days.tsxpackages/ui/src/components/Datepicker/theme.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/components/Datepicker/theme.ts`:
- Line 53: The "outside" style entry in theme.ts uses "text-gray-400 opacity-40"
but lacks a dark-mode override, so twMerge can't override base's
"dark:text-white"; update the "outside" entry to include a dark-mode muted color
(e.g., add a "dark:text-<muted-gray>" token) so outside days render muted in
dark mode as well — modify the "outside" key in the theme object (alongside the
existing "base" key) to include the dark: text class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 531b52a0-e552-4460-9d53-0bb4de987447
📒 Files selected for processing (2)
packages/ui/src/components/Datepicker/Views/Days.tsxpackages/ui/src/components/Datepicker/theme.ts
2 participants
Problem
Out-of-month days in the Datepicker render with the same
visual emphasis as in-month days, making the calendar
harder to scan.
Solution
outsidestyle key toDatepickerViewsDaysThemeisOutsidevariableopacity-40 text-gray-400to out-of-month daysRelated Issue
Fixes #1647
Summary by CodeRabbit
New Features
Style