Posted by aufumy on July 7, 2010 at 10:57pm
| Project: | Colorbox |
| Version: | 6.x-1.0-beta5 |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Add hook_colorbox_form_access($form_id);
Allow other modules to use colorbox, passing in the form id
Example:
/**
* Implementation of hook_colorbox_form_access().
*/
function my_module_colorbox_form_access($form_id) {
$access = FALSE;
if ($form_id == 'forward_form') {
return user_access('access forward');
}
return $access;
}
| Attachment | Size |
|---|---|
| colorbox_hook_access.patch | 1.95 KB |
Comments
#1
This looks neat!
Just some minor stuff.
Could we not simply skip the following part?:
if ($access) {return $access;
}
And what you think about wrapping the module_implements part like this:
// hook_colorbox_form_accessif (!$access) {
foreach (module_implements('colorbox_form_access') as $module) {
$access = module_invoke($module, 'colorbox_form_access', $form_id);
}
}
No need to run the hook if access is already true?
#2
The if (!$access) , is a good idea. Alternatively, there could be returns inside the case statement.
Regarding removing the
if ($access)within the foreach, if there are two modules A and B with hook_colorbox_form_access, then by default both will return false, unless A allows $form_id to go through.However, without the
if ($access)inside the foreach, hook for $form_id will be overwritten.#3
Sorry for forgetting this issue.
Does this patch looks ok to you?
#4
Looks great to me.
Thank you.
#5
Finally committed to 6-dev.
#6
Automatically closed -- issue fixed for 2 weeks with no activity.
#7
Can't find this patch in 7.x version, can you port it to 7.x too?