Skip to content

Moment.JS support#5

Open
StackTheFennec wants to merge 7 commits into
gampleman:masterfrom
StackTheFennec:master
Open

Moment.JS support#5
StackTheFennec wants to merge 7 commits into
gampleman:masterfrom
StackTheFennec:master

Conversation

@StackTheFennec
Copy link
Copy Markdown

Moment.JS is better at dates.
This PR removed strftime.js and added support for i18n month and week names and also time management in browser's timezone.

Signed-off-by: Yuri Mikhaylov <me@yurijmi.ru>
Signed-off-by: Yuri Mikhaylov <me@yurijmi.ru>
Signed-off-by: Yuri Mikhaylov <me@yurijmi.ru>
@gampleman
Copy link
Copy Markdown
Owner

Hi @yurijmi and thanks for the PR.

Using the browsers timezone is a bit tricky, since that may or may not be the same as the server timezone which has the IceCube objects (this is the main reason we were enforcing UTC). It might be worth having the timezone that is enforced be user configurable perhaps.

@gampleman
Copy link
Copy Markdown
Owner

Also the develop cake task should include the moment library.

@StackTheFennec
Copy link
Copy Markdown
Author

@gampleman I think Moment has an option for that.
Oh, forgot that. How can I add that? Download from momentjs.com or?

@StackTheFennec
Copy link
Copy Markdown
Author

StackTheFennec commented Jun 1, 2016

The timezone is being sent to server. So no confusing about that.
Current time from my Safari: 2016-06-02T00:17:59+03:00

@gampleman
Copy link
Copy Markdown
Owner

Hotlinking from somewhere is probably the easiest. I use that task for manually deving/testing.

@StackTheFennec
Copy link
Copy Markdown
Author

Done

@gampleman
Copy link
Copy Markdown
Owner

So let's work through a few timezone hypotheticals (since timezones are from the programmers point of view one of the more terrible inventions of mankind).

Say the server time is running in PDT/UTC-8. My laptop is running in CET/UTC+1. An event coming from the server has a start time at 4pm. What will I see as a user? What would I expect to see as a user? I think there are two possible answers here. Either I want to see 4pm (i.e. I care about the time the server sent me - this happens to be my usecase, as I am editing the times from the perspective of where the event takes place, which happens to be server time) or I want to see 1 AM (i.e. I want to see the time that the event will take place in my timezone.

The (admittedly bad) way we solved this before was by forcing all times to UTC. I agree that is a hacky solution, but removing it with no replacement makes this library somewhat unusable to me.

@StackTheFennec
Copy link
Copy Markdown
Author

Yep, timezone is a bad thing. The user will always see their timezone. E.g. Server at +1, client at +3: 01:00, 03:00.
You can use moment.utc().format(); instead of moment.format(); to force UTC+0

But moment is a trusted library and handles zones quite well

@StackTheFennec
Copy link
Copy Markdown
Author

StackTheFennec commented Jun 1, 2016

Anyways, if you don't believe me, you can try to parse my browser time:

Time.parse("2016-06-02T00:47:18+03:00")

As long as user has correct time on their device - everything should be okay.

Edit: that's rails syntax, not plain ruby.

@gampleman
Copy link
Copy Markdown
Owner

OK, another example (note that this is based on an issue I did encounter with real users unbelievable as it may be).

User A is in the UK (UTC) but during DST. He creates an event that uses his TZ (so +1). He then edits it the next day but is surprised that the event is an hour later than he saved it the day before (since DST has passed since).

I think we should provide an options, say forceUTC to the configuration object that will use moment.utc rather than moment.

@StackTheFennec
Copy link
Copy Markdown
Author

Yeah, I get you. Never encountered but I heard about.
It's a great idea, and I kinda was pointing to a solution like this a comment ago.

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.

2 participants