symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony

https://ux.symfony.com/

MIT License

856 stars 315 forks source link

[Translator] Handle W3C locale format on document element #2390

Closed aleho closed 2 days ago

aleho commented 3 days ago

Q	A
Bug fix?	yes
New feature?	no
Issues	Fix #2378
License	MIT

According to W3C (and RFC1766) the valid format for language codes is "primary-code"-"subcode", but Symfony expects underscores as separator, resulting in broken translations.

This changes language codes specified in the correct format for HTML to the Symfony format when reading the "lang" attribute.

github-actions[bot] commented 3 days ago

📊 Packages dist files size difference

Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR. Please review the changes and make sure they are expected.

File	Before (Size / Gzip)	After (Size / Gzip)
Translator
`translator_controller.js`	`9.17 kB` / `2.27 kB`	`9.23 kB`^{+1% 📈} / `2.29 kB`^{+1% 📈}

aleho commented 2 days ago

This is a direct BC break for people using <html lang="{{ app.request.locale }}"> with a _ inside the locale

Is it? Did I miss something? In my tests it didn't matter whether I used de, de_DE or de-DE, all work with this PR. (See new tests.)

What doesn't work is using en(-|_)US if there's no en_US configured in Symfony. Neither does de(-|_)AT if only de and de_DE are configured. (But that's unrelated to my PR, it wouldn't fallback to the available primary code before either.)

However, it miss the CHANGELOG update

If this isn't a BC break, does it still need a changelog entry? Or can this be a bug fix?

Now with tests. Sorry about that, completely forgot to commit them.

smnandre commented 2 days ago

If this isn't a BC break, does it still need a changelog entry? Or can this be a bug fix?

Almost a feature here :)

# CHANGELOG

+ ## 2.22.0
+ 
+ -  Support both "fr-FR" and "fr_FR" format in html lang attribute

## 2.20.0

Something like this maybe ? (wording to illustrate only 😅 )

aleho commented 2 days ago

@smnandre @Kocal Thanks!

Kocal commented 2 days ago

Thank you @aleho.