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

Fix time formatting with some environments #115

Merged
merged 1 commit into from Aug 1, 2019
Merged

Conversation

etrappg
Copy link
Contributor

@etrappg etrappg commented Jul 31, 2019

Fix time formatting issue with some environments

Fix #112
Fix #114

@phstc
Copy link
Owner

phstc commented Jul 31, 2019

Hi @etrappg

Thanks for submitting this 🎉

Would you mind to add some tests here to cover the formats this change is fixing?

@etrappg
Copy link
Contributor Author

etrappg commented Jul 31, 2019

Unfortunately it's difficult to add test for this case: function which bug is internal (parseTime) and the bug occurs when calling parseDate with a Date object, at line 127 of dateFormat.js, the toTimeString() call result is different with some envs, for example, with date 2019-07-31 15:12:13, I have:

  • on Firefox, and Windows 10: Date Wed Jul 31 2019 15:12:13 GMT+0200 (heure d’été d’Europe centrale), no problem,
  • on Firefox, and Linux Mint: Date Wed Jul 31 2019 15:11:29 GMT+0200 (GMT+02:00), problem with last : of GMT+02:00 timezone

Maybe you have a solution to make this reproducible with tests and node env?

@phstc phstc merged commit 540a0a5 into phstc:master Aug 1, 2019
phstc added a commit that referenced this pull request Aug 1, 2019
But it isn't really testing the problem, since it depends on the browser.
See #115 (comment)
@phstc
Copy link
Owner

phstc commented Aug 1, 2019

@etrappg I see. I think we can leave it as is, since the test suite is still passing, and we have a plenty of use cases in there, so it shouldn't be breaking any existing formatting.

Thank you for addressing this problem 🎉

I've just released a new version 1.0.4 with your fix.

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.

International date line time format Time formatting stopped working with Chrome on Mac
2 participants