Skip to content

Conversation

jamieklassen
Copy link
Member

fixes #2226

I've chosen to approach this PR as essentially a bugfix, as I took a shortcut and always showed UTC when upgrading to elm 0.19.

The task that requests the current time zone from the browser can fail. In this case we default to showing UTC. I think it may be worthwhile in general to surface the timezone in various places where we show timestamps, or at least to give some explanation of the default choice of UTC in the failure case. However I'd like to keep that out of the scope of this PR.

#2226

Signed-off-by: Jamie Klassen <cklassen@pivotal.io>
Signed-off-by: Jamie Klassen <cklassen@pivotal.io>
Copy link
Member

@vito vito left a comment

Choose a reason for hiding this comment

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

👍 makes sense to treat this like a bugfix

also confirmed that the timestamps were wrong before and are now correct with this change:

image

image


&::before {
Copy link
Member

Choose a reason for hiding this comment

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

🤔 I can't remember why this was done this way before, but it seems to be fine either way. Looks like this makes it possible to test, so 👍

@vito vito merged commit 54850b7 into master Apr 2, 2019
@jamieklassen jamieklassen deleted the issue/2226 branch April 3, 2019 21:13
@vito vito added the release/no-impact This is an issue that never affected released versions (i.e. a regression caught prior to shipping). label Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/no-impact This is an issue that never affected released versions (i.e. a regression caught prior to shipping). web-ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting the Time Zone
2 participants