Hook usage
hvorragend opened this issue · 39 comments
I am not sure, because I am not a hook pro, but I think that you are using the hook object wrong.
public function ui_view(Zikula_DisplayHook $hook)
{
$modname = $hook->getCaller();
$textfieldname = $hook->getId();
You are using getId() for getting the textfield.
A few months ago I was corrected that Id is the object of an item.
I don't have much experience with OOP, so it is hard to describe.
Just take a look at the News module or Craig's Tag module.
Example:
{notifydisplayhooks eventname='news.ui_hooks.articles.form_edit' id=$item.sid}
I will check that.
Here the example of Dizkus
{notifydisplayhooks eventname='dizkus.ui_hooks.editor.display_view' id='message'}
I used the the 'id' tag because {notifydisplayhooks} just accepts eventname, id and objecturl. So id is the only thing we can use.
{notifydisplayhooks eventname='dizkus.ui_hooks.editor.display_view' textfield='message'}
is not possible.
IMHO id does not have to be always the object of an item.
what is the other information?
a text from a textfield?
why don't just use FormUtil::getPassedValue to get the contents of the posted data?
There is no other information to pass to the hook. If it needs to access the request object it should do so itself.
I think he is looking for the DOM id of the text field, not the actual contents. but I'm not sure.
I think he is looking for the DOM id of the text field, not the actual contents. but I'm not sure.
yes
All our hooks (LuMicuLa, BBCode, BBSmile) are having the problem now, that we don't know the DOM id of the textarea.
How should we know where the hook interface should be attached to?
maybe it could be encoded in the Zikula_ModUrl()
I don't like the idea of Zikula_ModURL() .
How about if we do that just as in the Scribite module?
I think that we only have two possible solution:
We add a kind of "module registry table" in the backend where we can configure the following fields:
- modulename
- modfuncs
- modareas
- textarea ID (maybe)
If the above conditions match, we search via javascript for all textarea IDs.
Can someone explain, from the beginning, what the problem is?
@hvorragend - in postcalendar's hook implementation, I add an item to any hooked module's admin menu. this allows the admin to make some selections about how he hook is used on a per module basis. Something like this may work (the admin could define the DOM id).
@craigh: You are talking about "PostCalendar Event Maker", right? Nice feature.
But unfortunately for me that is all too complicated. I'm not a PHP programmer and I have no idea of OOP.
Unfortunately I am no longer able to help with the programming of the modules, because it is technically too complicated to me.
@Drak: There are situations where a hook must attach an interface to a textarea.
LuMicuLa, BBCode or BBSmile have a user interface with several buttons. And if I want to use BBCode or smilies in some of our forms (new topics in Dizkus, new articles in News), then I need the icon bar.
The problem is that the interface has to be embed near the textarea. So I need the DOM ID of this textarea or of all textareas. There can be more than one textarea tag.
I hope this is somewhat understandable. :-)
it seems the easiest choice is to add a specific class to the element that you want to attach the hook.
for example <textarea class="lumicula"></textarea>.
then, it can be attached with js.
@tfotis - Then every module will have to add this class - that sort of defeats the point.
How does Scribite handle this? It has no problems with multiple textareas afaik. It might be that hooks are just not the way to do this particular task - look at Scribite: https://github.com/zikula-modules/Scribite/blob/master/src/modules/Scribite/lib/Scribite/Listeners.php
@Drak let me clarify, I am not saying that every module should do that, of course this defeats the whole point of hooks.
But let's say that i am an end-user that wants to attach lumicula to a certain textarea of News module. Lumicula hook could have some instructions of how to do that eg attach the hook, override the template, edit it and add the needed class.
Now that i think of it, it's not that simple (it needs some basic understanding of zikula) :)
Sure, but Scribite has a solution so why not let's use that?
Because LuMicuLa, BBCode and BBSmile are using filters we need individuel templates anyway. So the Scribite solution (core hook and database) is not necessary. I dont see the problem. The current solution works without problems.
I can't remember about this for sure, but I think I remember that BBcode or BBSmile had an API that the "calling" module would use to insert the needed code and render the textarea correctly. I know that Scribite does the same.
Hooks are probably a poor way to implement an editor. filters are different - that could be done with hooks, but the editors need to be inserted.
maybe each editor (scribite, BBsmile, Lumicula, etc) could implement an interface that could be utilized by any module wishing to utilize the interface? So the calling module just tests for the presence of an editor and initializes it on the page. then the editor module handles which editor and so forth.
BBCode was called by an smarty plugin with ModUtil::available. The current solution is much better.
what is the current solution? I thought that was the problem?
The current solution is a hook (instead of the smarty plugin). This allows to use different engines (BBCode, LuMicuLa).
The textfield will handled by the id field. This is not the official way, but there is no problem because this specific editor hook does not need id's. It works without problems.
In my opinion the solution is okay.
An alternative solution would be to add the possibile for hooks to assign other params. But I think this effort is not really needed.
There are 3 solutions:
- The current solution. Use the id field
{notifydisplayhooks eventname='dizkus.ui_hooks.editor.display_view' id=$textareaid} - Add a class to identify the possible textaereas:
<textarea id="message" name="message" cols="10" rows="60" class="editor"></textarea> - convert the hooks to core hooks and add database tables in LuMicuLa, BBCode and BBSmile to store the textarea ids (Scribite solution)
I am still for solution 1. But solution 2 would be okay for me as well.
Solution 1 follows the standard.
No, that is not correct.
There is no field $textareaid. It looks like a php parameter, but meant is just the DOM ID of a textarea. What if we have more than one textarea in a HTML form?
Perhaps this commit helps to explain it: zikula-modules/DizkusModule@54f309a
What if we have more than one textarea in a HTML form?
That is no problem. In this case you have to add another
{notifydisplayhooks eventname='dizkus.ui_hooks.editor.display_view' id='textarea2'}
But solution 2 has in the case of LuMicuLa a little bit a better performance.
Then you see the other hooks (Captcha, Tag) twice, too.
----- (Sent via mobile phone) -----
Am 06.05.2012 um 10:56 schrieb phaidonreply@reply.github.com:
What if we have more than one textarea in a HTML form?
That is no problem. In this case you have to add another
{notifydisplayhooks eventname='dizkus.ui_hooks.editor.display_view' id='textarea2'}
Reply to this email directly or view it on GitHub:
#18 (comment)
Then you see the other hooks (Captcha, Tag) twice, too
No, because there are not using dizkus.ui_hooks.editor.display_view.
Solution 1 follows the standard.
not exactly - if they use a non-integer as their $textareaid then it does not follow the standard.
Something just occurred to me. I heard BBCode etc are implemented using filters? If this is so, then DisplayHook is not the right object, it should be the FilterHook because DisplayHooks add content responses, they don't filter existing content.
They are two hooks. A filter hook for the transformation and a Display hook for the editor.
I vote for solution 1 and 2.
Both are easy to realize and there are no additional queries needed.
No, because there are not using
dizkus.ui_hooks.editor.display_view.
Unfortunately, I do not know much about the new Zikula hooks. However, in my installation I can attach more hooks to the Dizkus ui_hooks hook type.
Dizkus editor
(subscriber.dizkus.ui_hooks.editor)
|
|-- LuMicuLa editor
| (provider.lumicula.ui_hooks.lml)
|
|-- Captcha Service Display
| (provider.captcha.ui_hooks.service)
|
|-- Content tagging service
| (provider.tag.ui_hooks.service)
|
|-- EZComments Comment Hooks
(provider.ezcomments.ui_hooks.comments)
The area dizkus.ui_hooks.editor.display_view is associated with the display_view ui_hooks hook category. The same category is used by other hooks, too. As a result, now I see in Dizkus each hook with each other. This is obviously illogical and destroys the layout.
Furthermore, it is quite complicated to determine the exact position of the hooks. At the point where your editor should normally be displayed, will now also display Tags and Catcha. So all at once. Better would be if your module is displayed only at the textarea and the other hooks elsewhere. But it seems there is no way that we can separate the display from one another.
Tag
$bundle = new Zikula_HookManager_ProviderBundle($this->name, 'provider.tag.ui_hooks.service', 'ui_hooks', $this->__('Content tagging service'));
$bundle->addServiceHandler('display_view', 'Tag_HookHandlers', 'uiView', 'tag.service');
$bundle->addServiceHandler('form_edit', 'Tag_HookHandlers', 'uiEdit', 'tag.service');
$bundle->addServiceHandler('validate_edit', 'Tag_HookHandlers', 'validateEdit', 'tag.service');
$bundle->addServiceHandler('process_edit', 'Tag_HookHandlers', 'processEdit', 'tag.service');
$bundle->addServiceHandler('process_delete', 'Tag_HookHandlers', 'processDelete', 'tag.service');
LuMicuLa
$bundle = new Zikula_HookManager_ProviderBundle($this->name, 'provider.lumicula.ui_hooks.lml', 'ui_hooks', __('LuMicuLa editor'));
$bundle->addServiceHandler('display_view', 'LuMicuLa_HookHandler_Lml', 'ui_view', 'lumicula.lml');
$this->registerHookProviderBundle($bundle);
$bundle = new Zikula_HookManager_ProviderBundle($this->name, 'provider.lumicula.filter_hooks.lml', 'filter_hooks', __('LuMicuLa transform'));
$bundle->addStaticHandler('filter', 'LuMicuLa_HookHandler_Lml', 'filter', 'lumicula.lml');
$this->registerHookProviderBundle($bundle);
EZComments
$bundle = new Zikula_HookManager_ProviderBundle($this->name, 'provider.ezcomments.ui_hooks.comments', 'ui_hooks', $this->__('EZComments Comment Hooks'));
$bundle->addServiceHandler('display_view', 'EZComments_HookHandlers', 'uiView', 'ezcomments.hooks.comments');
$bundle->addServiceHandler('process_edit', 'EZComments_HookHandlers', 'processEdit', 'ezcomments.hooks.comments');
$bundle->addServiceHandler('process_delete', 'EZComments_HookHandlers', 'processDelete', 'ezcomments.hooks.comments');
$this->registerHookProviderBundle($bundle);
$bundle = new Zikula_HookManager_SubscriberBundle($this->name, 'subscriber.ezcomments.ui_hooks.comments', 'ui_hooks', $this->__('EZComments Comment Hooks'));
$bundle->addEvent('display_view', 'ezcomments.ui_hooks.comments.display_view');
$bundle->addEvent('form_edit', 'ezcomments.ui_hooks.comments.form_edit');
$bundle->addEvent('form_delete', 'ezcomments.ui_hooks.comments.form_delete');
$bundle->addEvent('validate_edit', 'ezcomments.ui_hooks.comments.validate_edit');
$bundle->addEvent('validate_delete', 'ezcomments.ui_hooks.comments.validate_delete');
$bundle->addEvent('process_edit', 'ezcomments.ui_hooks.comments.process_edit');
$bundle->addEvent('process_delete', 'ezcomments.ui_hooks.comments.process_delete');
$this->registerHookSubscriberBundle($bundle);
Sometimes you just talk about it. Then one realizes often where the problem really lies. Now I know what you meant. :-)
We simply create several areas and then we can assign the hooks differently.
I am currently trying it with the News module.
The problem here is really that id=$textareaid is legacy and not possible anymore because in the new hook system the id argument is used for the object id already (id=$forumTopic.id or similar).
When it comes to the decision about how to pass the textarea id the way Scribite is doing it would be one possibility. However, it would require introducing new database tables in all affected modules (like BBCode, BBSmile, LuMicuLa, and others). Another possible solution would be a new interface defining a standard for display hook arguments. One of these arguments could be some (optional) textarea information. If it is not available then the hook provider returns nothing.
I think the best solution is to separate the problem.
- The editor I want to solve by a coreint- event listener (Scribite solution)
- The filter I want to solve by an filter hook (like now)
- The management of the module settings will happen by the LuMicuLa (Scribite solution)
This is not a super elegant solution but it does not break any standards and we can make it user friendly.
I will also fix BBCode, BBSmile and Wikka. So all modules can be used.
I know this was a discussion from a while ago, but I just finished reworking BBCode and BBSmile. I like the solution I came up with which actually doesn't use any of the solutions above.
I simply wrote a Javascript 'listener' to track where the input was last, then when clicking on the BB interface, it inserts the text into the last active textarea. It works very well and I believe is an elegant solution.
This method allows for the current 'hook-agnostic' implementation in the hooks system.
I don't think this exact solution would work for editors however. But there might be a similar way to solve it in this manner by scanning the html form for textareas after it is rendered and then attaching the editor afterwards.