WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#17240 closed feature request (fixed)

Themes: Add ability to show a random header image

Reported by: lancewillett Owned by: ryan
Milestone: 3.2 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

Goal:

Add the option to load a different header image with each page load.

(Note: this is not intended to be a "random image rotator" that changes the image without a page load.)

Admin

If a theme has multiple headers defined with register_default_headers(), and the theme chooses to enable this feature, show a new option in checkbox in Appearance > Header that turns on the randomization.

Make the new option the first (or last) radio option in the form so that you choose not to use a single image, but to show any of them on any page or post. See attached screenshot "random-option-admin-example.png" for a visual example.

Theme Support

  1. Themes wishing to use this feature should define multiple header images with register_default_headers().
  2. Then turn it on with define( 'HEADER_IMAGE', 'random' ) to set it on by default for the theme. If set to "random" and a user visits Appearance > Header the "Show Random Image" option should be selected instead of one of the images.

When get_header_image() is called from a template, instead of returning the value in "header_image" theme_mod, the function should pick a random node from the global $_wp_default_headers variable and return its URL instead.

Reference:

  • This was originally brought up in #14151.
  • A further idea is to allow end users to rotate multiple header images that they've uploaded. See #15100 for the general idea.

Attachments (9)

random-option-admin-example.png (138.1 KB) - added by lancewillett 3 years ago.
17240-random-image.diff (2.9 KB) - added by lancewillett 3 years ago.
17240-random-2.diff (4.6 KB) - added by lancewillett 3 years ago.
Second pass
17240.diff (6.9 KB) - added by ryan 3 years ago.
Show uploaded headers
17240.2.diff (8.6 KB) - added by ryan 3 years ago.
Separate default and uploaded
17240-fix-hide-header.diff (1.3 KB) - added by lancewillett 3 years ago.
17240-fix-default-is_random_header_image.diff (667 bytes) - added by kawauso 3 years ago.
Allow for $type 'default' when no header is chosen
random-option-above.png (232.8 KB) - added by lancewillett 3 years ago.
17240-random-input-fix.diff (3.5 KB) - added by lancewillett 3 years ago.

Download all attachments as: .zip

Change History (43)

comment:1 iandstewart3 years ago

  • Cc ian@… added

comment:2 batmoo3 years ago

  • Cc batmoo@… added

comment:3 lancewillett3 years ago

As Nacin mentioned over on #17242:

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.

This could be changed to use that technique also:

add_theme_support( 'random-header-image', array( 'on' => true ) ); where it'd be off by default.

comment:4 follow-up: aaroncampbell3 years ago

I'd rather see the ability to upload several images and then just choose "random" in the ui. I don't particularly see why a theme should choose whether it supports random header images...that seems more like a user choice (as long as all the header images WORK with the theme, why should the theme care if they change on each page load).

comment:5 matveb3 years ago

  • Cc mv@… added

comment:6 in reply to: ↑ 4 lancewillett3 years ago

Replying to aaroncampbell:

I don't particularly see why a theme should choose whether it supports random header images...that seems more like a user choice.

Good point. So you're saying don't add any theme support, but update core functionality to add the random option if multiple images exist?

That makes sense, and it's good for backwards compatibility.

comment:7 lancewillett3 years ago

  • Cc lance@… added
  • Keywords has-patch dev-feedback added; needs-patch removed

Attached patch implements this for images registered with a theme (Twenty Ten, for example).

Ready for testing.

comment:8 ryan3 years ago

  • Milestone changed from Awaiting Review to 3.2

lancewillett3 years ago

Second pass

comment:9 lancewillett3 years ago

Second pass at patch introduces two new functions:

get_random_header_image()

and

is_random_header_image()

  • Check if random header image is in use.
  • Always true if user expressly chooses the option in Appearance > Header.
  • Also true if theme has multiple header images registered and no specific header image is chosen.

Theme does not have to explicitly add support for this feature—all that's needed an array of registered images.

In Appearance > Header form, show the random image option if this theme has multiple header images. The option is checked by default if no header has been set.

comment:10 ryan3 years ago

Looks good so far in my testing.

comment:11 follow-up: nacin3 years ago

Patch looks good. But I don't really like this as a core feature currently. If I want to have random header images, then chances are I'll want to upload a series, rather than cycle through the ones registered in code.

I think we need better integration with the media library before doing something that feels, personally, half-baked.

comment:12 in reply to: ↑ 11 lancewillett3 years ago

Replying to nacin:

Patch looks good. But I don't really like this as a core feature currently. If I want to have random header images, then chances are I'll want to upload a series, rather than cycle through the ones registered in code.

I agree, but we have to start somewhere. The rotation of the default headers is a big improvement, in my opinion. It's better than nothing and paves the way for a better solution later.

There is an open ticket for allowing multiple uploaded header images, see #15100. I'd absolutely love to get that in 3.2 but realize we're very close to the freeze, so that's why this ticket is split out on its own.

ryan3 years ago

Show uploaded headers

comment:13 ryan3 years ago

Patch shows uploaded headers in the header list. Uploaded headers have a post meta key named _wp_attachment_is_custom_header that isset to the stylesheet of the current theme. Currently uploaded and default headers are shown in the existing header selector and are part of the same pool when a random image is selected. I'm doubting someone who was uploaded headers will want to see the default headers in the random mix. I'm thinking we'll want to have separate header selector lists in the UI for the default and uploaded headers.

comment:14 jane3 years ago

Or instead of a Random: show random image radio button with each set (as I imagine Ryan's suggestion to work?), have option for Randomize Header Images: [] default header images only [] my uploaded header images only [] all header images

That's my UI suggestion. I semi-agree with Nacin, that maybe putting this in core right now is rushing it? Trying it out in the theme first is sort of the equivalent of trying it in a plugin, right?

comment:15 ryan3 years ago

Having it in the radio is convenient though. We don't have to do anything extra to clear the selection.

ryan3 years ago

Separate default and uploaded

comment:16 ryan3 years ago

That separates the UI and the random pools for default and uploaded.

comment:17 ryan3 years ago

Needs some styling and markup validation work but otherwise pretty much complete.

comment:18 ryan3 years ago

Also needs caching for the attachment query.

comment:19 ryan3 years ago

(In [17757]) Allow selecting previously uploader headers and randomly serving previously uploaded or default headers. Props lancewillett. see #17240

comment:20 lancewillett3 years ago

Looking good! One issue I found in testing:

If I upload an image that needs a crop, it works beautifully and shows in the Uploaded list.

But, if I upload an image at the exact dimensions, it doesn't add the meta value and doesn't show in the list.

comment:21 ryan3 years ago

(In [17758]) Set meta when upload matches dimensions. see #17240

comment:22 lancewillett3 years ago

When a theme has multiple "default" images registered, and the HEADER_IMAGE definition is empty, we turn on randomness by default. That's fine, but in that case there is no way to remove the header image entirely.

Maybe the "Remove Header" action should not set the header_image theme mod to be empty but instead make it explicitly "remove-header" or something similar. That way get_header_image() knows to hide all header images from display.

Patch attached.

comment:23 kawauso3 years ago

is_random_header_image() will return true when no header image is selected, but this is conditional on $type being 'any'. The checked() call for the Random radio button on the Header screen uses 'default', so no radio buttons appear checked. Following patch adds a clause to catch this.

Also, shouldn't the default-header class and input name in show_header_selector() be called something more logical since they're also used for user uploaded headers?

Last edited 3 years ago by kawauso (previous) (diff)

kawauso3 years ago

Allow for $type 'default' when no header is chosen

comment:24 ryan3 years ago

(In [17770]) Fix hiding of header image. Fix radio selection when falling back to randomized default headers. Props lancewillett, kawauso. see #17240

comment:25 ryan3 years ago

lancewillet, twentyeleven_admin_header_image() emits an img tag with empty src when the header is removed. This results in the broken image icon being displayed.

comment:26 ryan3 years ago

There are two divs with the available-headers id. Perhaps that should become a class.

The random radios need to have the vertical alignment fixed.

comment:27 lancewillett3 years ago

@ryan I can look at both those issues shortly.

comment:28 lancewillett3 years ago

The vertical alignment is super-tricky with this layout because it requires knowing the height of the thumbnail images. A better solution might be to put it above the thumbnails on its own line. That avoids it needing to line up visually with the thumbnails. (See attached screenshot "random-option-above.png" for an example.)

Patch has this random option fix, plus some text updates and fix for a missing end paragraph tag. Patch also changes "available-headers" to be a class attribute instead of ID.

comment:29 lancewillett3 years ago

[17773] fixes the twentyeleven_admin_header_image() empty img src issue.

comment:30 ryan3 years ago

In [17773]:

Remove h3s. see #17240

comment:31 lancewillett3 years ago

In [17775]:

Twenty Eleven: improve calls to get_header_image() - see #17240 and #17198

comment:32 scribu3 years ago

Related: #17291

comment:33 lancewillett3 years ago

  • Keywords dev-feedback removed

@ryan A reminder that I added some minor fixes with 17240-random-input-fix.diff -- patch is ready for review and testing.

comment:34 ryan3 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [17814]:

Custom header admin UI fixes. Props lancewillett. fixes #17240

Note: See TracTickets for help on using tickets.