Conversation
|
PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here. PR Reviewer Guide 🔍
|
|
PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here. PR Code Suggestions ✨
|
|
About the sortable datatables, pester (which you linked to) uses there own table implementation https://github.com/pester/docs/tree/main/src/components/PesterDataTable |
|
Looks good. About index page. Content
Another group
And unlinked. For detail page.
But I think what you made is good.
I would keep only the event name in the header, and move the class in to description. onContentPrepareEvent class Mix of the event class and the event name in the headers is a bit hard to read. |
|
@HLeithner @Fedik many thanks for your comments - very useful. I've revised the documentation
Details page is at |
I think we better move this sections in to The "Accessing Event Parameters / Arguments" looks good. The "Returning Values" is very confusing 😉 The legacy event which is expecting the result: function onFooBar()
{
// ...
return $myValue;
}The event class which is expecting the result: function onFooBar(FooBarEvent $event): void
{
// ...
$event->addResult($myValue);
}Parameters which was with "reference" Also there is events which had parameters as reference, in j5 this events got a special method to update the parameter (because referencing does not work anymore). I have tries do give a migration example at: but this also better to be part of
Yes, that is good idea. Maybe for arguments that changed their names can do kind of this:
|
Yes, agreed! I'm going to do this. Otherwise it's going to end up unmaintainable.
The problem is that this works only if it's a concrete class. If it's a generic class it raises an exception. So if someone is trying to return a value to onContentAfterTitle in Joomla 4.4 then they can't use this, and simply returning a value won't work, so they have to use the generic event approach. I think I know how to make this clearer though.
Ok, I wasn't aware of this. As the update functions are going to have different names I think it's best to document this in the detailed section, eg in the onContentPrepareData documentation. Will this just be done for some parameters and not for others? For example, for onContentPrepare where you can update the $subject/item? |
Yes, you are right. function onFooBar(Joomla\Event\Event $event): void
{
// ...
if ($event instanceof ResultAwareInterface) {
$event->addResult($value);
} else { // use GenericEvent approach
$result = $event->getArgument('result') ?: []; // get the result argument from GenericEvent
$result[] = $value; // add your return value into the array
$event->setArgument('result', $result);
}
}
Yes, only for specific parameters, in specific events. I tried to document all of them at https://manual.joomla.org/migrations/44-50/removed-backward-incompatibility/#module-event-onrendermodule-backward-compatibility The list: These events should use Technicaly, you can use $event->setArgument() for any argument, but it is not recomended, because most of the event class is |
|
I'm going to go with Fedir's suggestion of putting the description in Joomla 4/5 changes page, and linking to it from each details page. Otherwise there's a lot of maintenance overhead. I've updated the explanation hopefully to make it clearer. I've changed item to subject in the first content events, but I've finding a problem with NormaliseRequestDataEvent getting the subject. I think getSubject() might be missing. |
|
|
PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here. PR Reviewer Guide 🔍
|
|
PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here. PR Code Suggestions ✨
|
|
Please remove all But just removing The rest is looks good now. |
|
@Fedik I've removed the & symbols and changed some of the text as you suggested. Many thanks for your help sorting out this page - it was important to get it in a good state as a pattern for the rest. Would you mind having a quick look at the updated index page to make sure it's accurate? (I've changed it a bit). |
|
Sorry a bit late. The order of events is mixed: Actualy the Also in the events descriptions it mixed, for However it is triggered before validation. |
|
Ah thanks @Fedik , I missed that. I'll fix it up in the next pull request. |
User description
This is a draft structure for plugin events - submitted for review.
It's similar to the existing format at https://docs.joomla.org/Plugin/Events/Content but with some differences.
Let me know if you disagree with any of the following:
Also, it might be worth at some point including sortable headers - as in https://pester.dev/docs/additional-resources/articles/
PR Type
Documentation
Description
Changes walkthrough 📝
joomla-4-and-5-changes.md
Detailed Guide on Accessing Event Arguments and Returning Valuesdocs/building-extensions/plugins/joomla-4-and-5-changes.md
values for concrete, generic, and traditional legacy methods.
content.md
Comprehensive Documentation of Content Plugin Eventsdocs/building-extensions/plugins/plugin-events/content.md
return values, and examples.
onContentAfterTitle, onContentBeforeDisplay, etc.
index.md
Index of Plugin Events with Detailed Linksdocs/building-extensions/plugins/plugin-events/index.md
groups, and release versions.
classes.