Feat: Day picker with Month and Year selection dropdowns #6391
Conversation
|
|
WalkthroughThis pull request focuses on upgrading the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
web/core/components/inbox/modals/snooze-issue-modal.tsx (1)
51-52: Consider modal-specific styling adjustmentsThe border styling might create unnecessary visual nesting within the modal. Consider removing the border since the modal already provides visual containment.
- classNames={{root: `${defaultClassNames.root} rounded-md border border-custom-border-200 p-3`}} + classNames={{root: `${defaultClassNames.root} rounded-md p-3`}}web/core/components/core/filters/date-filter-modal.tsx (1)
102-103: Optimize duplicate styling configurationBoth DayPicker instances use identical classNames configuration. Consider extracting this into a constant to follow DRY principles.
+ const dayPickerClassNames = {root: `${defaultClassNames.root} border border-custom-border-200 p-3 rounded-md`}; // Then use it in both DayPicker components: <DayPicker - classNames={{root: `${defaultClassNames.root} border border-custom-border-200 p-3 rounded-md`}} + classNames={dayPickerClassNames} captionLayout="dropdown" // ... other props />Also applies to: 125-126
web/styles/react-day-picker.css (1)
33-44: Consider adding transition for smooth hover effectsThe day button styles look good, but adding a transition property would make hover and selection effects smoother.
.rdp-day_button { + transition: background-color 0.2s ease-in-out; display: flex; overflow: hidden; align-items: center; /* ... rest of the properties ... */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/core/components/core/filters/date-filter-modal.tsx(4 hunks)web/core/components/dropdowns/date-range.tsx(3 hunks)web/core/components/dropdowns/date.tsx(3 hunks)web/core/components/inbox/modals/snooze-issue-modal.tsx(3 hunks)web/package.json(1 hunks)web/styles/react-day-picker.css(4 hunks)
🔇 Additional comments (7)
web/core/components/dropdowns/date.tsx (1)
171-171: LGTM! Consistent styling approachThe padding adjustment from Combobox.Options to DayPicker's classNames maintains the same visual spacing while following the new styling pattern.
Also applies to: 179-180
web/core/components/dropdowns/date-range.tsx (2)
209-210: LGTM! Consistent implementationThe changes correctly implement the month/year dropdown feature while maintaining consistency with other date picker components.
4-4: Verify react-day-picker version compatibilityThe upgrade to support month/year dropdowns requires react-day-picker v9.5.0. Let's verify the version and any potential breaking changes.
Also applies to: 2-2, 4-4, 5-5
✅ Verification successful
✓ react-day-picker version is compatible
The project uses react-day-picker ^9.5.0, which supports month/year dropdowns. All components using react-day-picker follow consistent import patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the installed version and dependencies cat package.json | jq '.dependencies["react-day-picker"]' # Look for any other components using react-day-picker that might need updates rg -l "react-day-picker"Length of output: 330
Script:
#!/bin/bash # Check web/package.json for react-day-picker version cat web/package.json | jq '.dependencies["react-day-picker"]' # Check import patterns rg "from ['\"]react-day-picker['\"]" -A 1Length of output: 1015
web/package.json (1)
58-58: Verify breaking changes from react-day-picker v8 to v9This is a major version upgrade that may include breaking changes. Please ensure:
- All breaking changes from v9 are properly handled
- The upgrade is compatible with date-fns v4
Run this script to check the changelog and compatibility:
✅ Verification successful
Breaking changes verification completed - no issues found
The codebase is already properly migrated to react-day-picker v9:
- No usage of breaking changes (dropdown formatters, Persian calendar) found
- CSS customizations are using the correct v9 syntax with proper variables and class names
- No deprecated features or patterns detected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check breaking changes and compatibility # Get the changelog between versions gh api repos/gpbl/react-day-picker/releases | jq -r '.[] | select(.tag_name | startswith("v9.")) | .body' # Check for any compatibility issues mentioned in issues gh api search/issues -f q='repo:gpbl/react-day-picker is:issue label:compatibility date-fns'Length of output: 27564
Script:
#!/bin/bash # Search for dropdown formatter usage echo "=== Searching for dropdown formatter usage ===" rg -l "formatMonthDropdown|formatYearDropdown" -t js -t jsx -t ts -t tsx # Search for Persian/Jalali calendar imports echo -e "\n=== Searching for Persian/Jalali calendar imports ===" rg -l "react-day-picker/(persian|jalali)" -t js -t jsx -t ts -t tsx # Search for custom CSS overrides echo -e "\n=== Searching for DayPicker CSS customizations ===" rg -l "\.rdp-|--rdp-" -t css -t scss -t lessLength of output: 619
Script:
#!/bin/bash # Search for dropdown formatter usage (both old and new patterns) echo "=== Searching for dropdown formatter usage ===" rg "formatMonthDropdown.*\(" --type-add 'web:*.{js,jsx,ts,tsx}' -t web # Search for calendar-related imports and usage echo -e "\n=== Searching for calendar imports and usage ===" rg "from ['\"]react-day-picker/(persian|jalali|utc)" --type-add 'web:*.{js,jsx,ts,tsx}' -t web # Search for DayPicker CSS customizations (both v8 and v9 patterns) echo -e "\n=== Searching for DayPicker CSS customizations ===" fd -e css -e scss -e less | xargs cat 2>/dev/null | grep -E '(rdp-|DayPicker)'Length of output: 3344
web/styles/react-day-picker.css (3)
Line range hint
1-28: LGTM! Well-structured CSS variables and root stylesThe root styles and CSS variables are well-organized, providing good customization options for the date picker.
Line range hint
188-238: LGTM! Comprehensive dropdown stylesThe dropdown styles are well-implemented with proper accessibility features:
- Proper z-index management
- Focus-visible states
- Disabled states
- Hover effects
95-104: Verify the today indicator's visibilityThe today indicator uses a small dot with background color. Ensure it has sufficient contrast against different backgrounds.
✅ Verification successful
Today indicator implementation provides sufficient contrast
The today indicator uses a semi-transparent bright blue dot (rgba(63, 118, 255, 0.5)) that provides good visibility against different backgrounds while maintaining a subtle appearance. The implementation follows proper design system patterns using semantic color variables.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are any reported issues about today indicator visibility # Search for issues related to today indicator visibility gh api search/issues -f q='repo:makeplane/plane is:issue "today indicator" OR "today dot" visibility contrast'Length of output: 309
Script:
#!/bin/bash # Search for rdp-background-color variable definition and usage rg --type css "rdp-background-color" # Search for theme-related styles that might affect the date picker rg --type css "\.rdp.*color|--rdp-.*color" # Look for any theme configuration files fd -e css -e scss "theme|color" --type fLength of output: 2100
Script:
#!/bin/bash # Search for color-primary-100 variable definition rg --type css "color-primary-100" # Look for theme configuration files that might define these colors fd "theme" --type f --exec rg "color-primary"Length of output: 2363
| .rdp-range_start, | ||
| .rdp-range_middle, | ||
| .rdp-range_end { | ||
| position: relative; | ||
| } | ||
|
|
||
| td:has(.rdp-day_range_start)::before, | ||
| td:has(.rdp-day_range_middle)::before, | ||
| td:has(.rdp-day_range_end)::before { | ||
| .rdp-range_start::before, | ||
| .rdp-range_middle::before, | ||
| .rdp-range_end::before { | ||
| content: ""; | ||
| position: absolute; | ||
| background-color: var(--rdp-background-color); | ||
| top: 50%; | ||
| height: 100%; | ||
| width: 50%; | ||
| transform: translate(0, -50%); | ||
| z-index: -1; | ||
| } | ||
|
|
||
| td:has(.rdp-day_range_start)::before { | ||
| .rdp-range_start::before { | ||
| left: 50%; | ||
| } | ||
|
|
||
| td:has(.rdp-day_range_middle)::before { | ||
| .rdp-range_middle::before { | ||
| left: 50%; | ||
| width: 100%; | ||
| transform: translate(-50%, -50%); | ||
| } | ||
|
|
||
| td:has(.rdp-day_range_end)::before { | ||
| .rdp-range_end::before { | ||
| right: 50%; | ||
| } | ||
|
|
||
| td:has(.rdp-day_range_start.rdp-day_range_end)::before { | ||
| .rdp-range_start.rdp-range_end::before { |
There was a problem hiding this comment.
💡 Codebase verification
Use logical properties for better RTL support in date range picker
The date range picker styles currently use physical properties (left/right) which won't work correctly in RTL layouts. Replace them with logical properties:
- Replace
left: 50%withinset-inline-start: 50% - Replace
right: 50%withinset-inline-end: 50%
🔗 Analysis chain
Ensure range selection works with RTL layouts
The range selection uses left/right positioning which might need adjustment for RTL layouts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if RTL support is properly handled
# Search for RTL-related issues or discussions
gh api search/issues -f q='repo:makeplane/plane is:issue rtl "date picker" OR "date range"'
# Check if there are any RTL-specific styles in the codebase
rg -l 'dir="rtl"'
rg -l '[^-]right-to-left'
Length of output: 338
Script:
#!/bin/bash
# Search for RTL configurations and styles with improved patterns
rg -l "direction:\s*rtl"
rg -l "html\[dir=('|\")rtl('|\")]"
rg -l "dir:\s*rtl"
# Search for date picker related files
fd ".*day-picker.*\.(css|scss)$"
fd ".*datepicker.*\.(css|scss)$"
# Check for logical properties usage
rg "margin-(inline|block)|(inline|block)-start|(inline|block)-end"
rg "inset-(inline|block)|(inline|block)-start|(inline|block)-end"
# Check package.json and other config files for RTL related settings
fd "package.json|next.config|tailwind.config"
Length of output: 1280
There was a problem hiding this comment.
Ignoring this as we are positioning cell background here
There was a problem hiding this comment.
@vineetk13 Understood, I'll note this for future reviews.
✏️ Learnings added
Learnt from: vineetk13
PR: makeplane/plane#6391
File: web/styles/react-day-picker.css:249-282
Timestamp: 2025-01-17T05:17:51.812Z
Learning: In the date range picker's CSS, left/right positioning for cell background effects (like in `.rdp-range_start::before`, `.rdp-range_middle::before`, `.rdp-range_end::before`) should use physical properties instead of logical properties, as these create visual effects that should remain consistent regardless of text direction.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
web/styles/react-day-picker.css (2)
179-181: 🛠️ Refactor suggestionRemove !important declarations
The use of !important for hover and focus states should be avoided as it makes styles harder to override and maintain.
.rdp-button_next:hover, .rdp-button_previous:hover, .rdp-button_next:focus-visible, .rdp-button_previous:focus-visible { - background-color: rgba(var(--color-background-80)) !important; + background-color: rgba(var(--color-background-80)); }
249-282: 🛠️ Refactor suggestionUse logical properties for better RTL support
Replace physical properties (left/right) with logical properties to ensure proper functionality in RTL layouts.
.rdp-range_start::before { - left: 50%; + inset-inline-start: 50%; } .rdp-range_middle::before { - left: 50%; + inset-inline-start: 50%; width: 100%; transform: translate(-50%, -50%); } .rdp-range_end::before { - right: 50%; + inset-inline-end: 50%; }
🧹 Nitpick comments (3)
web/styles/react-day-picker.css (3)
Line range hint
1-27: Merge duplicate.rdp-rootselectorsThe
.rdp-rootselector is defined twice. Merge the properties from lines 24-27 into the first declaration block to improve maintainability..rdp-root { font-size: 12px; + position: relative; + box-sizing: border-box; --rdp-cell-size: 40px; /* Size of the day cells. */ --rdp-caption-font-size: 1.15rem; /* Font size for the caption labels. */ --rdp-caption-navigation-size: 1.25rem; /* Font size for the caption labels. */ --rdp-accent-color: rgba(var(--color-primary-100)); /* Accent color for the background of selected days. */ --rdp-background-color: rgba(var(--color-primary-100), 0.5); /* Background color for the hovered/focused elements. */ --rdp-dark-background-color: rgba(var(--color-primary-300)); /* Background color for the hovered/focused, already selected elements. */ --rdp-outline: 2px solid var(--rdp-accent-color); /* Outline border for focused elements */ --rdp-selected-color: #ffffff; /* Color of selected day text */ background: transparent; } -.rdp-root { - position: relative; - box-sizing: border-box; -}
47-52: Standardize opacity values for consistencyThe opacity values for outside (0.5) and disabled (0.25) days are magic numbers. Consider using CSS variables for these values to maintain consistency and make future updates easier.
.rdp-root { /* ... existing variables ... */ + --rdp-outside-opacity: 0.5; + --rdp-disabled-opacity: 0.25; } .rdp-day.rdp-outside:not(.rdp-selected) .rdp-day_button { - opacity: 0.5; + opacity: var(--rdp-outside-opacity); } .rdp-day.rdp-disabled:not(.rdp-selected) .rdp-day_button { - opacity: 0.25; + opacity: var(--rdp-disabled-opacity); }
188-193: Remove commented codeRemove the commented
width: 100%property as it adds noise to the codebase. If this property might be needed in the future, document it in a separate technical document or issue..rdp-dropdowns { position: relative; - /* width: 100%; */ display: inline-flex; align-items: center; }
|
Hi @aaryan610, any feedback or suggestions here? |
Description
Date pickers now have year and month selection dropdowns for easy and quick navigation. This addition requires upgrading
react-day-pickerpackage to version^9.5.0, because this latest version now supports both button navigation and caption dropdown effectively. Additionally, the styling classnames are better named and standardized for easy customizations.Type of Change
Screenshots and Media
References
Issue -> #6389
Summary by CodeRabbit
Dependency Update
react-day-pickerlibrary from version 8.10.0 to 9.5.0New Features
Style Changes