Theming of content boxes - code is repeated inside each layout plugin and not abstract enough
electricmonk - January 7, 2009 - 11:36
| Project: | MySite |
| Version: | 5.x-3.3 |
| Component: | - Layout plugin |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Jump to:
Description
Currently, the same piece of code used to theme the different boxes is repeated inside each layout plugin. In addition, if one wishes to change the appearance of all boxes, one has to override the layout theme function(s).
I've attached a patch which delegates the responsibility of theming the boxes themselves to a separate function. This makes for better software engineering and prevents the excess theme overriding mentioned above.
| Attachment | Size |
|---|---|
| mysite.patch | 10.9 KB |
#1
That's a really good patch.
#2
It seems to work OK for me, but I'm only using the 'columns' layout. I hope that there weren't any small differences between the way boxes are rendered in the different layout plugins.
#3
I don't believe so. I think it was all just copy/paste.
The only reason for keeping the code separate is if people want to make changes to one layout but not another.
#4
Ok, so my patch is definitely a good idea - they can still do that by overriding the box theming function in their template file or elsewhere.
As a general rule, I strongly encourage using multiple layers of abstraction in order to allow users to override stuff without having to duplicate whole pages worth of functionality.
#5
And if users want to re-theme one layout but not the others? We should probably pass a $layout parameter to the theme.
Understand that I agree with you, this is just a very early module that I wrote. I was surprised and pleased to get it to work at all.
#6
In this case, I would use a mediator theming function, which would look something like this: (I'm at home now, so I don't have the code in front of me)
<?phpfunction theme_mysite_box($args...) {
$func = theme_get_function('mysite_box_' . $layout);
if ($func) {
$func($args...)
}
else {
// default behavior, or a call to a default function
}
}
?>
What do you say?
#7
Works for me.
#8
Hmmm... so, are you waiting for me to make those changes, or are you going to implement them yourself sometime in the future?
#9
Waiting for a patch. This module is not in active development, and this is not critical.
#10
What I mean is - are you waiting for a patch submission by myself?
#11
Yes.