Themes: Allow flexible sizes for custom header uploads

[lancewillett]

If a theme allows flexible header images the custom header feature should adjust the crop settings on upload to let the image through.

Theme Support

add_theme_support( 'flexible_header_image_upload', 1000, 200 );
Where 1000 is the maximum width and 200 is the maximum height allowed.

Custom Header Code

If the theme_support option exists and passes second argument (dimension as an integer), and the uploaded image dimension is higher, crop to size. If the uploaded image dimension is lower, save it with no crop.

If the option is on but dimension value is false or 0, let any size through (no crop at all).

Admin UI

If a theme supports this, list the maximum dimensions in Appearance > Header. If no maximums are defined, leave it blank.

[ocean90]

Why we should do this only for the height?

[nacin]

One thing to note, the constants are largely left over from when we had header images in Kubrick. They're ugly and inflexible. It would be great to move away from them everywhere possible, and go in the direction of add_theme_support() with an array of extra arguments being passed to it.

[nacin]

Directly referring to this ticket, I don't understand why there needs to be a UI for this. If the theme allows flexible height images, then they can adjust the crop upon upload, assuming the image is not exactly the default dimensions. Simple enough.

[aaroncampbell]

Things I've struggled with relating to this ticket:

• It's really annoying that you can only use one header image and one background image. This is only tangentially related to this ticket, but it would be nice to allow a theme to use multiple "header images" (obviously allowing them to name them differently) each with their own sub page area to manage them from. Consider the simple case of a theme that allows a background image, a header image, and a logo.
• Since I see people trying to use this for logos pretty often (even tried it myself before I gave up on it) it would be nice to be able to set a maximum width or maximum height (and I mean 'or', setting a max width and no max height would be ideal in most cases). Basically allowing someone to say "as long as it fits here it's fine".

[lancewillett]

Replying to ocean90:

Why we should do this only for the height?

Because theme width isn't flexible generally, but height of the header is.

Replying to nacin:

It would be great to move away from them everywhere possible, and go in the direction of

add_theme_support() with an array of extra arguments being passed to it.

Totally agree.

[lancewillett]

Replying to aaroncampbell:

It's really annoying that you can only use one header image and one background image. This is only tangentially related to this ticket

I agree. See #15100 (reusing header images) and #17241 (see note about reusing background images).

it would be nice to be able to set a maximum width or maximum height (and I mean 'or', setting a max width and no max height would be ideal in most cases). Basically allowing someone to so "as long as it fits here it's fine.

Good idea. That way it's flexible up to the constraints of the theme layout. I think that would work for logos, but for full-width header images it might lead to some ugly-ish layouts if the header image doesn't fill the width allotted.

[lancewillett]

Replying to nacin:

If the theme allows flexible height images, then they can adjust the crop upon upload, assuming the image is not exactly the default dimensions. Simple enough.

Let's do that. :)

That approach is what I have in the ticket under "Crop Without Height" but maybe implemented with add_theme_support and not the ugly definitions.

[aaroncampbell]

Replying to lancewillett:

Replying to ocean90:

Why we should do this only for the height?

Because theme width isn't flexible generally, but height of the header is.

I'm not sure what percentage of themes allow it, but even on fixed width themes is not that uncommon to allow the header image to continue out past the fixed width. I'll upload a screenshot of a theme where we could use a flexible width as well (in this example there's no background image, but you CAN have a background image in addition to the header in this theme).

[lancewillett]

Replying to aaroncampbell:

I'm not sure what percentage of themes allow it, but even on fixed width themes is not that uncommon to allow the header image to continue out past the fixed width.

I'm seeing now that this could really be for both width and height, as ocean90 pointed out initially.

I think we should go with Nacin's idea that if a theme allows flexible header images the custom header code can adjust the crop settings on upload to let an image through.

Idea for the theme notation:

• add_theme_support( 'allow_flexible_header_height', 200 );
  Where 200 is the maximum height allowed.
• add_theme_support( 'allow_flexible_header_width', 1000 );
  Where 1000 is the maximum width allowed.

If the option is on and a second dimension argument is passed (integer), and the uploaded image dimension is higher, crop to size. If the uploaded image dimension is lower, save it with no crop.

If the option is on but dimension value is false or 0, let any size through (no crop at all).

The max dimension cropping is "nice-to-have" for this feature — but it would make this a great compromise between allowing flexibility while allowing theme authors to control the look of their themes to some extent.

It might also help with the case where a user uploads an original (potentially huge) image from their camera and want to use it as a header. :)

[lancewillett]

I've been working on this for a few days now, changing the crop parameters and seeing how things work when you change only the width or height when you crop an uploaded image.

I also experimented with replacing HEADER_IMAGE_WIDTH and HEADER_IMAGE_HEIGHT in a few places.

Attached is a patch with my first pass, it's mostly a proof-of-concept for the cropping mechanism changes.

There are a to main blocker issues with this feature:

1. Image dimensions are not available if not the default size.

• Themes with flexible uploaded images have no mechanism to get the width/height dimensions of the uploaded image, for use in HTML attributes or CSS.
• Themes that use the same values stored in HEADER_IMAGE_WIDTH and HEADER_IMAGE_HEIGHT definitions in other places than the custom header are going to have trouble if they allow flexible sizes. For example, if upload a smaller header than is defined in the theme — there isn't a way for the theme to get its dimensions and apply it to other areas in the theme.
• In Twenty Eleven you could end up with a short header image (works great) but taller featured image headers since the featured image call doesn't change to the smaller dimension.

2. Cropping tool isn't ready for flexibility.

• To allow correct cropping by one dimension only (when the width or height isn't flexible) the existing crop functionality would need a rewrite to remove the need for an exact aspect ratio.
• I found that when one dimension is flexible (and the other not) the cropping tool "zooms up" to the uploaded image dimension that you've fixed as not flexible. So even if you crop correctly the fixed dimension changes.
• Allowing the 'flexible_header_image_upload' call with ints as arguments proved a bit too complex. My patch implements the arguments as boolean values, so the flexibility is either on or off. The maximum width or height is then taken from HEADER_IMAGE_WIDTH and HEADER_IMAGE_HEIGHT.

All in all I think this idea is still worth pursuing, but it seems that we'd need to first revamp all the cases where HEADER_IMAGE_WIDTH and HEADER_IMAGE_HEIGHT are used, and replace them with set and get functions. Set to denote the defaults (and any flexibility desired) and get to make sure you can return image info anywhere in the theme — not only the source path (as get_header_image() does now) but also other image attributes.

If the flexible height is a blocker for Twenty Eleven we should consider implementing it in the theme itself — using user input to set the height value — and use that dimension to override HEADER_IMAGE_HEIGHT in the theme.

[jane]

Couldn't make it before freeze. Will shoot for next cycle.

[aaroncampbell]

I'm still really interested in getting this done. Lance, maybe we can work on this some at Gangplank next Friday?

Currently I have settings in the theme to allow a user to change these before they upload, but that makes for a really poor user experience.

[lancewillett]

Replying to aaroncampbell:

I'm still really interested in getting this done. Lance, maybe we can work on this some at Gangplank next Friday?

Maybe... I think the widget fixes are higher priority right now.

[jane]

Moving to 3.4 because we'll be dealing with custom headers, don't want this to get lost in shuffle in case there's any code here that can be used.

[aaroncampbell]

I think our first step needs to be deciding what we want to support, then deciding how that will work for theme developers.

1. I think it makes sense to support flexible widths and heights. Height is obvious, but even in fixed width themes allowing the header to extend past the content can help eliminate the boxy feel.
2. We need to decide if we're going to allow for min and/or max. Here are the possible options:
   1. Support flexible sizes for width and/or height. This would let a theme author say any of the following:
      1. Header should be 1000px wide and 300px high
      2. Header should be 1000px wide and any height
      3. Header can be any width and 300px high
      4. Header can be any size
   2. Support minimums. This would add the ability to say that the header should be at least 1000px wide or at least 40px high. This seems to make sense for widths more than heights.
   3. Support maximums. This would add the ability to say that the header should be no more than 1000px wide or no more than 500px high. This seems to make more sense for heights than widths.
   4. Support both min and max. This would let the theme say that the header should be at least 1000px wide and no more than 500px high.
3. Once we decide the above we'll need to decide what theme authors need to do to add support. It seems obvious to add_theme_support() but depending on whether we support min and/or max we'll have to decide on the exact wording/syntax.

My original stance was that it would be really nice to allow for the min/max options. However, the more I think about it the more I realize this really encourages theme authors to take control away from users. While I look at a 1000px wide design and think a header should be at least that wide, what's the big deal if a user wants a 500px wide header? It seems like they should be able to make their site look however they want. I think option 'a' that just allows fixed or flexible "helps you make things look the way you want" more than the others.

[sabreuse]

I agree that supporting fully flexible sizes is the way to go with this: aside from there being good reasons to allow going both smaller and larger than a default image, we also don't want to be prematurely limiting what themes can be designed to allow.

[aaroncampbell]

In the last dev chat (​Starting here on 01/18/2012) we decided to handle flex height and see how that works, with the possibility of adding flex height on the next two week cycle.

The new patch does that. It also updates the imgAreaSelect jQuery plugin, which wasn't strictly necessary, but seemed to make sense.

Basically a theme can add support for custom headers with flex width like this:

add_theme_support( 'custom-header', array( 'flex-height' => true, 'suggested-height' => 300 ) );

It also seems like we should add a minimum width requirement for uploaded images so that we don't scale them up (you can currently upload a 125x125 image, go through the JS crop, and get a horribly ugly header image that has stretched it to 7x it's original width). If we add flex width it won't matter for themes that support that, but it would still affect all the others.

[aaroncampbell]

Sorry, use the .2 patch​. The first one was applying the flex height functionality to the javascript cropping ALL the time instead of only for themes that support flex height.

[ocean90]

It also updates the imgAreaSelect jQuery plugin, which wasn't strictly necessary, but seemed to make sense.

#19881

[aaroncampbell]

The one real issue that we face is getting the size of the header image since it's not fixed. For the preview on admin I just moved it from being a background image to being a regular image tag. This fixes the unknown height issue. The alternative that I see is to do something like this and drop it into the CSS:

$header_image = get_header_image();
$header_image_post = get_page_by_title( substr( $header_image, strrpos( $header_image, '/') + 1 ), null, 'attachment' );
$header_image_info = image_downsize( $header_image_post->ID, 'full' );
$header_height = $header_image_info[2];

This seems really heavy just to get the height of an image (too bad everyone doesn't have getimagesize()). However, I'm wondering if we're going to need something for theme authors to be able to use. It seems that in *most* cases they'll want to add the header image as a background and need to know how big to make the container they apply it to.

[aaroncampbell]

Oh, and the latest patch doesn't have the imgareaselect update in it anymore. That's in #19881 now

[sabreuse]

This approach to resizing works really well for me on Linux (FF + Chrome) and Mac (FF + Chrome + Safari), but the switch to an img tag in the last patch messes with the UI on the Custom Header admin page.

[aaroncampbell]

Ok, there are two issues that I'm concerned about and want feedback on:

1. It seems like we should add a minimum width requirement for uploaded images so that we don't scale them up. You can currently upload a 125x125 image, go through the JS crop, and get a horribly ugly header image that has stretched it to 7x it's original width. If we add flex width it won't matter for themes that support that, but it would still affect all the others. Talked to both ​Rboren and Koop in irc and they both agreed.
2. Additionally, the issue with getting the header image dimensions is nagging at me. For the admin preview we could change to using an image tag instead of a background image, but the themes that layer site name over the top would have issues. Additionally, as I pointed out above, I think it's common for a theme to need to know the dimensions, so I think we probably need a way to give those to them. I could put the code above into a helper function of some kind (maybe even return an array like ​getimagesize()), but I don't like the weight of the function and was hoping someone else had a brilliant solution.

[sabreuse]

First pass at using a helper function

[sabreuse]

17242.4.diff is a first pass with a helper function.

ISSUES:

• As of now, it's only testing for either of an old-school non-flex header with a fixed height, or a resized header -- it won't check in the case that a theme supports flex headers but one hasn't been set yet.
• Next pass, I'll move the check out of the function so we don't run it on non-flex themes.
• We also talked a bit in IRC about grabbing the attachment ID so we don't have to query the db every time.

[aaroncampbell]

​More IRC talk. Currently we store just the header image URL, storing it in a 'header_image' theme mod. We talked about possibly storing the attachment ID along with the url. This makes sense because even if it's a slightly expensive function it would only run when you change the header. We could possibly even store the image dimensions when the header is set (would help handle the default headers that aren't in the media gallery). However, the issue is with the random header option. Currently we have get_random_header_image() which returns a URL to header image. The problem is that if a random header is used, we have no way to know which one was used to give a size later.

We could consider a whole new function that returns a random image as well as the size info, but (get_)header_image() already uses the get_random_header_image() and needs a URL. Any ideas for an elegant solution to handling the random headers?

[aaroncampbell]

17242.5.diff​ adds a helper function get_header_image_height(). It also adds 'height' and 'width' elements to the header image array. This will give themes the ability to package header images of all different heights. This patch also puts the header preview back how it was (CSS background image) and uses the new get_header_image_height() for the CSS.

We just need to figure out what we want to do with random headers and we'll be all set for flex heights. Flex widths would actually be really easy to add at this point, so I still think we should try to get both into 3.4.

[sabreuse]

17242.6 is 17242.5 with the additional change of removing the 100px min height from #headimg to allow small images in preview without tiling.

[sabreuse]

Leaving aside the question of how to handle random headers, I'm seeing a couple of issues with storing the height -

1. We need to redraw the preview panel in step 3 if a new image has been uploaded. Right now, preview is keeping the height from the previous and stretching/shrinking, and then refreshing to the correct size after confirming.
2. The "Restore Original Header Image" button doesn't reset to the original header's height.

[aaroncampbell]

Thanks sabreuse, 17242.7.diff​ should fix both those issues. Still thinking on the random header issue.

[sabreuse]

This rocks! And yeah, still puzzling over random here too.

[aaroncampbell]

Ideas for consideration:

1. Leave get_random_header_image() how it is and create get_random_header_data() (or something similar) which will return all the data on a header instead of just the URL. Themes would have to switch to the new function if they support random flex-height headers and need access to the height. The big disadvantage here is that get_header_image_height() won't know the correct height for random headers.
2. Add another global called something like $wp_random_header that stores the header data in it.
3. A combination of the above: Create get_random_header_data() and make get_random_header_image() use it. In the new function we could create the $wp_random_header global (Actually we could call it $wp_header and also create it in get_header_image() for consistency if we think it's useful). Then if we're using a random header get_header_image_height() could pull from the global.

I think #3 seems like the best option, but I'm open to ideas.

[aaroncampbell]

Ok, I think 17242.8.diff​ is a good commit candidate. It handles the random header issue by adding a $_wp_random_header global which stores the data for the header image. One thing of note is that you'll have to call (get_)?header_image() BEFORE calling get_header_image_height().

Seems to work well here, and we're REALLY close to being able to do flex widths too...I'll actually create a patch that includes this in case we want to put both into this one dev cycle.

[SergeyBiryukov]

printf( __( ' Suggested height is <strong>%1$d pixels</strong>.' ), ... ); 

It's probably better to move the space out of the translated string.

[aaroncampbell]

17242.9.diff​ removes the leading space from translated string, adds a get_header_image_height() helper function, and fixes a notice that would occur when NOT using a random header.

[aaroncampbell]

17242-height-width.diff​ adds both flex height AND flex width.

For both recent patches you should be able to use the latest ​Essence theme from github to test.

[aaroncampbell]

If you want to test with your own theme, here's how you add support for flex headers:

// Add support for flexible headers
$header_args = array(
	'random-default' => true,
	'flex-height' => true,
	'suggested-height' => 200,
	'flex-width' => true,
	'suggested-width' => 950,
);
add_theme_support( 'custom-header', $header_args );

Then when registering default headers you want to specify a width and height:

register_default_headers( array (
	'berries' => array (
		'url' => '%s/images/headers/berries.jpg',
		'thumbnail_url' => '%s/images/headers/berries-thumbnail.jpg',
		'description' => __( 'Berries', 'essence' ),
		'width' => 940,
		'height' => 198,
	),
) );

To display the header you can do something like this:

<div id="headimg" style="background-image:url(<?php esc_url ( header_image() ) ?>);width:<?php echo get_header_image_width(); ?>px;height:<?php echo get_header_image_height(); ?>px;"></div>

[aaroncampbell]

17242-height-width.2.diff​ is based on feedback from nacin and Mark Jaquith in ​IRC.

• It gets rid of theme_supports_flex_width_headers() and theme_supports_flex_height_headers() in favor of current_theme_supports( 'custom-header', 'flex-width' ).
• It changes get_random_header_data() to return an object so it can be chained like get_random_header_data()->width.
• It introduces get_current_header_data() which also returns an object, and it adds the attachment id to that object as ->id if it's an uploaded image.
• I kept get_header_image_width() and get_header_image_height() because they fall back to the constants.
• Additionally it gets rid of the $_wp_random_header global in favor of a static variable in get_random_header_data().

[aaroncampbell]

Hopefully 17242-height-width.3.diff​ is the last patch before this lands. The only change from the previous patch was to change get_random_header_data() to _get_random_header_data() and call it private since it really isn't something that anyone should need to use (they should be using get_current_header_data()). Also, nacin mentioned possibly removing it at some later point.

[aaroncampbell]

17242-height-width.4.diff​ fixes the logic in current_theme_supports( 'custom-header', 'flex-*' ) and also limits the max-width of a header to 1500px unless the theme specifies otherwise. You can specify a larger max size like this:

// Add support for flexible headers
$header_args = array(
	'random-default' => true,
	'flex-height' => true,
	'suggested-height' => 200,
	'flex-width' => true,
	'max-width' => 2000,
	'suggested-width' => 950,
);
add_theme_support( 'custom-header', $header_args );

The reason for this is that cropping an image that is too wide is cumbersome.

[ryan]

PHP Catchable fatal error:  Object of class stdClass could not be converted to string in /Applications/XAMPP/xamppfiles/htdocs/trunk/wp-includes/theme.php on line 1354

The header_image_data theme mod is fetched with a $default of an empty object. This kills the sprintf.

[aaroncampbell]

17242-height-width.5.diff​ doesn't pass a default to get_theme_mod(), but instead uses wp_parse_args() to mesh in a set of defaults for url, thumbnail_url, width, and height.

[ryan]

How about avoiding that sprintf for non-strings/scalars?

[aaroncampbell]

17242-height-width.6.diff​ adds an is_string() check before running sprintf() on $default in get_theme_mod()

[aaroncampbell]

I talked to rboren in IRC...unfortunately it was via PM, so I can't link to it, but it ended with

sounds like id -> attachment_id and use attachment_id instead of uploaded will do.

17242-height-width.7.diff​ adds 'attachment_id' to the header array/object instead of id. It also removes 'uploaded' as part of the header image array/object, and replaces the only place it was used with a check for attachment_id instead.

[ryan]

In [19815]:

Allow flexible sizes for custom header uploads. Round 1. Props aaroncampbell, sabreuse. see #17242

[kawauso]

[19815] broke _get_random_header_data() with:

Fatal error: Call to undefined function stdClass() in \wp-includes\theme.php on line 1493

Looks like stdClass doesn't expect parentheses.

[kawauso]

Itty bitty patch

[aaroncampbell]

Actually, a class has to be instantiated. Probably return new stdClass(); or return (object) array(); to just cast an empty array.

[nacin]

In [19821]:

Return an empty object properly. see #17242.

[aaroncampbell]

Now that we allow headers of different shapes and sizes the layout isn't always great:

Using something like ​Masonry can clean this up quite a bit:

Masonry seems to work really well, but I don't know a lot about it's efficiency.

[aaroncampbell]

The newest patch (17242.masonry.diff​) uses jquery.masonry.dev.js​ and adds masonry to the header images to cause them to tile more intelligently.

[aaroncampbell]

koop took a look and noticed that I left in a comma that would mess up IE...the new patch fixes that. Otherwise he said he thinks it's a good fix.

[ryan]

License diligence: Masonry is MIT licensed. It includes smartresize and imagesLoaded, both of which are MIT. It props jcarousel and jquery UI, both of which are MIT/GPL dual.

[ryan]

In [20060]:

Introduce jQuery Masonry. Use it to arrange header thumbnails on custom header screen. Props aaroncampbell. fixes #17242

[nacin]

I have been able to reproduce an issue where, on occasion, Masonry causes headers to overlap each other. I doubt this is an issue with Masonry as much as it is something weird going on with our markup, padding, margins, etc. We should turn it off until we can figure out what is going on.

[nacin]

death-to-constants-and-lame-functions.diff​ follow-up: #20249.

[nacin]

In [20206]:

Remove jQuery Masonry from Appearance > Header until we figure out sporadic issues relating to overlapping images (probably due to lame markup). see #17242.

[nacin]

In [20212]:

Introduce new registration methods for custom headers and custom backgrounds. Backwards compatible, but old methods will be deprecated. see #20249. see #17242.

Custom header: Use add_theme_support('custom-header', $args) instead of add_custom_image_header(). Deprecates all use of constants.

• HEADER_TEXTCOLOR is now (string) 'default-text-color'.
• NO_HEADER_TEXT is nowi ! (bool) 'header-text'.
• HEADER_IMAGE_WIDTH (and _HEIGHT) are now (int) 'width' and 'height'.
• HEADER_IMAGE is now (string) 'default-image'.
• The 3.4 arguments 'suggested-width' and 'suggested-height' are now just 'width' and 'height' (they are "suggested" when flex-width and flex-height are set).
• Callback arguments for add_custom_image_header() can now be passed to add_theme_support().

Custom background: Use add_theme_support('custom-background, $args) instead of add_custom_background(). Deprecates all use of constants.

• BACKGROUND_COLOR is now (string) 'default-color'.
• BACKGROUND_IMAGE is now (string) 'default-image'.
• Callback arguments for add_custom_background() can now be passed to add_theme_support().

Inheritance: add_theme_support() arguments for custom headers and custom backgrounds is a first-one-wins situation. This is not an unusual paradigm for theming as a child theme (which is included first) overrides a parent theme.

• Once an argument is explicitly set, it cannot be overridden. You must hook in earlier and set it first.
• Any argument that is not explicitly set before WP is loaded will inherit the default value for that argument.
• It is therefore possible for a child theme to pass minimal arguments as long as the parent theme specifies others that may be necessary.
• Allows for a child theme to alter callbacks for <head> and preview (previously, calling add_custom_image_header more than once broke things).
• The just-in-time bits ensure that arguments fall back to default values, that the values of all constants are considered (such as one defined after an old add_custom_image_header call), and that all constants are defined (so as to be backwards compatible).

get_theme_support(): Introduce new second argument, which headers and backgrounds leverage to return an argument. current_theme_supports() already supported checking the truthiness of the argument.

• For example, get_theme_support( 'custom-header', 'width' ) will return the width specified during registration.
• If you had wanted the default image, use get_theme_support( 'custom-header', 'default-image' ) instead of HEADER_IMAGE.

Deprecate remove_custom_image_header(), remove_custom_background(). Use remove_theme_support('custom-header'), 'custom-background'.

Deprecate short-lived custom-header-uploads internal support; this is now (bool) 'uploads' for add_theme_support().

New 3.4 functions renamed or removed: Rename get_current_header_data() to get_custom_header(). Remove get_header_image_width() and _height() in favor of get_custom_header()->width and height.

[ryan]

In [20340]:

Remove jQuery Masonry. Fully reverts [20060]. see #17242

[mbijon]

Agree with removing Masonry so this ticket is completely closed.

Is the best practice on Trac to open a separate ticket for that part (cleaning up the header layout & debugging Masonry overlaps)? I'm happy to do that & update w/ info from here if it's no frowned upon.

[nacin]

Is the best practice on Trac to open a separate ticket for that part (cleaning up the header layout & debugging Masonry overlaps)? I'm happy to do that & update w/ info from here if it's no frowned upon.

Yes, absolutely. Thanks!

[mbijon]

Removed Masonry fix re-added here, #20346 (related ticket)