adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.8k stars 1.1k forks source link

Selecting a selected date for a calendar using DateTime has a different behaviour to a Date. #4750

Open FredsW47 opened 1 year ago

FredsW47 commented 1 year ago

πŸ› Bug Report

When selecting a date using useDatePicker, selecting the already selected date hides the calendar, however this behaviour is different for a DateTime. When selecting the selected date with DateTime this does not hide the calendar. Therefore no interaction is recorded.

The affect: When using a placeholder date in the calendar (for example highlighting todays date when the user first opens the calendar) means the user can not select that date and therefore the text field will not update.

Case One:

https://github.com/adobe/react-spectrum/assets/117086272/eec404fa-39b5-4e86-aef8-fd7f574772df

Case Two:

https://github.com/adobe/react-spectrum/assets/117086272/011a12b0-8bed-4abf-be75-79c471aeff29

πŸ€” Expected Behavior

The behaviour when using a DateTime should be the same as when using a Date.

😯 Current Behavior

The behaviour can be seen on the React Aria documentation.

When selecting the selected date for the example date, the calendar closes (Example) When selecting the selected date for Timezones, the calendar does not close. (Example)

πŸ”¦ Context

We have implemented both calendars within our environment and we see the same behaviour occur.

πŸ’» Code Sample

The behaviour can be seen on the React Aria documentation.

Can use the code sandbox in the documentation: ((Date), (DateTime))

🌍 Your Environment

Issue occurs using your code sandbox,

Our Environment:

LFDanLu commented 1 year ago

Did some digging, this seems to happen specifically for DatePickers that are using ZonedDateTimes. The code that closes the Popover on date selection relies on being called as the onChange from the setControlledValue here. For CalendarDates, the else part of the if statement is triggered and the newValue is always different from the current value even when the dates are the same since they are different objects and thus passes the check here. For CalendarDateTime, we trigger this part of the if statement, and .set() always returns a new CalendarDateTime due to this .copy call.

However, for a ZonedDateTime we actually return the same object via .set(), resulting in useControlledState deciding not to call onChange hence why the popover doesn't close. Perhaps this useCalendarState line can be changed to setControlledValue(value.copy().set(newValue)); or we could update ZonedDateTime's .set to actually always return a new object via .copy(). Haven't tested this thoroughly so not sure of the ramifications