Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
mysite.patch | 10.9 KB | electricmonk |
Comments
Comment #1
agentrickardThat's a really good patch.
Comment #2
electricmonk CreditAttribution: electricmonk commentedIt 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.
Comment #3
agentrickardI 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.
Comment #4
electricmonk CreditAttribution: electricmonk commentedOk, 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.
Comment #5
agentrickardAnd 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.
Comment #6
electricmonk CreditAttribution: electricmonk commentedIn 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)
What do you say?
Comment #7
agentrickardWorks for me.
Comment #8
electricmonk CreditAttribution: electricmonk commentedHmmm... so, are you waiting for me to make those changes, or are you going to implement them yourself sometime in the future?
Comment #9
agentrickardWaiting for a patch. This module is not in active development, and this is not critical.
Comment #10
electricmonk CreditAttribution: electricmonk commentedWhat I mean is - are you waiting for a patch submission by myself?
Comment #11
agentrickardYes.
Comment #12
agentrickard