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

Inconsistent use of time formatting #184

Closed
michaelarnauts opened this issue Jun 17, 2015 · 20 comments
Closed

Inconsistent use of time formatting #184

michaelarnauts opened this issue Jun 17, 2015 · 20 comments

Comments

@michaelarnauts
Copy link
Contributor

The format of time is inconsistent in the interface. We use 24 hour style here. I assume this should be configured trough the Timezone parameter in the configuration. This is set to Europe/Brussels.

The sun card:
image
The timeline at the top is displaying correct 24-hour style. The information at the bottom is showing 12-hour style.

Logbook:
image
The time in the logbook is in 12-hour style.

Timeline popover:
image
The timeline itself is displayed fine, but the popup is in 12-hour style again.

@balloob balloob added the bug label Jun 19, 2015
@balloob
Copy link
Member

balloob commented Jun 19, 2015

The logbook and rising message in the sun card are actually being rendered using browser locale aware formatting. So for you seeing the AM/PM times means that is what your browser prefers. You can check your browser settings using these instructions.

That being said, you're right that it is still inconsistent. It looks like I have hard coded the history to support the 24h system instead of being locale aware.

@michaelarnauts
Copy link
Contributor Author

Hmm, strange, my browser is set up for the Dutch language:

image

I also have the same behavior on my Android device. Are your sure it's not the 12h system that's hardcoded?

@michaelarnauts
Copy link
Contributor Author

I've digged into it a bit, and don't you need to specify the current locate to the moment library? It seems to be using LT what is the shortcode for locale aware time, but by default moment is using english.

window.hass.uiUtil.formatTime = function(dateObj) {
  return moment(dateObj).format('LT');
};

See http://momentjs.com/docs/#/i18n/changing-locale/

@balloob
Copy link
Member

balloob commented Jun 20, 2015

Ah dang, it seems like you're right.

If only Apple would implement toLocaleDateString and toLocaleTimeString, it would make our lives so much easier.

@infamy
Copy link
Contributor

infamy commented Jun 2, 2016

Still present as of 0.20.x

@CoalaJoe
Copy link

Is this still a thing?

@michaelarnauts
Copy link
Contributor Author

Yep, still the same.

@CoalaJoe
Copy link

I think moment.js could handle this. But I don't think we want add a new dependency.

@michaelarnauts
Copy link
Contributor Author

AFAIK, moment is already a dependency. Check my comment above. It just isn't initialised correctly.

@balloob
Copy link
Member

balloob commented Oct 27, 2016

We removed moment, it is way too big for what it does. We use JavaScript
native toLocaleString and toLocaleDateString.

On Wed, Oct 26, 2016 at 10:38 PM, Michaël Arnauts notifications@github.com
wrote:

AFAIK, moment is already a dependency. Check my comment above. It just
isn't initialised correctly.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#184 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABYJ2tCH_Hmk_4mNHcQvhTWExJCiYpXrks5q4DjOgaJpZM4FFpdB
.

PaulusSchoutsen.nl
It's nice to be important but it's more important to be nice.

@michaelarnauts
Copy link
Contributor Author

Okay, I didn't know that.

@balloob
Copy link
Member

balloob commented Oct 28, 2016

The times in the graphs are configured in a different way however.

@CoalaJoe
Copy link

Is there any Polyfill?

@michaelarnauts
Copy link
Contributor Author

michaelarnauts commented Jan 5, 2017

There seems to be a different implementation between browsers.

  • Firefox takes the topmost language in the browser configuration (nl), and considers that to be the preferred language.
  • Chrome takes the language of the installed Chrome version (en), and considers that to be the preferred language, even though nl is at the top of my language configuration list.
  • I'm not sure what Edge and Internet Explorer 11 do.

Firefox:

> navigator.language
"nl"
> navigator.languages
Array [ "nl", "en", "en-US" ]

Chrome:

> navigator.language
"en-US"
> navigator.languages
["nl", "en", "en-US"]

A more general solution could be to use the following line instead of just navigator.language.

    language = navigator.languages ? navigator.languages[0] : navigator.language || navigator.userLanguage;

This picks the first selected language in your settings, and fallbacks to navigator.language or navigator.userLanguage (for old IE versions).

What do you think?

@michaelarnauts
Copy link
Contributor Author

According to this page, IE returns the language of the Operating System, what also don't accurately describe the time formatting requested.

Someway, it doesn't feel right to depend the time formatting on the language of the operating system or browser installed. I think a lot of installed browsers in non-english countries can be English, but they don't use AM/PM notation (what seems to be used mainly in US and UK).

@robbiet480
Copy link
Member

Closed via home-assistant/frontend#170!

@sillevl
Copy link

sillevl commented Feb 6, 2017

I would like to reopen this issue. home-assistant/frontend#170 Indeed solves part of the problem stated in the start topic. It fixes the date format in the logbook, but not in the state history chart timeline. The problem as shown in the screenshots is still present.

I opened an issue at the home-assistant-polymer project as wel: home-assistant/frontend#196

@fanaticDavid
Copy link
Contributor

I can confirm I'm still seeing this issue in the timeline pop-over (both in the States UI and the History UI). It seems to have been addressed everywhere else.

Seen in Firefox 51.0.1 set to Dutch (nl) on macOS 10.12.4 bèta.
Same behaviour in Chrome 55.0.2883.95 set to Dutch (nl), as well as in Safari 10.1 (12603.1.20.1) which seems to use the OS language setting (Belgian Dutch).

@emlove
Copy link
Contributor

emlove commented Feb 14, 2017

Agree this should be reopened. home-assistant/frontend#170 is a step in the right direction, but the Google charts time formatting still hasn't been addressed.

@emlove emlove reopened this Feb 14, 2017
@michaelarnauts
Copy link
Contributor Author

There is a new issue in the home-assistant-polymer repo: home-assistant/frontend#196
The backend is reporting the time like 2017-02-13T18:38:47.427063+00:00, so the issue lies in the frontend.

@home-assistant home-assistant locked and limited conversation to collaborators May 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants