#18331 closed enhancement (wontfix)
Make get_header(), get_sidebar() and get_footer() use get_template_part()
Reported by: | Viper007Bond | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.2.1 |
Component: | Template | Keywords: | has-patch commit |
Focuses: | Cc: |
Description (last modified by )
We should consolidate code and make get_sidebar()
and it's compatriots use get_template_part()
.
While we're at it, these functions should support a subfolder argument so that sidebars can be stored in a "sidebar" folder for example.
AaronCampbell came up with this elegantly simple code on IRC: http://pastebin.com/xYjASLYe
Attachments (3)
Change History (40)
#1
@
13 years ago
- Summary changed from get_sidebar(), get_header(), etc. should make use of get_template_part() to get_sidebar(), get_header(), etc. should be refreshed
#3
@
13 years ago
Related: #15086
Once that one lands we can make all these functions wrappers of get_template_part()
AND they can support files in sub-directories cleanly.
#5
follow-up:
↓ 6
@
13 years ago
Neat, but what about theme-compat
? I'm guessing it's too early to drop it.
#6
in reply to:
↑ 5
@
13 years ago
Replying to scribu:
Neat, but what about
theme-compat
? I'm guessing it's too early to drop it.
True. Then something like this should do: http://pastebin.com/7KgqUXR5 Possibly without the sidebar-
prefix on the subfolder path.
#8
@
13 years ago
- Keywords has-patch added; needs-patch removed
18331.diff: initial patch; no subfolder parameter.
#9
@
13 years ago
- Cc trac@… added
- Summary changed from get_sidebar(), get_header(), etc. should be refreshed to deprecate get_header(), get_sidebar() and get_footer() for get_template_part()
#10
@
13 years ago
- Summary changed from deprecate get_header(), get_sidebar() and get_footer() for get_template_part() to Make get_header(), get_sidebar() and get_footer() use get_template_part()
We won't be deprecating these, just change the internals.
#13
@
13 years ago
+1 as well, and I'm all in favor of dropping theme_compat, the earlier the better. By now, Theme devs know that they are required to include header.php
when calling get_header()
(et al).
#15
@
13 years ago
18331.2.diff: remove unused $subfolder arg.
#16
@
13 years ago
Should I make a new, separate ticket for subfolder support then? (I know it'd be reliant on #15086 going in.)
#17
@
13 years ago
If this patch gets in, the subfolder discussion could be held entirely in #15086.
#19
@
13 years ago
Should we consider something like this in get_template_part(), to make these functions completely wrappers, and get_template_part('header') identical to get_header()?
if ( 'header' == $slug || 'sidebar' == $slug || 'footer' == $slug ) do_action( "get_$slug", $name );
#21
follow-up:
↓ 23
@
13 years ago
if ( 'header' == $slug || 'sidebar' == $slug || 'footer' == $slug ) do_action( "get_$slug", $name );
I don't see any benefit in that.
#23
in reply to:
↑ 21
@
13 years ago
Replying to scribu:
if ( 'header' == $slug || 'sidebar' == $slug || 'footer' == $slug ) do_action( "get_$slug", $name );I don't see any benefit in that.
I wouldn't put these checks in every call to get_template_part.
If we want to slim the functions down more we could create a function to fire the action and hook it onto the action in get_template_part that is fired in these three cases.
#24
follow-up:
↓ 27
@
13 years ago
- Resolution set to fixed
- Status changed from new to closed
Or we could just leave them as is.
Adding a helper function to save one line of code isn't good for readability or performance.
I think we're done here.
#27
in reply to:
↑ 24
@
13 years ago
Replying to scribu:
Or we could just leave them as is.
Adding a helper function to save one line of code isn't good for readability or performance.
Agreed - I was just pointing how I would implement it if we did not that I advocate doing it.
I think we're done here.
Agreed
#28
follow-ups:
↓ 32
↓ 33
@
13 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
This breaks themes that call get_header() twice as it does a double include of header.php. This seems to be a not uncommon situation.
#30
@
13 years ago
Can we add $require_once to get_template_part to fix?
function get_template_part( $slug, $name = null, $require_once = false ) { do_action( "get_template_part_{$slug}", $slug, $name ); $templates = array(); if ( isset($name) ) $templates[] = "{$slug}-{$name}.php"; $templates[] = "{$slug}.php"; return locate_template($templates, true, $require_once); }
#31
@
13 years ago
- Milestone 3.3 deleted
- Resolution set to wontfix
- Status changed from reopened to closed
We could, but get_template_part() is a higher level function, and there's been talk about adding more useful arguments like $folder.
This was designed as a simple internal abstraction, but it's not a perfect match. Closing.
#32
in reply to:
↑ 28
@
13 years ago
Replying to ryan:
This breaks themes that call get_header() twice as it does a double include of header.php. This seems to be a not uncommon situation.
Great catch!
#33
in reply to:
↑ 28
;
follow-up:
↓ 34
@
13 years ago
Replying to ryan:
This breaks themes that call get_header() twice as it does a double include of header.php. This seems to be a not uncommon situation.
Ryan, can you provide an example? I'm extremely curious why a Theme would need/want to call get_header()
twice.
#34
in reply to:
↑ 33
;
follow-up:
↓ 35
@
13 years ago
Replying to chipbennett:
Ryan, can you provide an example? I'm extremely curious why a Theme would need/want to call
get_header()
twice.
It shouldn't be, but it's being done anyway. ;)
#35
in reply to:
↑ 34
@
13 years ago
Replying to Viper007Bond:
Replying to chipbennett:
Ryan, can you provide an example? I'm extremely curious why a Theme would need/want to call
get_header()
twice.
It shouldn't be, but it's being done anyway. ;)
So, not that I disagree with the wontfix
ticket resolution (I have no real stake in the matter), but I do wonder why we would decide that resolution based on what is essentially a _doing_it_wrong() action?
#36
@
13 years ago
Including a file twice can result in a fatal error pretty easily, so warning them wouldn't help much. You basically have to rely on require_once(), which get_template_part() deliberately avoids using.
?
Any abstraction will require either another parameter or some conditional check for header/footer/sidebar, neither of which is particularly neat and enough for me to come out and say wontfix.
I'm annoyed I didn't catch this first time through as I had been down this path before..
#37
@
13 years ago
Chip, the use-case would be when a themer uses get_header() in a non-traditional way such as using it for an html5 header element include (which can be used more then once) rather then the complete top part of the html document. Now this isn't really normal for WordPress themes, but isn't incorrect per se as there was never a requirement to do it the normal way (only suggested via example in default themes). Hope that answers you question.
Corrected the link.