Skip to content
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.

Sign up for GitHub

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

Jump to bottom

input-date-picker: selecting day activates previous day in certain cases #9422

Closed
2 of 6 tasks
eriklharper opened this issue May 24, 2024 · 5 comments
Closed
2 of 6 tasks

input-date-picker: selecting day activates previous day in certain cases #9422

eriklharper opened this issue May 24, 2024 · 5 comments
Assignees
@geospatialem @DitwanP
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (dev) Issues logged by Calcite developers. calcite-components Issues specific to the @esri/calcite-components package. estimate - 3 A day or two of work, likely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive
Milestone
2024-06-25 - Jun ...

Comments

@eriklharper
Copy link
Contributor

eriklharper commented May 24, 2024 •
edited
Loading

Check existing issues

Actual Behavior

When I pick a date from earlier times ( e.g. ~1850) from the DatePicker, event.target.value returns a day before the selected date. event.target.valueAsDate returns the correct value.

CEST.Date.Picker.Bug.mov

Expected Behavior

event.target.value returns the equivalent year, month and day as valueAsDate.

Reproduction Sample

https://codepen.io/nsulzberger_esri/pen/gOJMjyd (input-date-picker)
https://codepen.io/nsulzberger_esri/pen/rNgLKMK (date-picker)

Reproduction Steps

  1. Change your system timezone to Europe/Zurich
  2. Open the linked codepen
  3. Select March 7th, 1850 from the Date Picker popup
  4. Result: March 6th, 1850 is selected instead of March 7th.

Reproduction Version

2.8

Relevant Info

The current implementation uses the dateToISO util function which relies on Date.prototype.toISOString method on the Date object stored in valueAsDate. toISOString returns the date in UTC, not in the local time which is what will be reflected in valueAsDate, which can lead to discrepancies like this where the two values won't match. Since valueAsDate is a Date object and will thus always format in local time, we should avoid using toISOString for computing value.

We won't be able to add spec test coverage for changing the dateToISO because we can't emulate timezones in the spec environment. We should be able to test in specific timezones in the e2e environment.

Regression?

No response

Priority impact

impact - p3 - not time sensitive

Impact

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components

Esri team

Calcite (dev)
@eriklharper eriklharper added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels May 24, 2024
@github-actions github-actions bot added calcite-components Issues specific to the @esri/calcite-components package. Calcite (dev) Issues logged by Calcite developers. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive labels May 24, 2024
@eriklharper eriklharper mentioned this issue May 24, 2024
Closed
@eriklharper eriklharper self-assigned this May 24, 2024
@eriklharper eriklharper added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels May 24, 2024
@eriklharper eriklharper mentioned this issue May 24, 2024
Merged
@eriklharper eriklharper removed the needs triage Planning workflow - pending design/dev review. label May 24, 2024
@nsulzberger
Copy link

@eriklharper I am not sure, but I think the ticket description is not quite correct, the title is right.
date-picker has the behavior you are describing. value contains the day before, but valueAsDate is fine. So users can work around that.

The bad issue is in the input-date-picker, where the codepen is here: https://codepen.io/nsulzberger_esri/pen/gOJMjyd

2024-05-30_22-12-57
Here valueAsDate also contains the wrong value.

@eriklharper
Copy link
Contributor Author

eriklharper commented Jun 5, 2024 •
edited
Loading

@eriklharper I am not sure, but I think the ticket description is not quite correct, the title is right. date-picker has the behavior you are describing. value contains the day before, but valueAsDate is fine. So users can work around that.

The bad issue is in the input-date-picker, where the codepen is here: https://codepen.io/nsulzberger_esri/pen/gOJMjyd

Thanks Nicole for reporting that! I've added some tests and verification for input-date-picker too, which is also fixed by the changes I made.

@eriklharper eriklharper added this to the 2024-06-25 - Jun Release milestone Jun 10, 2024
@geospatialem geospatialem added p - medium Issue is non core or affecting less that 60% of people using the library labels Jun 10, 2024
@eriklharper eriklharper added estimate - 3 A day or two of work, likely requires updates to tests. and removed p - medium Issue is non core or affecting less that 60% of people using the library labels Jun 10, 2024
eriklharper added a commit that referenced this issue Jun 10, 2024
@eriklharper
fix(input-date-picker, date-picker): ensure day selection doesn't act…
ab77212 
…ivate the previous day in certain scenarios (#9424)

**Related Issue:** #9422 

## Summary

This fixes an issue where in some cases selecting a date in the
date-picker popup of `input-date-picker` would select the previous day
instead of the one selected.
@eriklharper eriklharper added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Jun 10, 2024
@github-actions GitHub Actions
Copy link
Contributor

Installed and assigned for verification.

@github-actions github-actions bot assigned DitwanP and unassigned eriklharper Jun 10, 2024
@eriklharper eriklharper mentioned this issue Jun 10, 2024
Merged
eriklharper added a commit that referenced this issue Jun 11, 2024
@eriklharper
test(date-picker): removing describe.only and skipped test missed in …
4d7089d 
…PR for #9422 (#9555)

**Related Issue:** #9422 

## Summary

This PR removes a skipped test change and a `describe.only` test.
@DitwanP
Copy link
Contributor

DitwanP commented Jun 12, 2024

🍭 Verified locally on dev

Screen.Recording.2024-06-12.at.12.29.47.PM.mov

@DitwanP DitwanP closed this as completed Jun 12, 2024
@DitwanP DitwanP added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Jun 12, 2024
@nsulzberger
Copy link

Cool, thanks for fixing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

@DitwanP DitwanP

Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (dev) Issues logged by Calcite developers. calcite-components Issues specific to the @esri/calcite-components package. estimate - 3 A day or two of work, likely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive
Projects
None yet
Milestone
2024-06-25 - Jun Release
Development

No branches or pull requests
4 participants
@eriklharper @nsulzberger @geospatialem @DitwanP