fix: add separate state for calendar selected date #4207
Conversation
This PR introduces a separate state for tracking the selected date by: 1. Adding a `selectedDate` property to the Calendar context 2. Extending the `useCalendarDate` hook to maintain separate states 3. Updating the GridRow component to properly render selected dates based on both date equality and month context
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue in the DatePicker component where the selected date was lost when navigating between months by introducing a separate selected date state.
- Updated DatePicker.tsx to use the new selectedDate state
- Modified useCalendarDate.ts to manage both calendarDate and selectedDate
- Adjusted GridRow.tsx, CalendarProvider.ts, and CalendarContainer.tsx to work with the new selectedDate
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DatePicker/DatePicker.tsx | Updated state handling and selection logic for the DatePicker |
| src/Calendar/hooks/useCalendarDate.ts | Added selectedDate state and setSelectedDate callback with a potential dependency issue |
| src/Calendar/Grid/GridRow.tsx | Updated selection logic to use selectedDate and added month checking |
| src/Calendar/CalendarProvider.ts | Added selectedDate field in Calendar context |
| src/Calendar/CalendarContainer.tsx | Passed selectedDate through context to CalendarContainer |
| @@ -15,6 +16,14 @@ export const useCalendarDate = (value: Date | null | undefined, defaultDate: Dat | ||
| }, | ||
| [calendarDate] | ||
| ); | ||
| const setSelectedDate = useCallback( | ||
| (date: React.SetStateAction<Date> | undefined) => { | ||
| if (date && date?.valueOf() !== selectedDate?.valueOf()) { |
There was a problem hiding this comment.
The dependency array for the setSelectedDate callback only includes 'calendarDate'. It should also include 'selectedDate' to ensure the callback captures the latest selectedDate value.
Copilot uses AI. Check for mistakes.
| @@ -469,7 +467,8 @@ const DatePicker: RsRefForwardingComponent<'div', DatePickerProps> = React.forwa | ||
| * The callback triggered after clicking the OK button. | ||
| */ | ||
| const handleOK = useEventCallback((event: React.SyntheticEvent) => { | ||
| updateValue(event); | ||
| console.log('selectedDate', selectedDate); |
There was a problem hiding this comment.
A debugging statement is present in production code; consider removing it to clean up the output.
| console.log('selectedDate', selectedDate); |
Copilot uses AI. Check for mistakes.
Fix DatePicker calendar selection persistence when changing months
Description
This PR addresses a UX issue in the DatePicker component where the selected date is lost when navigating between months. Currently, when a user selects a date and then navigates to a different month, the calendar treats the navigation date as the selected date, which is inconsistent with user expectations.
I've investigated several popular date picker implementations including MUI, Angular Material, Ant Design, and jQuery UI Datepicker - none of these exhibit this behavior. In all these implementations, the selected date remains distinct from the navigation date when browsing through months.
Proposed Solution
The core issue is that our calendar doesn't properly distinguish between:
calendarDate)selectedDate)This PR introduces a separate state for tracking the selected date by:
selectedDateproperty to the Calendar contextuseCalendarDatehook to maintain separate statesbased on both date equality and month context
Implementation Details
CalendarProvider.tsto includeselectedDatein the contextuseCalendarDate.tsto track bothcalendarDateandselectedDateseparatelyhandleCalendarSelectmethod inDatePicker.tsxto update both statesselectedDateto theCalendarContainerGridRow.tsxto handle both states correctlyNote
I'm submitting this PR as an illustration of the problem and a possible solution approach. This implementation may not be complete as there might be edge cases I haven't addressed. I welcome feedback and suggestions on how to improve this solution or alternative approaches to solve the issue.
Testing