Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Form] Add time zone translations#26851
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
79abebc tob863e06Comparemvrhov commentedApr 9, 2018
IMO punic should not be required. We have intl and the translations should be taken from there and this should be the "hard" requirement for translating. Otherwise we are just making a mess out of it. |
c960657 commentedApr 9, 2018
I previously suggested adding timezone names to the Intl component in#17636. One comment I received was this:
To be fair, the patch had other problems due to my then limited understanding of CLDR. CLDR is an excellent source of translation data, and I think it does not get the attention it deserves from the PHP community. I think Symfony should embrace a library that exposes CLDR data, either included by expanding the Intl component or by requiring an external library. However, the Intl component is currently very limited in this respect, and bringing it on par with e.g. Punic would require a substantial amount of work. But if there is consensus that this data should live in the Intl component, I will reopen#17636. |
mvrhov commentedApr 9, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
with intl I have meant the extension itself. This extension provides the translations from CLDR. |
c960657 commentedApr 10, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
True, the intl PHP extension contains the localised city names. AFAICT it does not contain names of countries and continents, though, but these few strings can be provided manually in translation files.
I'm not sure what you mean. The localised city names are not included in Symfony? |
mvrhov commentedApr 10, 2018
I'm talking about this:https://github.com/symfony/intl/ |
| /** | ||
| * Returns a normalized array of timezone choices. | ||
| * | ||
| * This function is not part of the public API but should only be called |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It should be marked@internal then.
| <tooltool-id="update-timezone-translations.php"tool-name="CLDR time zone translation generator"/> | ||
| </header> | ||
| <body> | ||
| <trans-unitid="xKNxrNf"resname="Africa"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
These ids are autogenerated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, based on a hash of the source string. Seehttps://github.com/symfony/symfony/blob/master/src/Symfony/Component/Translation/Dumper/XliffFileDumper.php#L148
c960657 commentedApr 10, 2018
@mvrhov Just to clarify, Symfony has anIntl component, and PHP has anIntl extension. Both are based on ICU that is based on CLDR. The localised city names are included in the PHP extension but not in the Intl component. The Intl component includes country names but not continent names. The PHP extension includes neither country names nor continent names. |
b863e06 to1b75db4Comparemvrhov commentedApr 11, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sorry for the confusion then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
for the implementation, i think we should follow same the approach as other intl types. Thus aTimeZoneBundle
while at it could solve#22302 too
| @@ -33,7 +33,8 @@ | |||
| "symfony/http-kernel": "~3.4|~4.0", | |||
| "symfony/security-csrf": "~3.4|~4.0", | |||
| "symfony/translation": "~3.4|~4.0", | |||
| "symfony/var-dumper": "~3.4|~4.0" | |||
| "symfony/var-dumper": "~3.4|~4.0", | |||
| "punic/punic": "~3.1" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
not sure we need this? see e.g.https://3v4l.org/HOv3k
ro0NL commentedOct 12, 2018
See#28831 for an alternative |
ro0NL commentedApr 16, 2019
To replace and legitimately close this issue i've updated#28836 |
fabpot commentedApr 21, 2019
closing in favor of#28831 |
This PR was squashed before being merged into the 4.3-dev branch (closes#28831).Discussion----------[Intl] Add Timezones| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | replaces#26851,#17628,#17636| License | MIT| Doc PR |symfony/symfony-docs#11447This PR compiles a new `TimezoneBundle` from ICU data, based on the following resources:-https://github.com/unicode-org/icu/tree/master/icu4c/source/data/zone-https://github.com/unicode-org/icu/blob/master/icu4c/source/data/misc/timezoneTypes.txt-https://github.com/unicode-org/icu/blob/master/icu4c/source/data/misc/metaZones.txtThe goal is to provide a fixed set of timezones, with a localized human readable name.For inspiration and to double check some data i used the timezone widget from google, e.g. when using Google Calendar.I've only pushed some common compiled locales for review, the rest will follow once finished.cc@jakzalCommits-------4bea198 [Intl] Add Timezones
This is a second attempt to localise
Symfony\Component\Form\Extension\Core\Type\TimezoneType. First attempt was#17628 that was rejected, because it changed the existing UI a bit, and because I was not fully aware of how to use the CLDR data.I really think localisation is important. Not everyone speaks English, and if you visit e.g. a Russian-language site, you would expect the timezone picker to show city names written in Cyrillic letters. Let's find a way to fix this, even if it requires adjusting the existing UI slightly.
In this attempt I have tried to stay closer to the existing UI than in#17628. One intentional change, though, is that “Argentina - San Juan” is now displayed as “San Juan, Argentina”. CLDR uses this formatting for US states, e.g. “Tell City, Indiana”, and I think it makes sense, so that list items can be sorted alphabetically by the city name.
One improvent over the existing solution is that
Antarctica/DumontDUrvilleis now displayed as “Dumont d’Urville” rather than “DumontDUrville”.Matching the current UI is a bit tricky, because CLDR does not contain translations for the oceans (Pacific/Atlantic/Indian), so they must be provided manually. But this is necessary, if we want to preserve the existing UI. One way to get around this (and simplify the code a bit) is to group timezones by continent as shown inthis list, rather than by the tzdata area. This will give nearly identical results, except that timezones in
Atlantic/xxx,Pacific/xxxandIndian/xxxwill be shown under the continent they belong to, and a few other exceptions (e.g. Turkey now belongs to Asia, while tzdata puts it in Europe, because Istanbil is there – seehttps://unicode.org/pipermail/cldr-users/2018-April/000795.html). These are all minor changes IMO.I have chosen to base the implementation on the excellent Punic library rather than downloading the XML files directly, like I did in#17628, or trying to add them to the Intl component, like I did in#17636. The Punic library supports large parts of CLDR (the data and the specification on how to use it), so I think it is a good choice. It is not a runtime dependency, but is only used for generating the translation files, so I hope adding this dependency is not too controversial.
Disclaimer: I was recentlyinvited to become a co-author of Punic, because I had contributed a bit to the library. However, I do not have any interests in this library, except as an interested user and contributor.
One thing still missing from this PR is that timezones are not sorted according to their localised name. Stack Overflow tells me that in order to do that, one should translate them and sort them prior to adding them to the form. This is e.g. when
CountryTypedoes. However, I'm not sure about the best way to do this (i.e. how to inject the translator). Another option is to use Punic as a runtime dependency to get the localised strings without using the translator, but I don't know if Symfony wants this kind of dependencies. It could be made as an optional dependency with fallback to the English names like to today. So I kindly ask for advice on how to handle this.