Update Datepicker.svelte #1494
Update Datepicker.svelte #1494
Conversation
fix days of previous month shown in wrong order
|
@cimagle is attempting to deploy a commit to the Themesberg Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request involve a modification to the Changes
Possibly related PRs
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 (
|
|
the datepicker is showing the previous months dates wrong |
![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
🧹 Outside diff range and nitpick comments (2)
src/lib/datepicker/Datepicker.svelte (2)
Line range hint
93-107: Consider improving function organization and documentation.The
getDaysInMonthfunction handles multiple responsibilities and its name doesn't fully reflect its complete functionality. Consider these improvements:
- Rename to better reflect its purpose (e.g.,
generateCalendarGrid)- Split into smaller, focused functions
- Add JSDoc comments explaining the logic
Here's a suggested refactor:
- function getDaysInMonth(date: Date): Date[] { + /** + * Generates a complete calendar grid for the given month, including padding days + * from adjacent months to ensure full weeks. + * @param date - Any date within the target month + * @returns Array of dates representing the calendar grid + */ + function generateCalendarGrid(date: Date): Date[] { const year = date.getFullYear(); const month = date.getMonth(); + return [ + ...getPreviousMonthPadding(year, month), + ...getCurrentMonthDays(year, month), + ...getNextMonthPadding(year, month) + ]; + } + + /** Get padding days from the previous month */ + function getPreviousMonthPadding(year: number, month: number): Date[] { const firstDay = new Date(year, month, 0); - const lastDay = new Date(year, month + 1, 0); const daysArray: Date[] = []; let start = firstDay.getDay() - firstDayOfWeek; if (start < 0) start += 7; for (let i = 0; i < start; i++) { daysArray.unshift(new Date(year, month, -i)); } + return daysArray; + } - // Add days of the current month + /** Get all days in the current month */ + function getCurrentMonthDays(year: number, month: number): Date[] { + const lastDay = new Date(year, month + 1, 0); + const daysArray: Date[] = []; for (let i = 1; i <= lastDay.getDate(); i++) { daysArray.push(new Date(year, month, i)); } + return daysArray; + } - // Add days from next month to fill the last week + /** Get padding days from the next month */ + function getNextMonthPadding(year: number, month: number): Date[] { + const currentDays = getCurrentMonthDays(year, month); + const previousDays = getPreviousMonthPadding(year, month); + const daysArray: Date[] = []; - const remainingDays = 7 - (daysArray.length % 7); + const remainingDays = 7 - ((currentDays.length + previousDays.length) % 7); if (remainingDays < 7) { for (let i = 1; i <= remainingDays; i++) { daysArray.push(new Date(year, month + 1, i)); } } return daysArray; }
Line range hint
308-316: Enhance calendar grid accessibility.While the component has good accessibility support, consider these improvements for the calendar grid:
- Add
aria-labelto the calendar grid- Add
aria-current="date"for today's date- Add
aria-disabledfor dates from adjacent months- <div class="grid grid-cols-7 gap-1" role="grid"> + <div + class="grid grid-cols-7 gap-1" + role="grid" + aria-label={`Calendar for ${currentMonth.toLocaleString(locale, { month: 'long', year: 'numeric' })}`}> {#each weekdays as day} <div class="text-center text-sm font-medium text-gray-500 dark:text-gray-400" role="columnheader">{day}</div> {/each} {#each daysInMonth as day} - <Button color={isSelected(day) ? color : 'alternative'} size="sm" class="w-full h-8 {day.getMonth() !== currentMonth.getMonth() ? 'text-gray-300 dark:text-gray-600' : ''} {isToday(day) ? 'font-bold' : ''} {isInRange(day) ? getRangeBackgroundClass(color) : ''}" on:click={() => handleDaySelect(day)} on:keydown={handleCalendarKeydown} aria-label={day.toLocaleDateString(locale, { weekday: 'long', year: 'numeric', month: 'long', day: 'numeric' })} aria-selected={isSelected(day)} role="gridcell"> + <Button + color={isSelected(day) ? color : 'alternative'} + size="sm" + class="w-full h-8 {day.getMonth() !== currentMonth.getMonth() ? 'text-gray-300 dark:text-gray-600' : ''} {isToday(day) ? 'font-bold' : ''} {isInRange(day) ? getRangeBackgroundClass(color) : ''}" + on:click={() => handleDaySelect(day)} + on:keydown={handleCalendarKeydown} + aria-label={day.toLocaleDateString(locale, { weekday: 'long', year: 'numeric', month: 'long', day: 'numeric' })} + aria-selected={isSelected(day)} + aria-current={isToday(day) ? 'date' : undefined} + aria-disabled={day.getMonth() !== currentMonth.getMonth()} + role="gridcell"> {day.getDate()} </Button> {/each} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/lib/datepicker/Datepicker.svelte(1 hunks)
🔇 Additional comments (1)
src/lib/datepicker/Datepicker.svelte (1)
95-95: LGTM! The fix correctly addresses the previous month days ordering issue.
The change from push to unshift ensures that days from the previous month are added in chronological order. Previously, using push was causing the days to appear in reverse order since we were iterating through them backwards.
A few recommendations to ensure robustness:
- Consider adding test cases specifically for the calendar grid's first week to verify the ordering
- Update the component's documentation to mention this fix if there was a known issue being tracked
Let's verify the calendar grid's first week behavior:
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
fix days of previous month shown in wrong order
Closes #
📑 Description
Status
✅ Checks
ℹ Additional Information
Summary by CodeRabbit
This change enhances the user experience by ensuring that the calendar view is more intuitive and visually organized.