-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add helper function to get the supported locales excluding mappings #101
base: master
Are you sure you want to change the base?
Conversation
87e3452
to
40af670
Compare
not sure about this. Why do you need this? |
I use this to show a dropdown of locales to select. I will store it in the database and then later use it in jobs to determine which language to send an e-mail or generate a PDF for that particular user. It makes no sense to show mapped locales in this dropdown, they would be duplicates and using them would break the application as they are not supported. The mapped locales are only used for Accept-Language headers they aren't visible anywhere else. This was always the intention. See discussion #42 IMHO mapped locale's should not have to be included in Detector::supported in order to work. I misinterpreted the original PR believing that it would work that way (#87) TLDR; Instead of this feature I should fix it so that you can have mapped-from locales which are not present in the "supported" array. Agreed @svycka? The mapping feature itself is still necessary I see no way around that. The added complexity is unfortunate but seems necessary. |
for dropdown we have https://github.com/basz/SlmLocale/blob/master/src/SlmLocale/View/Helper/LocaleMenu.php this view helper with use supported locales can't you use this? Or does this return wrong locales? Maybe then we should fix this helper? |
*/ | ||
protected $supported; | ||
|
||
/** | ||
* Optional list of locale mappings | ||
* | ||
* @var array | ||
* @var array & string[string] |
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.
should be array
and maybe add an example or/and description
The view helper returns the wrong locales also. My config is currently like so (shortened example): return [
'slm_locale' => [
'supported' => [
'de_DE',
// will be mapped:
'de_AT',
],
'mappings' => [
'de_AT' => 'de_DE',
]
],
]; I want it to be like this but this doesn't work: return [
'slm_locale' => [
'supported' => [
'de_DE',
],
'mappings' => [
'de_AT' => 'de_DE',
]
],
]; If I fix that then I don't need this PR. Understand? |
okay now I understand your pain :) then fixing this would be better or maybe add an option to localeMenu helper to include or skip mapped locales? @basz what do you think? |
I've been using the mappings feature and found I needed this helper function often enough that it might be worthwhile to add.
Maybe it's also nice to include this option in LocaleMenu