First of all, I've gotta say this is an amazing theme, but I'm encountering this issue quite a lot with the bootstrap theme: We have many overridden core functions (for this example I'll use bootstrap_pager()), and often it's necessary to add/remove/change classes and markup for the output of these functions.
In this case, we want to add a class conditionally to the pager wrapper (currently classed with "text-center" but there is no way I can think to do this without copying the entire function into our theme and modifying that one line. This causes issues with updates to the bootstrap theme when bug fixes/feature enhancements come through as we'd have to manually merge the changes into our overridden function. What would be ideal is if there was a tpl.php file for these "wrapper" type markup, that way instead of:
return '<div class="text-center">' . theme('item_list', array(
'items' => $items,
'attributes' => array('class' => array('pagination')),
)) . '</div>';
You could have something like:
$items = theme('item_list', array(
'items' => $items,
'attributes' => array('class' => array('pagination')),
));
return theme('bootstrap_pager', array('items' => $items));
And bootstrap_pager would simply be a template file with:
<div class="text-center">
<?php print $items; ?>
</div>
This would give several benefits:
1) Overriding the markup would be easy, just override the template file
2) You could do preprocessing on the $items without forking the function.
3) Probably others...
However I understand that this may add extra overhead that is not wanted/desired for the theme, and if we were to do it for 1 function, then we'd want to standardise the whole theme (which may add too much overhead). I wouldn't even know where to go about doing this as I haven't wrapped my head around the whole registry.inc style of doing things but I just wanted to raise this and see what people's opinions were. We have similar issues with panels_bootstrap_layouts as well but that's a separate thing :)
Cheers
Comment | File | Size | Author |
---|---|---|---|
#9 | pager_wrapper_markup_is-2321691-9.patch | 3.46 KB | neardark |
#7 | pager_wrapper_markup_is-2321691-7.patch | 5.52 KB | neardark |
Comments
Comment #1
markhalliwellThanks!
I would like to avoid creating an entirely new theme hook. It's not really necessary. How about we use a theme_hook__SUGGESTION, like
container__pager
instead?And in your sub-theme you could create a
container.vars.php
file and insert the following:Comment #2
markhalliwellFWIW, I think this is really a bug since it was this project introduced the wrapper in the first place. I am also updating the title to better better summarize the issue.
Comment #3
markhalliwellAlso, this will need a change record in case a sub-theme has overridden Bootstrap's override.
PPS lol: Feel free to create a patch. I'd be happy to attribute credit to you for discovering this :)
Comment #4
acbramley CreditAttribution: acbramley commentedThanks very much, great to see a good solution :) I'll try to get on to a patch some time today.
Cheers
Comment #6
markhalliwellI went ahead an committed the first code block in #1. This issue still needs a change record added though.
@neardark, I was going to apply this (or attempt to) on 8.x-3.x, but it seems we no longer have a template (or func) file for pagers? Where did this go?
Comment #7
neardark CreditAttribution: neardark commentedComment #9
neardark CreditAttribution: neardark commentedComment #11
neardark CreditAttribution: neardark commented