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.

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

moment-timezone compatability for DateInput #297

Merged
merged 2 commits into from
Dec 2, 2016

Conversation

mfedderly
Copy link
Contributor

Fixes #280

Changes proposed in this pull request:

  • Properly handle moment-timezone's setDefault
  • DateInput accepts dates in the local timezone
  • DateInput onChange returns dates in the local timezone

Reviewers should focus on:

  • DateInput.handleDateChange method
  • Making sure this works in multiple timezones
  • Making sure this works with and without moment-timezone

@palantirtech
Copy link
Member

Thanks for your interest in palantir/blueprint, @mfedderly! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@@ -320,4 +321,36 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat
private setInputRef = (el: HTMLElement) => {
this.inputRef = el;
}

/**
* Translate a moment into a Date object, adjusting the moment timezone into the local one.
Copy link
Contributor

Choose a reason for hiding this comment

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

where does the adjustment happen? i'm assuming it's inside moment but it's not obvious why this works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Date objects are always in the local timezone. Moment operates in the specified timezone after moment.tz.setDefault has been called (or the local timezone if none has been configured). Therefore directly copying the individual components of the dates corrects for the timezone discrepancy.

* Translate a moment into a Date object, adjusting the moment timezone into the local one.
* This is a no-op unless moment-timezone's setDefault has been called.
*/
private fromMomentToDate = (date: moment.Moment) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

a different name would be helpful to clarify that this is not a Date. momentDate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

@adidahiya adidahiya modified the milestone: 1.2.0 Dec 2, 2016
@llorca llorca removed this from the 1.2.0 milestone Dec 2, 2016
@giladgray giladgray merged commit 478ab73 into palantir:master Dec 2, 2016
@giladgray
Copy link
Contributor

thanks @mfedderly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants