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

Locale filter selector based on installed languages #716

Merged

Conversation

jhonatan-lopes
Copy link
Contributor

@jhonatan-lopes jhonatan-lopes commented Sep 23, 2023

The problem

On the "Translations" report, the current way to filter by locales is to type the language code in the field exactly as it was defined by the developers. This causes some issues.

First, we could have some editors/admins that might just want a quick reference into translations and might not experts in the field. Hence, it is not fair to expect them to know the language codes for all installed languages. For instance, although ubiquitous, one might not know that "de" is the language code for the German language.

The second problem is more subtle and more difficult to debug when it happens. If a project defines their WAGTAIL_CONTENT_LANGUAGES as

LANGUAGE_CODE = "en"
WAGTAIL_CONTENT_LANGUAGES = LANGUAGES = (
    ("en", gettext_lazy("English")),
    ("de", gettext_lazy("German")),
    ("es", gettext_lazy("Spanish")),
    ("fr", gettext_lazy("French")),
    ("fy-NL", gettext_lazy("Frisian")),
    ("nl", gettext_lazy("Dutch")),
    ("pl", gettext_lazy("Polish")),
    ("pt-BR", gettext_lazy("Portuguese (Brazil)")),
    ("sw", gettext_lazy("Swahili")),
)

for example, the field as it current stands will only recognise "pt-BR" as the "Portuguese (Brazil)" language, but not "pt-br".

From the ITEF wiki:

Subtags are not case-sensitive, but the specification recommends using the same case as in the Language Subtag Registry, where region subtags are UPPERCASE, script subtags are Title Case, and all other subtags are lowercase. This capitalization follows the recommendations of the underlying ISO standards.

So that the "correct-er" form would be "pt-BR", although if subtags are supposed to not be case-senstive then both cases must filter properly (which is not the currently behaviour).

The solution

This PR leverages the language codes defined in WAGTAIL_CONTENT_LANGUAGES to provide chooser fields for the locales instead of relying on having to type the proper language code. This abstracts the language code information from editors/admins with the bonus of displaying language names localized to their Wagtail admin choice.

An "All" option is defined to display translations in all locales.

Screenshots:

Using the provided test app with:

LANGUAGES = WAGTAIL_CONTENT_LANGUAGES = [
    ("en", "English"),
    ("fr", "French"),
    ("fr-CA", "French (Canada)"),
    ("es", "Spanish"),
]

image

image

image

Copy link
Collaborator

@zerolab zerolab left a comment

Choose a reason for hiding this comment

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

This is a welcome improvement @jhonatan-lopes

Left a few notes

wagtail_localize/views/report.py Outdated Show resolved Hide resolved
wagtail_localize/views/report.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e2de395) 93.27% compared to head (dcf744e) 93.27%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #716   +/-   ##
=======================================
  Coverage   93.27%   93.27%           
=======================================
  Files          47       47           
  Lines        3908     3911    +3     
  Branches      579      579           
=======================================
+ Hits         3645     3648    +3     
  Misses        154      154           
  Partials      109      109           
Files Changed Coverage Δ
wagtail_localize/views/report.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zerolab zerolab merged commit 7e75cbd into wagtail:main Oct 1, 2023
10 of 13 checks passed
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