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

[5.3] Language Association Codes #44551

Open
wants to merge 5 commits into
base: 5.3-dev
Choose a base branch
from

Conversation

brianteeman
Copy link
Contributor

In the Language Code plugin it is possible to change the code used for a specified language. eg fr-FR can be changed to fr-CA

However everywhhere in the admin that associations are displayed it uses the original code and not the changed code.

This PR is a "takeover" of the original PR #37289 created by @infograf768

That PR has merge conflicts and @infograf768 is now retired.

See the original PR for more screenshots

Testing Instructions

Create a multilingual site.
Enable the languagecode system plugin and give a new lang tag value for a language.

image

Actual result BEFORE applying this Pull Request

image

Expected result AFTER applying this Pull Request

image

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

In the Language Code plugin it is possible to change the code used for a specified language. eg fr-FR can be changed to fr-CA

However everywhhere in the admin that associations are displayed it uses the original code and not the changed code.

This PR is a "takeover" of the original PR joomla#37289 created by @infograf768

That PR has merge conflicts and @infograf768 is now retired.

See the original PR for more screenshots but an example is below

Signed-off-by: BrianTeeman <[email protected]>
@fgsw
Copy link

fgsw commented Nov 28, 2024

I have tested this item ✅ successfully on 8ccef0c


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44551.

Signed-off-by: BrianTeeman <[email protected]>
@Fedik
Copy link
Member

Fedik commented Nov 29, 2024

The feature maybe is good and usefull.
However the implementation is questionable, because hardcoding if (PluginHelper::isEnabled('system', 'languagecode')) everywhere promisses a loot of fun bugs.

Need to look for a better way of doing the same, maybe some extra event, or kind of that.

@universewrld
Copy link

@brianteeman will it work if instead of en-GB i just use en?
in your screenshots it is shown only for language+country codes.

@brianteeman
Copy link
Contributor Author

@brianteeman will it work if instead of en-GB i just use en? in your screenshots it is shown only for language+country codes.

yes but then then language code wont work as intended.

@MacJoom MacJoom added the PBF Pizza, Bugs and Fun label Feb 18, 2025
@fgsw
Copy link

fgsw commented Feb 20, 2025

Prebuilt packages are not available for a retest.

@richard67
Copy link
Member

Prebuilt packages are not available for a retest.

@fgsw Yes, they are deleted after a while. I've updated the branch so a new build was triggered, and now packages are available.

@fgsw
Copy link

fgsw commented Feb 20, 2025

I have tested this item ✅ successfully on 6cc6c8c

Before After
1 2

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44551.

@crimle
Copy link

crimle commented Feb 22, 2025

I have tested this item ✅ successfully on 6cc6c8c


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44551.

@alikon
Copy link
Contributor

alikon commented Feb 22, 2025

PBF


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44551.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 22, 2025
@Fedik
Copy link
Member

Fedik commented Feb 23, 2025

I am sorry, but this cannot be merged, because of hardcoding if (PluginHelper::isEnabled('system', 'languagecode')).
We need to find a better logic.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44551.

@Fedik Fedik removed the RTC This Pull Request is Ready To Commit label Feb 23, 2025
@brianteeman
Copy link
Contributor Author

I dont understand your reasoning. PluginHelper::isEnabled is used over 50 times in the core

@Fedik
Copy link
Member

Fedik commented Feb 23, 2025

It not an issue with PluginHelper::isEnabled, but with hardcoded plugin itself.

In the core, the plugins should never be used like:

 $languageCodeParams = new Registry(PluginHelper::getPlugin('system', 'languagecode')->params);

What if I want to use 3rd language plugin? With current approach the whole thing will not going to work.

The plugins should listen to events, instead of being used directly.
For current issue, it could be a new event that hook up in to LanguageHelper::getContentLanguages and extend it, or kind of that.

@brianteeman
Copy link
Contributor Author

As i said this is already being used in core

if (PluginHelper::isEnabled('system', 'redirect')) {
$params = new Registry(PluginHelper::getPlugin('system', 'redirect')->params);
$collect_urls = (bool) $params->get('collect_urls', 1);

Signed-off-by: BrianTeeman <[email protected]>
@Fedik
Copy link
Member

Fedik commented Feb 23, 2025

That another bad code, and not an excuse to do the same.

@brianteeman
Copy link
Contributor Author

When infograf wrote the code I assume he just followed existing practice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature PBF Pizza, Bugs and Fun PR-5.3-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants