? PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
28 Jun 2023

Summary of Changes

Similar text function for plugins as we do in #41079.

Testing Instructions

  • Open the back end
  • Go to the url /administrator/index.php?option=com_installer&view=install
  • Open the "Upload Package File" tab

Actual result BEFORE applying this Pull Request

All strings are translated.

Expected result AFTER applying this Pull Request

All strings are translated.

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

avatar joomla-cms-bot joomla-cms-bot - change - 28 Jun 2023
Category Libraries Front End Plugins
avatar laoneo laoneo - open - 28 Jun 2023
avatar laoneo laoneo - change - 28 Jun 2023
Status New Pending
avatar laoneo laoneo - change - 28 Jun 2023
Labels Added: PR-4.4-dev
avatar sandewt
sandewt - comment - 28 Jun 2023

After installing the patch, the following notice messages appear:

Notice: Only variables should be passed by reference in C:\xampp\htdocs\bugtesting5\joomla\plugins\content\contact\services\provider.php on line 38

Notice: Only variables should be passed by reference in C:\xampp\htdocs\bugtesting5\joomla\plugins\content\emailcloak\services\provider.php on line 37

And so on.

Doesn't seem to be a problem of this patch, but an underlying problem.
Assigning to $container->get(DispatcherInterface::class) a separate variable, solves this problem.

avatar laoneo
laoneo - comment - 28 Jun 2023

Does the same issue happen also on the 4.4-dev branch. For me it gives the impression that you somehow screwed up Joomla. But the message is definitely not related to the current pr.

avatar sandewt
sandewt - comment - 28 Jun 2023

is definitely not related to the current pr

I have also encountered this problem before.

[EDIT: PHPStorm reports 'Only variables can be passed by reference']

avatar sandewt sandewt - test_item - 28 Jun 2023 - Tested successfully
avatar sandewt
sandewt - comment - 28 Jun 2023

But the message is definitely not related to the current pr.

Oops, I tested in Joomla 5.


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

avatar sandewt
sandewt - comment - 28 Jun 2023

I have tested this item ✅ successfully on c1ec401


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

avatar viocassel viocassel - test_item - 28 Jun 2023 - Tested successfully
avatar viocassel
viocassel - comment - 28 Jun 2023

I have tested this item ✅ successfully on c1ec401


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

avatar Quy Quy - change - 28 Jun 2023
Status Pending Ready to Commit
avatar Quy
Quy - comment - 28 Jun 2023

RTC


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

avatar laoneo laoneo - change - 29 Jun 2023
Title
Add a text function for plugins
[5.0] Add a text function for plugins
avatar laoneo laoneo - edited - 29 Jun 2023
avatar laoneo laoneo - change - 29 Jun 2023
Title
[5.0] Add a text function for plugins
Add a text function for plugins
avatar laoneo laoneo - edited - 29 Jun 2023
avatar MacJoom MacJoom - change - 29 Jun 2023
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-06-29 12:17:59
Closed_By MacJoom
Labels Added: ?
avatar MacJoom MacJoom - close - 29 Jun 2023
avatar MacJoom MacJoom - merge - 29 Jun 2023
avatar HLeithner
HLeithner - comment - 29 Jun 2023

I'm not so happy with this, a plugin is normally not a thing which is used for translation, I also would move the tmpl from the plugin to layouts which is more the character then a "full" template at least in my understanding.

avatar laoneo
laoneo - comment - 29 Jun 2023

This here helps to ease the transition. At the end of the day the template files should work the same, regardless if they are used in views, modules, plugins and layouts. So this here is needed to provide a similar context in all of them. There are legit use cases where template files are used in plugins. So this is fine for the current architecture. If one some point we harmonize the whole code base then it will be even easier when all the template files use the same functions.

avatar HLeithner
HLeithner - comment - 29 Jun 2023

polluting the namespace/class for all plugins doesn't make sense anyway, better use a trait and the plugin that really needs this functionality should use it.

avatar brianteeman
brianteeman - comment - 29 Jun 2023

Why is it only this plugin that is updated and not all the others

avatar HLeithner
HLeithner - comment - 29 Jun 2023

Also this doesn't look like a proper "template engine" when you have to write the code your self:
https://github.com/Digital-Peak/joomla-cms/blob/c1ec40129dc59e9b21db1d64ab8ab7215fa66be6/plugins/installer/packageinstaller/src/Extension/PackageInstaller.php#L53-L55

and don't have at least a render() function

avatar sandewt
sandewt - comment - 30 Jun 2023

and don't have at least a render() function

Hey does such a render function look like?
Isn't this a proven technique in Joomla to render? See among others.

https://github.com/Digital-Peak/joomla-cms/blob/c1ec40129dc59e9b21db1d64ab8ab7215fa66be6/plugins/content/vote/src/Extension/Vote.php#L118-L121

avatar laoneo
laoneo - comment - 30 Jun 2023

Why is it only this plugin that is updated and not all the others

I will do this in 5.0 once it is upmerged, similar to #41079 and #41063.

Also this doesn't look like a proper "template engine" when you have to write the code your self

As I said, this pr is made for the current architecture. If you want to change it go ahead and make a pr. This here is fine for what we have now. Not saying it is the gold solution.

avatar laoneo
laoneo - comment - 30 Jun 2023

@brianteeman here we go #41092.

avatar brianteeman
brianteeman - comment - 30 Jun 2023

thanks - I guess I am/was confused why you change one here for 4.4 and all the others for 5.0

Add a Comment

Login with GitHub to post a comment