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

Contao 5 offers a whole lot of languages #5973

Closed
leofeyer opened this issue Apr 20, 2023 · 15 comments · Fixed by #6110
Closed

Contao 5 offers a whole lot of languages #5973

leofeyer opened this issue Apr 20, 2023 · 15 comments · Fixed by #6110
Assignees
Labels

Comments

@leofeyer
Copy link
Member

leofeyer commented Apr 20, 2023

Affected version(s)

5.1.3

Description

I understand that there are multiple language and region combinations, but do we really need "Kölsch", "Niedersorbisch" and "Obersorbisch" in the list? Also, "Englisch (Deutschland)" seems like a rare combination.

@leofeyer leofeyer added bug up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. labels Apr 20, 2023
@leofeyer leofeyer added this to the 5.1 milestone Apr 20, 2023
@bytehead
Copy link
Member

I like the Walliserdeutsch 😬

@aschempp
Copy link
Member

Schweizerdeutsch (Frankreich) 🤦

@aschempp
Copy link
Member

also, this is a list of locales, not languages, right? Where did you get that list?

@fritzmg
Copy link
Contributor

fritzmg commented Apr 20, 2023

Where did you get that list?

e.g. when selecting the language for a member or user.

It's the same in Contao 4.13.

@ausi
Copy link
Member

ausi commented Apr 20, 2023

also, this is a list of locales, not languages, right?

We probably should have changed this from locales to languages in Contao 5.0 as tl_member already has a country field. But this would be a BC break I think, or we could change it in Contao 5.2? (And it would still include „Kölsch“ (but not „Kölsch Deutschland“))

but do we really need "Kölsch", "Niedersorbisch" and "Obersorbisch" in the list?

But does it hurt?
There is some data in Intl that would allow us to filter the list (like information about how many people speak a language and if it is an official language in any country) but I think this is so project specific that it is hard to come up with a good default.

@fritzmg
Copy link
Contributor

fritzmg commented Apr 20, 2023

We probably should have changed this from locales to languages in Contao 5.0 as tl_member already has a country field. But this would be a BC break I think, or we could change it in Contao 5.2? (And it would still include „Kölsch“ (but not „Kölsch Deutschland“))

Well we introduced the locales in Contao 4.12 (#3138). Before that it was a list of languages managed by Contao itself, rather than via Intl.

@ausi
Copy link
Member

ausi commented Apr 20, 2023

Before that it was a list of languages managed by Contao itself, rather than via Intl.

Yes, but it also included the regions in Contao 4.11 (de, de_DE, de_AT, …), that’s why we kept it that way AFAIK.

@fritzmg
Copy link
Contributor

fritzmg commented Apr 20, 2023

In any case - it depends on the specific use case of the application what kind of languages should be able to be selected there. For example, if you use this information in order to translate notifications that you will be sending to your members or users, then they need to be locales, in theory.

However, you are unlikely to provide translations for all languages anyway. So for example for the registration front end module you will have to use a custom options_callback that returns exactly what you want to offer in your application.

The back end however can always display all possibilities imho. And otherwise you'll also have to use your own options_callback to reduce the options when needed.

@ausi
Copy link
Member

ausi commented Apr 20, 2023

And otherwise you'll also have to use your own options_callback to reduce the options when needed.

Or configure the list of locales you want to support in your whole application via contao.intl.locales.

@ausi
Copy link
Member

ausi commented Apr 21, 2023

How about only showing locales with primary territories?

private function getDefaultLocales(): array
{
    $locales = [];

    foreach (\ResourceBundle::create('supplementalData', 'ICUDATA', false)['languageData'] ?? [] as $language => $data) {
        if (!$regions = ($data['primary']['territories'] ?? null)) {
            continue;
        }

        $locales[] = $language;

        foreach (is_string($regions) ? [$regions] : $regions as $region) {
            $locales[] = "{$language}_$region";
        }
    }

    return array_values(array_intersect(\ResourceBundle::getLocales(''), $locales));
}

Bildschirmfoto 2023-04-21 um 16 20 46

@leofeyer
Copy link
Member Author

I would like that at lot. 👍

@ausi ausi self-assigned this Apr 24, 2023
@ausi
Copy link
Member

ausi commented Apr 24, 2023

I tried to implement this today, but I’m not sure if this is the right way to go here. We would exclude spanish for the united states now (because it is not an “official” language) even though there are more people in the USA than in spain who speak spanish.

If we still want to go that route, we might also remove the intersection with \ResourceBundle::getLocales(''), even though the list gets then longer again 😬

--- a/core-bundle/src/DependencyInjection/Compiler/IntlInstalledLocalesAndCountriesPass.php
+++ b/core-bundle/src/DependencyInjection/Compiler/IntlInstalledLocalesAndCountriesPass.php
@@ -28,7 +28,7 @@ class IntlInstalledLocalesAndCountriesPass implements CompilerPassInterface
             $definition = $container->findDefinition('contao.intl.locales');

             $enabledLocales = $this->getEnabledLocales($container);
-            $locales = array_values(array_unique(array_merge($enabledLocales, \ResourceBundle::getLocales(''))));
+            $locales = array_values(array_unique(array_merge($enabledLocales, $this->getDefaultLocales())));

             $definition->setArgument(2, $locales);
             $definition->setArgument(3, $enabledLocales);
@@ -71,4 +71,36 @@ class IntlInstalledLocalesAndCountriesPass implements CompilerPassInterface

         return array_values(array_unique($languages));
     }
+
+    private function getDefaultLocales(): array
+    {
+        $allLocales = [];
+
+        foreach (\ResourceBundle::create('supplementalData', 'ICUDATA', false)['languageData'] ?? [] as $language => $data) {
+            if (!$regions = ($data['primary']['territories'] ?? null)) {
+                continue;
+            }
+
+            $scripts = $data['primary']['scripts'] ?? [];
+            $locales = [$language];
+
+            foreach (is_string($scripts) ? [$scripts] : $scripts as $script) {
+                $locales[] = "{$language}_$script";
+            }
+
+            foreach ($locales as $locale) {
+                $allLocales[] = $locale;
+
+                foreach (is_string($regions) ? [$regions] : $regions as $region) {
+                    $allLocales[] = "{$locale}_$region";
+                }
+            }
+        }
+
+        if (!$allLocales) {
+            return \ResourceBundle::getLocales('');
+        }
+
+        return array_values(array_intersect(\ResourceBundle::getLocales(''), $allLocales));
+    }
 }

As an alternative, how about changing the tl_member.language callback to use the same as tl_user.language?

@aschempp
Copy link
Member

aschempp commented May 3, 2023

As an alternative, how about changing the tl_member.language callback to use the same as tl_user.language?

Wouldn't that limit the member languages to the installed languages of Contao? I don't think they are related, it would make more sense to limit them to the available root pages then. But again, that is also very application specific. On members.contao.org I manually limited the member languages to what we support for notifications etc., but Contao can't guess that for you.

@leofeyer
Copy link
Member Author

leofeyer commented Jun 1, 2023

As discussed in the Contao call, we want to keep tl_member.language as locale as suggested by @ausi above but without the array_intersect(\ResourceBundle….

@leofeyer leofeyer removed the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Jun 1, 2023
@ausi
Copy link
Member

ausi commented Jun 1, 2023

See #6110

@ausi ausi closed this as completed Jun 1, 2023
leofeyer pushed a commit that referenced this issue Jun 1, 2023
Description
-----------

Fixes #5973

Commits
-------

d0ef863 Use primary languages as the default locale list
3369fa9 Also add regional official languages
e9954fa Skip languages with missing names
32b25f6 Coding style
7f3669d Remove unused variable
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants