-
Notifications
You must be signed in to change notification settings - Fork 74
[Feat]: Date picker settings init #3586
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
- Replace settings button with Button component - Add click handler to initialize current month - Set date range to current month on settings click - Improve button styling and accessibility
- Use subDays to include today in disabled range - Only allow selection of past dates
- Use startOfDay to properly disable all future dates - Replace static disabled range with dynamic date comparison - Ensure consistent behavior across timezones
- Reduce calendar to single month view - Make buttons and spacing more compact - Decrease text size and button heights - Add shadow for better visual hierarchy - Optimize layout for smaller screens
- Initialize date range with current month instead of last month - Set calendar view to current month by default - Remove unnecessary lastMonth calculation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe changes update the date range picker component in the team dashboard. The state is now initialized with the start and end of the current month using a function. New dependencies, such as Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DateRangePicker
participant Calendar
User->>DateRangePicker: Open date range picker
DateRangePicker->>DateRangePicker: Initialize state (default month start/end)
DateRangePicker->>Calendar: Render calendar with updated layout & styles
User->>DateRangePicker: Select date range and interact with buttons
DateRangePicker->>User: Update display with the chosen date range
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
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 (
|
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: 0
🔭 Outside diff range comments (1)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (1)
241-260
: Add error handling for date range changes.The Clear and Apply buttons should handle potential errors during state updates.
<Button variant="outline" size="sm" onClick={() => { + try { handleDateRangeChange(undefined); setIsPopoverOpen(false); + } catch (error) { + console.error('Error clearing date range:', error); + // Show error notification to user + } }} > {t('common.CLEAR')} </Button> <Button size="sm" onClick={() => { + try { setIsPopoverOpen(false); + } catch (error) { + console.error('Error applying date range:', error); + // Show error notification to user + } }} > {t('common.APPLY')} </Button>
🧹 Nitpick comments (1)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (1)
178-196
: Consider adding loading state for async operations.The settings button triggers state updates and opens the popover. Consider adding a loading state to prevent multiple clicks during state updates.
<Button variant="ghost" + disabled={isLoading} onClick={() => { + setIsLoading(true); const today = new Date(); setCurrentMonth(today); handleDateRangeChange({ from: startOfMonth(today), to: endOfMonth(today) }); setIsPopoverOpen(true); + setIsLoading(false); }} title="Open date settings" aria-label="Open date settings" size="icon" className="flex items-center justify-center w-[36px] h-[36px] bg-white dark:bg-dark--theme-light border-l border-l-[#E4E4E7] dark:border-l-[#2D2D2D] rounded-r-md hover:bg-gray-50 dark:hover:bg-gray-800" > - <SettingsIcon /> + {isLoading ? <Spinner size="sm" /> : <SettingsIcon />} </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx
(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (3)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/date-range-picker.tsx (3)
35-41
: LGTM: Improved state initialization with meaningful defaults.The dateRange state now initializes with the current month's range, providing a better user experience with meaningful default values.
174-177
: LGTM: Enhanced visual consistency with standard calendar icon.The use of CalendarIcon from Radix UI improves visual consistency and accessibility.
207-220
: Verify date restriction logic.The calendar disables future dates using
startOfDay
, which is correct for preventing future selections. However, ensure this aligns with business requirements.
Description
#3251
Please include a summary of the changes and the related issues.
Type of Change
Checklist
Previous screenshots
Please add here videos or images of the previous status
Current screenshots
Please add here videos or images of the current (new) status
Summary by CodeRabbit
New Features
Style