#20249 closed defect (bug) (fixed)
Remove custom header and background constants
Reported by: | nacin | Owned by: | |
---|---|---|---|
Milestone: | 3.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Themes | Keywords: | has-patch |
Focuses: | Cc: |
Description
The custom header and custom background code are both weighed down heavily with constants and multiple functions for registering them.
For example, adding a custom header requires the simple add_custom_image_header() call, which takes three callback arguments. But, it also requires up to five constants to customize things. Additionally, for random default headers, you need to make an add_theme_support() call. If you don't want uploads, you need to do remove_theme_support('custom-header-uploads'). If you want the new flex-header stuff, they are additional arguments in add_theme_support(). If you want to remove support, you need to call remove_custom_image_header(), not remove_theme_support(). And, if you need to overload certain pieces in a child theme, good luck.
See, that's a lot going on. Let's simplify it.
A single add_theme_support('custom-header', $args) that handles everything except for default header registration. Multiple calls to it will result in the existing $args being merged with the newly passed args. So, for example, a child theme can remove uploads by simply calling add_theme_support('custom-header', array('uploads' => false) ).
Attached patch kills off the constants, removes extraneous functions, and causes everything to just wrap add_theme_support() and remove_theme_support().
It does some very particular things for backwards compatibility, like defining constants that aren't already defined, in case a plugin or parent theme references them, and to ensure that a child theme consistently overrides the wishes of the parent.
custom-header.php has not been ported over to rely on the new API. As a testament for this code's backwards compatibility (or potential thereof, in case of a minor bug or two), everything works.
I am going to try to crowd-source a test of all 90-something themes in http://wpcom-themes.svn.automattic.com to ensure full compatibility. (Some of the just-in-time stuff was a result of seeing that Twenty Eleven defined a header constant *after* add_custom_image_header(), which threw me in for a loop.)
Attachments (3)
Change History (38)
#2
@
13 years ago
I just deleted my local patch accidentally. There is death-to-constants-and-lame-functions.diff:ticket:17242, which I will refresh shortly with a few more changes I had made.
#5
@
13 years ago
20249.2.diff tears up custom-header.php as well. No more background or header constants are referenced outside of the back compat code. I'll post a summary of everything the patch does API-wise, as well as testing notes, over the weekend.
#12
@
13 years ago
Nitpick:
_deprecated_function( __FUNCTION__, '3.4', 'add_theme_support(\'custom-header\', $args)' );
I thought the policy was to avoid backslashes if possible.
#13
@
13 years ago
Ah, if you used double-quotes, you'd have to add a backslash anyway: \$args
. Nevermind.
#15
follow-up:
↓ 19
@
13 years ago
I find the callbacks, 'callback' and 'admin-header-callback', to be somewhat confusingly named. Of course, that could just be me. :) 'head-callback' and 'admin-head-callback' might be more helpful.
#19
in reply to:
↑ 15
;
follow-up:
↓ 21
@
13 years ago
Replying to iandstewart:
I find the callbacks, 'callback' and 'admin-header-callback', to be somewhat confusingly named. Of course, that could just be me. :) 'head-callback' and 'admin-head-callback' might be more helpful.
I agree. I called it 'callback' because we were already using that internally, but we could scrap that as it wasn't behavior anyone else would use. (It was only used to remove the hook in remove_custom_image_header().)
If I had to pick three names, it would be:
- wp_head
- admin_head
- admin-preview-callback
Obviously, those are not standardized. I could go for:
- wp-head-callback
- admin-head-callback
- admin-preview-callback
Essentially, dropping "header", as it's the admin <head>, not the header, and cleaning up the unsemantic admin-image-div-callback name.
#20
@
13 years ago
I do like the simplicity of 'callback', though, especially since it is frequently going to be the only callback used (for custom headers, at least; custom background callbacks are not often used).
#21
in reply to:
↑ 19
@
13 years ago
Replying to nacin:
Obviously, those are not standardized. I could go for:
- wp-head-callback
- admin-head-callback
- admin-preview-callback
Sounds good. Easy to understand and to remember.
#22
follow-up:
↓ 23
@
13 years ago
I can't get my default background image to show.
Code used:
add_theme_support( 'custom-background', 'default-color' => 'e3ecf2', 'default-image' => '%s/images/bg.png' ) );
By changing line 204 in wp-admin/custom-background.php from
$background_styles .= ' background-image: url(\'' . get_theme_mod('background_image_thumb', '') . '\');'
to
$background_styles .= ' background-image: url(\'' . get_background_image() . '\');'
it will show. I don't know if it's the best solution but it works.
#23
in reply to:
↑ 22
@
13 years ago
Replying to Jayjdk:
I can't get my default background image to show.
Code used:
add_theme_support( 'custom-background', 'default-color' => 'e3ecf2', 'default-image' => '%s/images/bg.png' ) );By changing line 204 in wp-admin/custom-background.php from
$background_styles .= ' background-image: url(\'' . get_theme_mod('background_image_thumb', '') . '\');'to
$background_styles .= ' background-image: url(\'' . get_background_image() . '\');'it will show. I don't know if it's the best solution but it works.
Did this work with constants under 3.3? Doesn't look like it would.
#25
@
13 years ago
[20231] Callbacks for custom headers and custom backgrounds registered through add_theme_support() are now wp-head-callback, admin-head-callback, and admin-preview-callback.
#26
in reply to:
↑ 24
@
13 years ago
Replying to Jayjdk:
Nope. It doesn't work in 3.3 with constants
The default color and default image should both be defined in the stylesheet, which is why the _custom_background_cb() does not handle them. The reason to additionally define them in PHP is so the Background and Header screens can provide a better experience as they relate to WYSIWYG and resetting to defaults. (That said, both screens are pretty bad at this, but they can be made better.)
#27
follow-up:
↓ 28
@
13 years ago
But shouldn't the background preview show the defined background image? It shows the defined background color
#28
in reply to:
↑ 27
@
13 years ago
Replying to Jayjdk:
But shouldn't the background preview show the defined background image? It shows the defined background color
Yes. At the moment, the admin screens poorly handle default images and colors. And sorry, I didn't realize your code in comment 22 came from wp-admin/custom-background.php; I figured it came from the front-end _custom_background_cb(). We are on the same page.
#29
@
13 years ago
Seems [20231] will need a check whether a callback exists for add_action( 'wp_head', $args[0]['wp-head-callback'] );
on line 1444 in theme.php same as on line 1430. Getting an empty action when using older theme (not Twenty Ten or Eleven)
#30
@
13 years ago
What is the theme using to register a custom background?
If a custom background's callback is empty, it uses _custom_background_cb(). This is different from headers, which allows no callback. Hence why there is no empty check on 1444 but there is one on 1430.
This is a follow-up from #17242.