Skip to content

Fix Date picker problems caused by different time zones and DST. - #605

Merged
mlaursen merged 1 commit into
mlaursen:release/1.2.xfrom
iporaitech:release/1.2.x
Nov 6, 2017
Merged

Fix Date picker problems caused by different time zones and DST.#605
mlaursen merged 1 commit into
mlaursen:release/1.2.xfrom
iporaitech:release/1.2.x

Conversation

@hisapy

@hisapy hisapy commented Nov 2, 2017

Copy link
Copy Markdown
Contributor

Due to different time zones, the Date picker presented inconsistencies in formatting
and also displaying of days of the month and days of weeks. For example, in some time zones,
when due to DST the time forwards one hour at midnight, the browser simply skips the 00:00
hours. For example, in Paraguay where the time is forwarded 1 hour on Oct 1st,
new Date(2017, 9, 1, 0) returns Sun Oct 01 2017 01:00:00 GMT-0300 (-03).
This was causing problems such as rendering 2017-11-1 on thursday and not on wednesday, and
also broke the active and today displaying in the calendar.

To fix this issues, this commit does the following:

  • Add required string prop timeZone to DatePickerContainer which defaults to UTC.
  • Propagates timeZone prop to child/descendant components such as DatePicker, CalendarHeader, DatePickerCalendar, CalendarMonth and CalendarDate
  • Always strips time when calculating the next date in CalendarMonth to avoid not displaying active/today dates.
  • Updates Date Pickers docs' Formatting section.

Screenshots of the problems taken from the docs page

  1. React console showing the date assigned to the component

react-console

  1. Component opened first time (wrong date and wrong days of the week)

dialog_opened_1st_time

  1. Date picker switched to October. No active or today working; the screenshot was taken on October 30th.

dialog_oct

  1. Date picker switched back to November. All dates displaced one day.

dialog_nov_after_oct

Caveats

  1. Could not make the new timeZone prop to appear in the PROP TYPES section of the docs. How can I do that?
  2. This does not address Time picker issues.

Due to different time zones, the Date picker presented inconsistencies in formatting
and also displaying of days of the month and days of weeks. For example, in some time zones,
when due to DST the time forwards one hour at midnight, the browser simply skips the 00:00
hours. For example, in Paraguay where the time is forwarded 1 hour on Oct 1st,
new Date(2017, 9, 1, 0) returns Sun Oct 01 2017 01:00:00 GMT-0300 (-03).
This was causing problems such as rendering 2017-11-1 on thursday and not on wednesday, and
also broke the active and today displaying in the calendar.

To fix this issues, this commit does the following:

- Add required string prop timeZone to DatePickerContainer which defaults to UTC.
- Propagates timeZone prop to child/descendant components such as DatePicker, CalendarHeader, DatePickerCalendar, CalendarMonth and CalendarDate
- Always strips time when calculating the next date in CalendarMonth to avoid not displaying active/today dates.
- Updates Date Pickers docs' Formatting section.

days.push(date);
currentDate = addDate(currentDate, 1, 'D');
// stripTime again to avoid problems when time is forwarded an hour for DST

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is explained in the commit message.

date: new Date(2016, 1, 1),
disabled: false,
onClick: jest.fn(),
index: 0,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this because noticed that the prop wasn't actually used in the implementation

/**
* The timeZone to be used in all formatting operations.
* For a full list of possible timeZone values check https://www.iana.org/time-zones.
*/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could not make this appear in the PROP TYPES section of the Date Pickers docs. Please help with some guidance or instructions.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something that I need to make a better flow for.. You'd need to run docgen or the docgen:create commands again from the /docs folder to see those updates. I currently don't have hot-reloading set up for server code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I tried all possible commands/scripts from the root dir and docs dir but I'm not sure. I'll take this into account for the next time :)

@mlaursen

mlaursen commented Nov 6, 2017

Copy link
Copy Markdown
Owner

This looks good to me and thanks for updating a few of my tests to use enzyme! I've been meaning to migrate more of them over but never get the time.

@mlaursen mlaursen added this to the v1.2.5 milestone Nov 6, 2017
@mlaursen mlaursen added the bug label Nov 6, 2017
@mlaursen
mlaursen merged commit 8d11c91 into mlaursen:release/1.2.x Nov 6, 2017
@hisapy

hisapy commented Nov 6, 2017

Copy link
Copy Markdown
Contributor Author

No problem, thanks for letting me change those tests ... It would've taken me more time trying to write those tests without enzyme. By the way, I also believe that tests run faster using enzyme than with the react-dom/test-utils.

@hisapy
hisapy deleted the release/1.2.x branch November 6, 2017 16:52
@hisapy
hisapy restored the release/1.2.x branch November 6, 2017 16:52
@johc

johc commented Dec 2, 2017

Copy link
Copy Markdown

Hi,
I noticed that the day of the week headers are incorrect in the calendar - it's shifted forward by a day. My timezone is GMT+13.

image

@hisapy

hisapy commented Dec 4, 2017

Copy link
Copy Markdown
Contributor Author

We're working on this error in #634 ... thanks for the screenshot and your timezone info

mlaursen added a commit that referenced this pull request Feb 7, 2018
This reverts commit 8d11c91, reversing
changes made to 50f7527.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants