Improve the user and developer experience for new custom header functions

[bradyvercher]

The subject is a bit open-ended, but the custom header functionality currently feels a bit glitchy. As I've dug into the code over the past week, I've uncovered a few bugs that were present before video headers were introduced and have opened tickets for those. They make it difficult to really test the experience in the Customizer.

I'm going to try to lay out a few concrete things that can be done to improve the new custom header functions that should hopefully create a much smoother experience for users in the Customizer as well as make it easer for theme authors to implement support in themes, especially as the pre-existing bugs are addressed.

Selective Refresh

A Customizer partial is registered by default and uses the_custom_header_markup() for the render callback, which allows both header images and videos to automatically support selective refresh. If a header image or video hasn't been set, we don't want to output empty markup on the front end, however, the container div needs to exist in the Customizer for selective refresh to work. Otherwise, setting an image or video will result in a full refresh.

the_custom_logo(), along with other features in core, output placeholders in the Customizer preview for the optimal experience, so making sure the container is always available in the preview isn't really breaking new ground. Themes can always use .wp-custom-header:empty { display: none;} to hide it if it causes styling issues in the preview.

Header Image Settings

It may also be worth changing the transport for the header_image and header_image_data settings to postMessage automatically when the the theme registers support for videos. Otherwise, it's kind of a weird experience when videos use selective refresh and images don't.

Front Page Restriction

I've been looking to incorporate this functionality in the ​Marquee theme and it feels broken if video only loads on the front page. This seems like a restriction that should be left up to the theme and is particularly difficult to workaround if a theme wants video on more than one page.

Browsers should be caching local videos, which should minimize the bandwidth impact a bit. I think we should make the API broadly useful and document best practices instead of trying to enforce onerous restrictions.

[bradyvercher]

In 38639.diff​:

• Removes the is_front_page() conditional in has_header_video()
• Adds a show_video argument to the_custom_header_markup() that defaults to only displaying video on the front page.
• Updates the get_customer_header_markup() to always output the container div in the Customizer for selective refresh support.
• Updates the Twenty Seventeen and Twenty Fourteen implementations.

I've also added a bunch of "unit" tests.

• The changes in _remove_theme_support() were required, otherwise all sorts of notices would be thrown, causing the tests to fail.
• #38633 and #38557 are required for the tests to pass.
• Calling add_theme_support( 'custom-header' ) in the tests should be avoided since it sets a number of constants, which would poison subsequent tests. The _add_theme_support() method isn't ideal, but it's the only way I could think of to get around the constants.
• The dummy video for the tests came from ​this repo and is public domain.

[celloexpressions]

+1 to automatically turning on selective refresh for images when video support is added. Since themes have to explicitly add video support, we can ask them to also ensure that they're either using the_custom_header_markup(), adjusting the custom header partial to reflect their custom output, or turning the transport back to refresh on those settings.

[bradyvercher]

In 38639.1.diff​:

• Set the transport for header_image and header_image_data to postMessage when support for video headers is registered
• Remove unnecessary tests for #38557

[peterwilsoncc]

I think this is best split into two tickets: 1) selective refresh; 2) removing the front page limitation.

The architectural and design decisions around one ought not block the other.

[peterwilsoncc]

Closing in favour of #38737 (selective refresh) and #38738 (header restriction).

[joemcgill]

Thanks @peterwilsoncc I agree that these are separate tickets. @bradyvercher, could you split the appropriate parts of 38639.1.diff​ to their related new tickets? If the unit tests don't go cleanly with either, feel free to add another ticket for adding unit tests.

[bradyvercher]

I'd like to see these changes through and help make sure video headers provide a good experience, but the various pieces here are intertwined and splitting them up at this point is going to take a good chunk of time. Getting to the bottom of numerous issues with custom headers and juggling the various tickets has already taken much more time than I anticipated.

There's been little discussion on this ticket and no opinions offered on the new ones. I realize everyone is busy, but I'd like to see some decisions made before moving forward with these changes.

[westonruter]

In 39227:

Customize: Use selective refresh for custom header changes when possible.

• Use postMessage transport for header image settings by default when video headers are supported, and thus the_custom_header_markup() will necessarily be used (and thus a selective refresh partial will be available).
• Ensure that the_custom_header_markup() always outputs a container element in the customizer preview even if the header is empty.
• Ensure that edit shortcut appearing for custom header does not get positioned off-screen.

Props bradyvercher, westonruter.
See #38639.
Fixes #38737.

[obenland]

In 40825:

Themes: More unit tests for Custom Header

Custom Header functionality is largely untested. This adds tests for existing
behavior as well as changes introduced in [39227].

Props bradyvercher.
See #38639.
Fixes #39241.