Fix: Using translated language name on language page titles#12041
Fix: Using translated language name on language page titles#12041cdrini merged 8 commits intointernetarchive:masterfrom
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
tfmorris
left a comment
There was a problem hiding this comment.
Thanks for tackling this! You should be able to update your dev instance's language data for testing purposes by going to e.g. languages/fre.yml?m=edit and adding
name_translated:
fr:
- français
and similarly for languages/ger.yml?m=edit
I didn't test it, but, by inspection, there are a few other places on the page where the translated version of the language are needed instead of page.name.
Also, the adding of the Deprecated tag can't use string concatenation to allow for internationalization.
…ed i18n string, and update pot file
|
@tfmorris Thanks for the feedback! I've addressed all the suggestions:
In relation to the test assertion, I kept 'French' for the |
tfmorris
left a comment
There was a problem hiding this comment.
In relation to the test assertion, I kept 'French' for the fre case intentionally since that test is specifically covering the 'no name_translated field at all' fallback path.
Understood. That sounds fine. The production label is "French / français" but it looks like that has drifted from what's in languages.page (which I just discovered the existence of). If that gets updated, it'll also solve your dev data issue with the missing translations.
It looks good from my point of view, but you'll still need to get someone with commit privileges to review and merge.
for more information, see https://pre-commit.ci
cdrini
left a comment
There was a problem hiding this comment.
Tested here and works: https://testing.openlibrary.org/languages/fre?lang=de
That returns 401 Authorization Required. Ditto for the home page. |
|
The testing server is now behind an access key in response to excessive crawling of it. Just emailed you the key. |
tfmorris
left a comment
There was a problem hiding this comment.
I missed that the last mention on the page had been overlooked when I did my previous review. Sorry!
|
|
||
| <div> | ||
| <span class="title">$_("Name")</span> | ||
| <span class="tag">$page.name</span> |
There was a problem hiding this comment.
| <span class="tag">$name</span> |
This needs to be either $name or $page_title
There was a problem hiding this comment.
Oops, looks like the suggestion markup is broken. That replaces the $page.name, if it's not obvious.
Closes #12005
Technical
The language page template (
openlibrary/templates/type/language/view.html) was usingname = page.namewhich always fetches the English name, ignoring thename_translatedfield.The fix replaces this with
name = get_language_name(page, get_lang() or 'en'), using the existingget_language_nameutility inutils.pywhich looks up the correct translation fromname_translatedbased on the user's current UI language, with a fallback to English if no translation is available.Testing
get_language_namehas been added toopenlibrary/plugins/upstream/tests/test_utils.pycovering three cases:name_translatedfield at all (falls back toname)name)/languages/freand the title and name should appear in the selected language.Screenshot
N/A - Unable to verify visually locally as dev data set does not seem to have
name_translatedpopulated on language records.Stakeholders
@cdrini