[date-picker] Selecting today doesn't always update the selected day indicator

juuso-vaadin commented 1 month ago

Description

When current day is scrolled away from the view and "Today" button is clicked, the visual indicator for selected day isn't updated even though the value on the field is.

https://github.com/vaadin/web-components/assets/19607782/e7b13f76-b994-43ae-894c-bb966f4565aa

Expected outcome

Selected day indicator in calendar should always match the value in the input.

Minimal reproducible example

Component demo in docs

Steps to reproduce

Open date picker overlay
Scroll current date out of view
Click "Today"

Environment

Vaadin version(s): 24

Browsers

Chrome

rolfsmeds commented 1 month ago

So, the way the DatePicker calendar works is that when you click Today, if the today-date is currently in the calendar's viewport, it selects it and closes the overlay, but if the today-date is not currently in the calendar's viewport, it will instead focus the date and scrolls it into the viewports calendar.

This seems to have been a design decision back when the DatePicker was created. The question now is if we should change that decision so that the behavior of the Today button would be more consistent.

@juuso-vaadin , @jouni do you have opinions on this?

juuso-vaadin commented 1 month ago

if the today-date is not currently in the calendar's viewport, it will instead focus the date and scrolls it into the viewports calendar.

It doesn't only focus the current date. Clicking Today will always set the value. To me this is a bug as the active value and visualization don't stay in sync.

rolfsmeds commented 1 month ago

Ah, indeed, but it's not just the Today button: focusing a date (manually by keyboard) also selects it (albeit without committing it to the field and closing the overlay). Observe the value in the field changing as you select:

https://github.com/vaadin/web-components/assets/4611559/4d11be9f-1cf1-480a-8e4e-f7c33bd0023f

In the DP calendar, focusing a date actually selects that date in the overlay, and that selection is committed to the field when you click outside the overlay or press enter. This means that

The focus ring matches the date currently selected in the overlay and displayed in the field
The blue background state actually means the date currently committed to the field (which may not be what is currently shown in the field in case another date has been focused)
Clicking Today when today's date is outside the viewport focuses today's date, which also selects it in the overlay (but does not commit it to the field).

So, the visual indication that happens when you click Today is consistent with how the calendar overlay behaves in general, but that behavior may be rather unintuitive and should probably be reconsidered.

juuso-vaadin commented 1 month ago

Ok, I missed the difference between selecting and committing. Overall I haven't heard the current behavior being an issue. Now it just seems a bit unexpected when reviewed critically.

I originally started looking into the Today button as a customer was asking why it sometimes closes the overlay but not always. Seems that if the overlay is scrolled even slightly, first click on Today will center to current date and second click will close overlay. Maybe the expectation would be to close the overlay on click of Today if the current date is already within view area.

web-padawan commented 1 month ago

Here's the logic in the overlay content where we detect whether to select and close, or scroll into view:

https://github.com/vaadin/web-components/blob/66675ccdf6a23090f14079eb6c6874f68274cd2b/packages/date-picker/src/vaadin-date-picker-overlay-content-mixin.js#L536-L543

vaadin / web-components