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 DatePicker's start of week for most cases #16587

Merged
merged 1 commit into from
Nov 10, 2021

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Nov 10, 2021

PF's DatePicker defaults to Sunday as the start of the week instead of
the given locale's convention. There is currently no official JS API
which exposes that piece of locale information [1], and
cockpit.language does not even contain the country in most cases. So
add a timefirmat.firstDayOfWeek() approximation by language, which at
least gets it right for most of our supported languages except
English-speaking countries in Europe (in particular, en-GB and en-IE).

Add missing aria and localization to the timer creation and user account
expiration dialogs, so that their month names are properly translated.

[1] tc39/ecma402#6


English looks as before:

date-picker-en

In German, the week now starts on Monday:

date-picker-de

PF's DatePicker defaults to Sunday as the start of the week instead of
the given locale's convention. There is currently no official JS API
which exposes that piece of locale information [1], and
`cockpit.language` does not even contain the country in most cases. So
add a `timefirmat.firstDayOfWeek()` approximation by language, which at
least gets it right for most of our supported languages *except*
English-speaking countries in Europe (in particular, en-GB and en-IE).

Add missing aria and localization to the timer creation and user account
expiration dialogs, so that their month names are properly translated.

[1] tc39/ecma402#6
@martinpitt martinpitt temporarily deployed to cockpit-dist November 10, 2021 08:31 Inactive
@garrett
Copy link
Member

garrett commented Nov 10, 2021

Is there a way to get the local computer's timezone? That would fix the approximation, right?

(The timezone in the screenshot is the remote system's timezone, of course.)

Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

I feel that weekStart should not be exposed property but rather set by PF according to the locale provided. However since it's missing from intl we can have this solution.

@garrett
Copy link
Member

garrett commented Nov 10, 2021

For most of English and much of Spanish: if it's in the Americas and Austrailia, then it's Sundays first, if in Europe, it's Monday.

(English in South Africa is Sunday first.)

@garrett
Copy link
Member

garrett commented Nov 10, 2021

FWIW, I'm consulting the map @
image
https://en.wikipedia.org/wiki/Week#/media/File:First_Day_of_Week_World_Map.svg

It's not perfect, but we could check offset for less than / greater than and make an exception for South Africa (SAST/EAT) if we know the timezone (instead of just the offset).

@garrett
Copy link
Member

garrett commented Nov 10, 2021

Alternatively:

We could export the server's locale information.

And if browser time is the same as the system time (or peraps +- 3 timezones off... think servers in Pacific time and living in Eastern time in the US, or servers in France but living in Finland), use that to instruct the weekday start.

Any solution is a bit hacky though, if we don't have access to this information from the browser itself. (Unless we add this as a preference in the language selection dialog too. But we'd still want something that generally solves it without a setting, naturally.)

Copy link
Member

@garrett garrett left a comment

Choose a reason for hiding this comment

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

If we decide to have a little more logic to get it closer to correct and/or add a preference in the language dialog, it would build upon this. So I'm happy that this PR makes it more correct for more people. It's definitely better than what we currently have.

Also, the first day of the week is not just Sunday or Monday, so this is incomplete. (But I guess for the languages we currently officially support, it's probably correct enough, except for European English and American Spanish.)

@martinpitt martinpitt merged commit 49310bc into cockpit-project:main Nov 10, 2021
@martinpitt martinpitt deleted the datepicker-l10n branch November 10, 2021 10:59
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.

3 participants