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

Localize times #13489

Merged
merged 3 commits into from
Feb 5, 2020
Merged

Conversation

marusak
Copy link
Member

@marusak marusak commented Feb 4, 2020

Fixes #8583
I haven't found any other place, where the format would not be localized. If you find something, please point it out, and I'll fix it out.
The only other place related to time, that should be localized, are calendars:
calendarbefore
(names of days, on which day week starts... - this can be done with setting up language but it requires to import all locale files - which cannot be automated... But there are also other options, so I just plan to open issue for that and take a look at it separately).

I am not sure about that change on the overview page. It seems so obvious that I doubt it was not purposely that way? Now looks like this in English and is localized:
timeformat
Is that right?

Likely breaks tests, so spinning them out

@martinpitt
Copy link
Member

I am not sure about that change on the overview page

In master I see "2020-02-04 16:19" right now, which is ISO format; both in German and in English. I suppose that works in every language, but indeed it seems fine to actually localize it.

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Looks mostly good, thanks! Please remember to add a Closes # still, when the tests come back.

if (pos !== -1)
string = string.substring(0, pos);
return string.replace('T', ' ');
moment.locale(cockpit.language); // HACK: when done at the top of the file, c.language is undefined
Copy link
Member

Choose a reason for hiding this comment

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

I think it should rather live in pkg/systemd/overview.jsx . @allisonkarlitskaya needs that for PR #13473 as well.

martinpitt
martinpitt previously approved these changes Feb 5, 2020
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks! I suppose this will fail on -distropkg, though.

test/verify/check-system-info Outdated Show resolved Hide resolved
@martinpitt martinpitt added the release-blocker Targetted for next release label Feb 5, 2020
@martinpitt martinpitt merged commit 1ccd915 into cockpit-project:master Feb 5, 2020
@vieira-temes
Copy link

vieira-temes commented Sep 30, 2021

Sorry if this is not the appropriate place to ask, but is it possible to have the language set to en_us but with a 24 hour format? How?

@martinpitt
Copy link
Member

@PabloGrok : You'd need to pick a locale with 24 hour format, perhaps en_GB?

@vieira-temes
Copy link

But I don't want British idioms in the UI. Shouldn't time formats be decoupled from locales?

@martinpitt
Copy link
Member

Unfortunately browsers don't have the fine-grained locale configuration like Linux has (LC_TIME, LC_MONETARY, LC_MESSAGES and such).

@garrett
Copy link
Member

garrett commented Nov 8, 2021

Additionally, the first day of the week is different in various places.

first day of the week
https://en.wikipedia.org/wiki/Week#/media/File:First_Day_of_Week_World_Map.svg

And either we're assuming English means US and have German incorrect, or we (or PF) hardcode the first day of the week as Sunday:

Screenshot 2021-11-08 at 11-47-02 Overview - garrett Bolt

Screenshot 2021-11-08 at 11-47-46 Überblick - garrett Bolt

PatternFly has a weekStart option. I guess we're not handling that with locales and/or not passing it to the PF widget:

Name Type Default Description
weekStart 0 | 1 | 2 | 3 | 4 | 5 | 6 | Weekday   Day of week that starts the week. 0 is Sunday, 6 is Saturday.

@martinpitt
Copy link
Member

@garrett : Indeed -- eww, that sucks. Standard Unix/Linux locale information has had that piece of information forever:

❱❱❱ LC_TIME=en_US.UTF-8 locale -k -c LC_TIME | grep first_weekday
first_weekday=1

❱❱❱ LC_TIME=de_DE.UTF-8 locale -k -c LC_TIME | grep first_weekday
first_weekday=2

However, I didn't find anything in Intl which would expose that piece of information to JS. There is a JS standard issue about this very topic: tc39/ecma402#6 but it has been open for 6 years now (but there is some progress).

So I'm afraid right now the only thing which we can do is to keep a copy of that information in our pkg/lib. I'll look into it.

@martinpitt
Copy link
Member

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Locale/weekInfo would be that, but it doesn't work in Firefox, Chromium, nor node.js.

@martinpitt
Copy link
Member

I sent PR 16587 to at least fix this for most users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Targetted for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

time & date formats are different across Cockpit
4 participants