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

resolves bug on days when DST is added. #2009

Merged
merged 4 commits into from
Oct 5, 2016

Conversation

kylethielk
Copy link
Contributor

On days when DST is added this results in getDate call returning the incorrect day.

As an example on October 2, 2016 many parts of Australia move from AEST to AEDT. Clicking on October 2, 2016 and then triggering datepicker('getDate') results in October 1, 2016 being returned.

Here is a JSFiddle demonstrating the issue (with instructions): https://jsfiddle.net/kylethielk/x6eafu3g/

Here is a JSFiddle with my proposed changes that resolve the issue:
https://jsfiddle.net/kylethielk/aea7jobw/2/

Let me know if you have any questions.

@acrobat
Copy link
Member

acrobat commented Sep 17, 2016

@vsn4ik Looks good to me, what do you think?

@vsn4ik
Copy link
Member

vsn4ik commented Sep 17, 2016

Can't reproduce issue. I can not find a city with AEDT.

@kylethielk
Copy link
Contributor Author

@vsn4ik are you on Mac? Try Point Cook, Australia. It is AEST and on October 2nd, it switches to AEDT.

screen shot 2016-09-18 at 11 10 56 am

Copy link
Member

@vsn4ik vsn4ik left a comment

Choose a reason for hiding this comment

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

Yes, I reproduced the problem. Complicated decision, but the best I can not offer.

@@ -528,7 +528,20 @@
},

_utc_to_local: function(utc){
return utc && new Date(utc.getTime() + (utc.getTimezoneOffset()*60000));

Copy link
Member

Choose a reason for hiding this comment

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

Excess line.

return utc && new Date(utc.getTime() + (utc.getTimezoneOffset()*60000));

if (!utc)
{
Copy link
Member

@vsn4ik vsn4ik Sep 18, 2016

Choose a reason for hiding this comment

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

{ move to the previous line.

var local = new Date(utc.getTime() + (utc.getTimezoneOffset() * 60000));

if (local.getTimezoneOffset() !== utc.getTimezoneOffset())
{
Copy link
Member

Choose a reason for hiding this comment

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

{ move to the previous line.

local = new Date(utc.getTime() + (local.getTimezoneOffset() * 60000));
}

return utc && local;
Copy link
Member

Choose a reason for hiding this comment

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

You can simply return local;

@kylethielk
Copy link
Contributor Author

@vsn4ik I'm not sure I understand what you mean by

Complicated decision, but the best I can not offer.

Are you saying you accept the pull request with the suggested changes? Do you want me to submit the changes?

@vsn4ik
Copy link
Member

vsn4ik commented Sep 22, 2016

@kylethielk LGTM! It is necessary to correct the remarks.

@acrobat
Copy link
Member

acrobat commented Sep 28, 2016

@kylethielk can you fix the comments from @vsn4ik. After those fixes the PR is ready!

@kylethielk
Copy link
Contributor Author

Sorry for the delay @vsn4ik and @acrobat I have submitted suggested changes.

@acrobat acrobat added this to the 1.7.0 milestone Oct 5, 2016
@acrobat acrobat added the is-bug label Oct 5, 2016
@acrobat
Copy link
Member

acrobat commented Oct 5, 2016

Thanks @kylethielk!

@acrobat acrobat merged commit 93a414e into uxsolutions:master Oct 5, 2016
@acrobat acrobat mentioned this pull request May 1, 2017
cpsievert added a commit to rstudio/shiny that referenced this pull request Aug 19, 2020
…e for bootstraplib theming

Note also that the 000 patch is no longer relevant as 1.9.0 includes the same fix uxsolutions/bootstrap-datepicker#2009
cpsievert added a commit to rstudio/shiny that referenced this pull request Aug 26, 2020
…e for bootstraplib theming

Note also that the 000 patch is no longer relevant as 1.9.0 includes the same fix uxsolutions/bootstrap-datepicker#2009
cpsievert added a commit to rstudio/shiny that referenced this pull request Sep 9, 2020
* upgrade bootstrap-datepicker from 1.6.4 to 1.9.0; setup infrastructure for bootstraplib theming

Note also that the 000 patch is no longer relevant as 1.9.0 includes the same fix uxsolutions/bootstrap-datepicker#2009

* Patch sass code for BS4 support and more general color contrasting

* Wrap sass compilation into reusable function

* remove check warning

* Have bootstrapPage() use bootstraplib

* yarn build

* Use new output_template()

* Deprecate bootstrapLib() in favor of bootstraplib::bootstrap()

* Require bootstraplib 0.1.0.9001

* Sync up DESCRIPTION

* document

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

Successfully merging this pull request may close these issues.

3 participants