WordPress.org

Make WordPress Core

Opened 12 months ago

Closed 11 months ago

Last modified 9 months ago

#38172 closed task (blessed) (fixed)

Enable Video Headers in Custom Headers

Reported by: davidakennedy Owned by: joemcgill
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: Themes Keywords: has-patch ui-feedback ux-feedback dev-feedback has-screenshots
Focuses: ui, accessibility, javascript Cc:

Description

Atmospheric videos on your site can be pretty dang cool, especially with the rise of more video content. Allowing video headers to be used, in addition to header images, would give users and themers more ways to make awesome sites with WordPress.

Some other site-building platforms have recently implemented video headers:

http://blog.squarespace.com/blog/video-backgrounds
https://www.weebly.com/4

Some prominent examples in the wild:

http://www.nytimes.com/projects/2012/snow-fall/#/?part=tunnel-creek
http://www.nytimes.com/interactive/2015/08/26/us/ten-years-after-katrina.html

This would be a great addition for WordPress users and is needed to fulfill the vision of Twenty Seventeen.

A few things to think about:

  • Handling the fallback for mobile devices and/or browsers that do not support the video tag.
  • File upload sizes: how should this be communicated to users?
  • Sizing, as in width and height. How should this be handled?
  • Accessibility: What are the best "default" options to give a good experience to everyone while pulling off the feature elegantly.
  • Should there be options for the user to control the video in different ways, like, speed, etc.?
  • Can video links/URLs be allowed as well as uploads, like Youtube, Vimeo, etc.

Attachments (24)

38172.1.patch (6.5 KB) - added by davidakennedy 12 months ago.
Basic implementation of video headers within add_theme_support( 'custom-header'. Patch by myself, @rosso99 @jonahbraun.
38172.2.patch (3.8 KB) - added by davidakennedy 11 months ago.
Revised patch for the theme API part of this. Worked on this along with @joshcummingsdesign
38172.3.diff (12.8 KB) - added by celloexpressions 11 months ago.
Add customizer UI, support for Twenty Fourteen, and theme API adjustments.
38172.4.diff (16.1 KB) - added by bradyvercher 11 months ago.
38172.diff (15.0 KB) - added by celloexpressions 11 months ago.
Add validation to prevent videos over 8MB from being used, add initial support to Twenty Seventeen (needs front end work).
38172.6.diff (22.2 KB) - added by laurelfulford 11 months ago.
Update theme styles and markup for proper video placement.
38172.7.diff (26.8 KB) - added by celloexpressions 11 months ago.
Customizer proof of concept for external support, with JS approach.
38172.js.gif (8.0 MB) - added by celloexpressions 11 months ago.
Testing with 38172.7.diff, note poor image quality due to recording this as a gif.
38172.8.diff (27.5 KB) - added by celloexpressions 11 months ago.
Additional validation, refresh for Twenty Seventeen structure change.
38172.9.diff (28.7 KB) - added by bradyvercher 11 months ago.
38172.10.diff (28.4 KB) - added by celloexpressions 11 months ago.
38172.11.diff (25.6 KB) - added by joemcgill 11 months ago.
38172.12.diff (28.9 KB) - added by bradyvercher 11 months ago.
38172.13.diff (29.9 KB) - added by joemcgill 11 months ago.
38172.14.diff (30.4 KB) - added by joemcgill 11 months ago.
38172.15.diff (30.4 KB) - added by joemcgill 11 months ago.
38172.16.diff (30.5 KB) - added by joemcgill 11 months ago.
38172.17.diff (30.5 KB) - added by joemcgill 11 months ago.
38172.18.diff (32.8 KB) - added by joemcgill 11 months ago.
Add postMessage support for themes
38172.19.diff (33.0 KB) - added by joemcgill 11 months ago.
38172.20.diff (33.1 KB) - added by joemcgill 11 months ago.
38172.21.diff (32.9 KB) - added by joemcgill 11 months ago.
38172.22.diff (478 bytes) - added by laurelfulford 11 months ago.
38172.23.diff (616 bytes) - added by bradyvercher 11 months ago.

Change History (112)

@davidakennedy
12 months ago

Basic implementation of video headers within add_theme_support( 'custom-header'. Patch by myself, @rosso99 @jonahbraun.

This ticket was mentioned in Slack in #core-themes by davidakennedy. View the logs.


12 months ago

#2 follow-up: @obenland
12 months ago

  • Version set to trunk

Looks like a decent first pass, we should talk about the API and function naming though.

In the current form it will not be backwards compatible with older themes but displays the video UI in the Customizer.
Could you add an example implementation that a theme would use? How are things like height/width enforced?

In terms of feedback about the patch: We shouldn't introduce new constants for jit theme support, it's a new feature. I saw a few formatting issues but those are minor. Fleshing out the API will take a bit more work.

#3 @celloexpressions
12 months ago

  • Keywords needs-patch added

Here are a bunch of initial thoughts on the patch (we'll need a lot more work here):

In terms of naming, the section should probably become either "Header" or "Custom Header," see the discussions around naming "Custom Logo" for history.

We should absolutely use the core video functionality, wp_video_shortcode(), rather than building video tags manually. We can add any additional parameters that this feature requires to the core function. See also the code I shared previously that uses this: https://gist.github.com/celloexpressions/77aa54eb7020aa921ec1d3a3ce29696e.

Using the core video functionality brings MediaElement.js, which is bundled with core, adding cross-browser support for any (generally) video format that was uploaded and fallbacks for browsers that don't support <video> at all.

MediaElement can skin external videos from Youtube or Vimeo; however, in my limited testing, autoplay was not possible. There may be ways to hack around it, but it's probably best to focus on local uploads and education around video file sizes.

We'll probably need to link off to a page on the codex or a handbook to explain video encoding in more detail, with filesize, bitrate, etc. as considerations and suggested editing software. In probably almost all cases, users will have to go through other software to get their video from where they shot it to being able to upload it. Many sites also have upload size limitations ranging from 1MB to 50MB; we'll need better educational resources around how that can (sometimes) be changed and why files needs to be kept small anyway so that visitors will see them.

Most themes should be able to add something like the_header_video(); to display the video. However, we should look at the interaction with header images more closely. Most likely, if there's a video, it'll be displayed in place of the header image, and there should be a function that wraps both and shows what's there automatically for themes. If we want to always show the headers UI even when there's a video, let's document that decision and the reasons, and potentially add help text to the header image control explaining that it's only shown when there isn't a video, if the theme supports video.

Is there a reason that there should be separate parameters for image size vs. video size? The API would be cleaner if we restricted that both should match. It will be extremely important for themes to exercise extreme caution with their recommended video sizes since that'll have the biggest impact on file size and the ability for users to even upload larger files on their server, so that may be one reason.

This should support selective refresh out of the box, by wrapping the video output in a div that core can target with a partial. That'll also provide support for cross-linking between the preview and the customize pane. postMessage isn't worth the effort for something like this since it essentially requires a partial refresh but without the Ajax call.

The theme support should probably be two things: a boolean video parameter that defaults to false, and the default fallback. We'll need major restrictions on file size from the TRT if we allow default videos to be bundled, as they will drastically increase the size of theme zips and slow down installs. Even 1MB of video may be too much. And external hotlinked defaults are a bad idea.

#4 @bradyvercher
12 months ago

Regarding MediaElement.js, I'm not sure it's absolutely necessary for this use-case. Fallback support is provided through Flash, which is being phased out of the major browsers and isn't supported on iOS at all. Much of MediaElement.js is focused on generating a consistent interface across browsers, which isn't needed since these are background videos without controls. If a browser doesn't support HTML5 video at this point, I would think a fallback image should be acceptable for this type of feature.

It would be nice to know what the most uploaded video formats are, but almost all browsers support MPEG-4/H.264. Other formats could be specified using the <source> tag if necessary.

As for third-party providers, I'm pretty sure Vimeo support in MediaElement.js doesn't work in the current version. The YouTube plugin really just injects an iframe and maps events between the interface and the iframe using the YouTube JavaScript API. Embedding an iframe directly using the player parameters would achieve the same result, with much lower overhead, but I imagine the video/logo would need to remain clickable to not violate their TOS.

#5 @celloexpressions
12 months ago

We need to use the core video functions for this; if we want to switch those to using something other than MediaElement.js (even such as raw video tags, when the time comes), that would be a different discussion.

I vaguely remember there actually being a higher-level function than wp_video_shortcode() that also handles additional source formats being uploaded, although we can probably recommend uploading .mp4/H.264 given the current browser support for that.

I also realized we could consider using the header image as the poster attribute if we use that as an automatic fallback and match the sizes.

This ticket was mentioned in Slack in #core by davidakennedy. View the logs.


12 months ago

#7 @bradyvercher
12 months ago

I disagree that the core function for rendering a video shortcode needs to be used to display a header video. They're two different use cases. One is content and the other is for appearance.

Based on my experience, MediaElement.js generates a bunch of extra markup and overly specific styles that themes would need to account for and it'll needlessly bloat the header area. It will also likely introduce compatibility issues with plugins that extend or even replace the core shortcode with another library. Consider sites that add a pre-roll video or track stats for video shortcodes -- there's no reason that functionality needs to be enabled for a header video. As far as performance is concerned, the video shortcode method requires at least 4 extra requests and 100KB in scripts and styles, plus however many other requests plugins might generate.

For additional sources, there is a media workflow that allows for defining alternative formats and setting a poster image that could be useful here.

#8 in reply to: ↑ 2 @davidakennedy
12 months ago

Replying to obenland:

Thanks for taking a look!

Looks like a decent first pass, we should talk about the API and function naming though.

Yeah, I know you mentioned during some in some previous feedback that we should consider writing an abstraction layer with no function names not reliant on image or video naming, right? I didn't worry about that with this because just thinking about how the functions would be the same or different for videos helped me learn and process everything. It should be simpler though.

In the current form it will not be backwards compatible with older themes but displays the video UI in the Customizer.
Could you add an example implementation that a theme would use? How are things like height/width enforced?

Right – do you have any suggestions for how best to handle back compat? I have an experimental branch of Twenty Seventeen going for this: https://github.com/davidakennedy/twentyseventeen/tree/vheader

And: https://github.com/davidakennedy/twentyseventeen/blob/vheader/inc/custom-header.php

Calling the function here:

https://github.com/davidakennedy/twentyseventeen/blob/vheader/components/header/header-image.php

In terms of feedback about the patch: We shouldn't introduce new constants for jit theme support, it's a new feature. I saw a few formatting issues but those are minor. Fleshing out the API will take a bit more work.

I can fix this.

Last edited 12 months ago by davidakennedy (previous) (diff)

#9 follow-up: @davidakennedy
12 months ago

@celloexpressions and @bradyvercher

I didn't think of using wp_video_shortcode() although Brady has some good points about how themes and plugins modify it or would need to modify it.

I went at this thinking a simple starting point was to use the HTML5 <video> tag as is and just fallback to an image for anything that doesn't support it. It seems reasonable, very future friendly and less complex. The idea of easily skinning MediaElement.js is nice though. I do think controls will be needed for accessibility reasons.

Explaining video encoding to users is frightening to me. :) I'm wondering if we can say it supports .mp4 and that's it.

Regarding the tie-in with header images – I imagined some kind of hierarchy. If video is there, it shows first, otherwise the image, etc. And I did think using header images would be smart for the poster and potentially for themers to wrap inside the video tag for a fallback too. I liked the idea of giving themers the file and letting them implement how they want to.

Also, I'm not sure we need height and width even though I put it in. We probably should tie it into the content width in some way... I need to do more research there.

#10 follow-up: @celloexpressions
12 months ago

I think the fundamental idea is that a higher-level feature like theme support for video should leverage lower-level core functionality rather than implementing things separately. If we want to add a way for wp_video_shortcode to skip the MediaElement stuff, that would work, but building the video tag here feels wrong and fragments the core APIs for video.

For what it's worth, I'm fairly neutral regarding whether or not we need MediaElement.js here. The browser compatibility aspects are nice, but the cross-browser consistency and the ability to skin (even to control-less youtube/vimeo) is a more compelling reason to use it. New core functionality should use existing core functionality rather than duplicating it.

#11 @bradyvercher
12 months ago

My understanding was that this feature was going to be limited to relatively short, looping background videos without controls -- a moving header image if you will -- which is why I felt MediaElement.js was overkill. Videos are hefty enough, so I think we should be optimizing the output as much as possible instead of further hurting performance on sites that use this feature.

That being said, as long as themes are provided with the source files to generate their own markup, I'm not too concerned about the default implementation.

Replying to davidakennedy:

The idea of easily skinning MediaElement.js is nice though.

I'm pretty sure the words "easily skinning MediaElement.js" don't belong together in the same sentence 😉

Replying to celloexpressions:

If we want to add a way for wp_video_shortcode to skip the MediaElement stuff, that would work, but building the video tag here feels wrong and fragments the core APIs for video.

I guess we just disagree on what constitutes a core API -- calling a method with a "shortcode" suffix and shortcode-specific parameters when shortcodes aren't involved feels wrong from an API standpoint. I don't think it's a showstopper by any means, though. If it has to be used and MediaElement.js isn't going to be loaded, there is a filter that would allow for disabling it.

#12 in reply to: ↑ 9 ; follow-up: @azaozz
12 months ago

Replying to celloexpressions:

I think the fundamental idea is that a higher-level feature like theme support for video should leverage lower-level core functionality rather than implementing things separately. If we want to add a way for wp_video_shortcode to skip the MediaElement stuff, that would work, but building the video tag here feels wrong and fragments the core APIs for video.

I agree in principle, however not sure we can use wp_video_shortcode() in this case. Problems are that it is a specific "high-level" function only for use with a shortcode. It does several things at the same time. It is intended to run in post_content, is intended to get arbitrary input from the users (through the shortcode), adds MediaElement.js with specific markup, etc. The first thing it does is to limit the video width to $content_width which is wrong for a header :)

We can try to abstract some parts of it in "low level" functions, but not sure what that will be like. At first look it seems there is not much that can be used in the current patch here and can be abstracted.

Replying to davidakennedy:

I went at this thinking a simple starting point was to use the HTML5 <video> tag as is and just fallback to an image for anything that doesn't support it. It seems reasonable, very future friendly and less complex. The idea of easily skinning MediaElement.js is nice though. I do think controls will be needed for accessibility reasons.

Explaining video encoding to users is frightening to me. :) I'm wondering if we can say it supports .mp4 and that's it.

Not sure what's best here either. Using Flash for a video player is outdated. On the other hand that means we will be supporting only a particular video format. Then should have nice help/explanations that only one video format can be uploaded for use in the headers. That implies the users will have to be able to convert videos they've shot, etc.

Agree that if we can get "skinless" MediaElement.js, we should try it. However relying on YouTube or Vimeo seems bad as they can change the way they handle requests at any time, and "break" every site that uses this. The only possible integration with an external service seems to be with VideoPress as we can make sure this use is supported there (but that is probably a plugin material).

Last edited 12 months ago by azaozz (previous) (diff)

#13 @davidakennedy
12 months ago

Regarding the controls, I talked to the accessibility team today to get a better idea of what an accessible, default implementation looks like for this feature out of the box. See: https://wordpress.slack.com/archives/accessibility/p1475520735001672

To summarize:

  1. Autoplay is okay for decorative videos.
  2. Controls should be there (so a user can pause/stop it if the motion is problematic).
  3. No sound (there was an idea if this was a Core feature that we should not have an option to turn sound on).

This was what I was thinking, should blend nicely with the type of experience imagined for this feature.

#14 in reply to: ↑ 12 @davidakennedy
12 months ago

Replying to azaozz:

Yeah, I'd like to keep the implementation as simple as possible. I'm starting to lean toward:

  1. Default video tag, no MediaElement.js. 2. A function that outputs the video tag, similar to get_custom_logo, with certain parameters that could be modified by themers. 3. Robust fallback support via header image, etc.

Replying to bradyvercher :

I would take your word on that. :) I like the idea of being very future friendly here and not requiring users to download more JavaScript for a feature, if possible.

This ticket was mentioned in Slack in #core-themes by davidakennedy. View the logs.


12 months ago

#16 @pento
12 months ago

Data-driven decisions are fun. :-)

Here's a list of the extensions from the last 10,000 uploads to VideoPress.

ExtensionUpload Count
mp46511
mov2890
m4v323
wmv93
avi72
mpg42
3gp38
peg17
gpp10
ogv4

It's a reasonable assumption that most (if not all) mp4 and m4v videos are using h.264 encoding, this is the primary (often only) container and encoding supported by consumer video recording devices, and video editing software. If a person knows enough to export in a different format, we can assume they know enough to use h.264 for web video.

mov files are not allowed to be uploaded to WordPress, by default. It's also not a reasonable assumption that they're using h.264. It's quite common for it to be a container for MPEG-2 encoded video, particularly video coming from hand-held video cameras more than a few years old. mov support sometimes comes up in the support forums, where the general advice is to convert it to mp4.

The final ~3% are a mixed bunch, none of which will reliably work in any browser, and wmv/ogv are the only ones that can be uploaded to WordPress.

So, my recommendations:

  • When the video is mp4 or m4v, use a plain video tag.
  • When the video is any other format, load mejs as a fallback. We can't guarantee it'll work, but it'll usually provide a better experience than not loading it, and it's no worse than WordPress current behaviour for embedded video.
  • I wouldn't mind YouTube support, it can be done quite nicely using the YouTube API.
  • Vimeo only works for videos upload by Pro members, so I'm inclined to not add Vimeo support.

This ticket was mentioned in Slack in #core by davidakennedy. View the logs.


12 months ago

#18 @bradyvercher
12 months ago

Thanks for the data @pento! For what it's worth, MediaElement.js appears to be planning on dropping support for wmv in the next release.

Replying to @davidakennedy

Yeah, I'd like to keep the implementation as simple as possible. I'm starting to lean toward:

  1. Default video tag, no MediaElement.js. 2. A function that outputs the video tag, similar to get_custom_logo, with certain parameters that could be modified by themers. 3. Robust fallback support via header image, etc.

I like this approach. Regarding controls, if play/pause is all that's needed, a button could be wired up using the native API.

#19 @davidakennedy
12 months ago

Thanks to everyone for the feedback so far!

To help everyone visualize this feature, here is a screencast of the current patch, 38172.1.patch: https://cloudup.com/ctFWMV-hoFF That's in an early version of Twenty Seventeen.

To focus the feature, and provide a scope that could be accomplished within the next week so it can have a chance to make it into 4.7, this is what I'd like to see for the next patch iteration:

  1. Output video tag, no MediaElement.js unless the format is something other than mp4 or m4v.
  2. A function that outputs the video tag, similar to get_custom_logo, with certain parameters that could be modified by themers. Basic controls should be included, and sound should be muted.
  3. Robust fallback support and integration with header image.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


12 months ago

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


12 months ago

This ticket was mentioned in Slack in #core-themes by davidakennedy. View the logs.


11 months ago

#23 @celloexpressions
11 months ago

  • Milestone changed from Awaiting Review to 4.7

We need to make sure we're tracking this with everything else in 4.7 given our target timeline.

@davidakennedy
11 months ago

Revised patch for the theme API part of this. Worked on this along with @joshcummingsdesign

#24 @davidakennedy
11 months ago

attachment:38172.2.patch revises the theme API part of the original patch:

  • Adds video param in custom header that defaults to false. To enable suppoet, themes would set it to true. It can be used to change the UI in the Customizer, based on that support too.
  • Uses custom header height and width instead of creating separate ones for videos.
  • Adds a the_custom_header() function which spits out markup for the custom header, giving the video tag precedence.
  • Uses header image as a poster attribute for <video> tag as a fallback when header image is available.

Thanks to @joshcummingsdesign for working along with me to create a better patch. @celloexpressions Does this give you enough to work off of for the Customizer part?

Things to think about still:

  • Should other fallbacks exist, like the MediaElement.js one that @pento suggested? I think so.
  • Should the <video> tag include anything else for browsers that don't support it. Or should that be left up to themers?

#25 @obenland
11 months ago

It would be helpful to keep everything in one patch rather than having to apply multiple patches that are potentially conflicting.

The API still seems not to be as solid as it probably should be at this point in the cycle. It's not backwards compatible for themes that already support custom headers (which personally I think is a blocker for making it a core feature), and as far as I can tell the customizer control gets displayed regardless of the theme supporting videos or not?

Posting an example implementation would really go a long way in understanding what the API should cover, currently there seems to be a myriad of ways for themes to interact with it, which might not make for the best dev experience.

Having videos taking precedent over images is something we probably should explain in the customizer control as well. And inform them how they can use that as a fallback mechanism.

This ticket was mentioned in Slack in #core by davidakennedy. View the logs.


11 months ago

#27 follow-up: @celloexpressions
11 months ago

@obenland when I add the customizer pieces into the more updated patch tomorrow, hopefully those pieces will become clearer, and I can also document the "typical" theme implementation here. I don't think this should be on by default in header images (due to likely-odd aspect ratio problems for many themes), so switching to a different template tag along with adding support in the theme support call wouldn't be too difficult. However, we should look more closely at how this integrates with the existing header template functions.

We should add support for Twenty Fourteen with the core patch to show the implementation and facilitate testing (video should work well with the way headers work in that theme). I don't think videos make sense for Ten, Eleven, Twelve, Thirteen, or Fifteen; I'm not familiar enough with Sixteen to say there.

#28 follow-up: @davidakennedy
11 months ago

@obenland Thanks for the continued feedback!

It would be helpful to keep everything in one patch rather than having to apply multiple patches that are potentially conflicting.

The idea was my patch would be something @celloexpressions could pick up on and use. We talked about this in a recent features meeting for Twenty Seventeen. I thought it would be more valuable to post a in-progress patch in public since that means other people can provide feedback, even if it was not complete. It's better to share work early than not, even if it's not fully fleshed out yet.

The API still seems not to be as solid as it probably should be at this point in the cycle. It's not backwards compatible for themes that already support custom headers (which personally I think is a blocker for making it a core feature), and as far as I can tell the customizer control gets displayed regardless of the theme supporting videos or not?

I realize that it still needs work, and the idea is for it to be backward compatible of course. I know we talked about some ideas in person recently for the theme API part of this feature, but could you lay out some more specific feedback? That would help immensely here. What would make it more "solid" in your mind, regardless of where we are in the cycle?

Having videos taking precedent over images is something we probably should explain in the customizer control as well. And inform them how they can use that as a fallback mechanism.

Agreed.

#29 @helen
11 months ago

  • Type changed from feature request to task (blessed)

@celloexpressions
11 months ago

Add customizer UI, support for Twenty Fourteen, and theme API adjustments.

#30 @celloexpressions
11 months ago

  • Keywords has-patch added; needs-patch removed

38172.3.diff adds the customizer UI, updated header images as needed. The header image is now used as a fallback if the browser can't play the video. The patch also adds support for video headers to Twenty Fourteen, which will facilitate testing (and that theme's use of headers lends itself to video).

Unfortunately it doesn't look like we can easily add selective refresh to header images in a backwards-compatible way. The new video functions will selectively refresh, though. This is important for visibile edit icons (#27403).

The implementation in Twenty Fourteen is a bit complex because there are a few issues and I wanted to minimize the markup changes there. The sticky menu code there gets a bit messy if the fallback image is a different size than the video.

New themes should use the_custom_header() to handle displaying both the video and the image as needed, with selective refresh for video. A lot of themes are adding header images as background images currently - when adding support for video and in new themes, equivalent styling should be created as needed based on the markup that the_custom_header() outputs.

#31 @mikeschroder
11 months ago

  • Type changed from task (blessed) to feature request

@davidakennedy asked me to take a look at this.

I'll note that I'm not a big fan of video headers, and find them very distracting (like Sliders), rather than helpful, but it looks like I'm in the minority opinion there.

On the code/method:
I defer to @obenland on his notes RE: the theme API, as it's not my area of expertise.

In general, the code seems fairly straight-forward and easy to follow.

I agree with a few others here that this really should use existing core infrastructure to generate the video markup (including MEjs), and abstracting the parts needed if current API does too much for this purpose.

It's been difficult/frustrating to maintain the other media related theme bits that use separate interfaces/generation and don't use the common media infrastructure (see current custom headers, and anything that crops for theme support, for example).

I'd hate to see this become another example of that. :)

#32 @mikeschroder
11 months ago

  • Type changed from feature request to task (blessed)

Whoops, apologies -- that was not intentionally changed. Back to task (blessed).

#33 @celloexpressions
11 months ago

+1 for avoiding things that need to be separately maintained. Custom headers are a nightmare in terms of the customizer as well. That being said, the current implementation is probably simple enough that it'll be okay. If we add any additional fallbacks or want a way to let themes uniformly style the controls, we should switch to the existing functionality, which can be abstracted from wp_video_shortcode() to a generic video function used in both places.

For selective refresh, I think the best approach for adding it to header images is to add partials where they can be added in core, and then rely on themes switching transport to postMessage on the header image setting. That way they'll always get edit icons when not using it as a background image, and themes can add the better selective refreshing with one line of code.

#34 in reply to: ↑ 10 ; follow-up: @aaroncampbell
11 months ago

After reviewing the patch, my first thoughts were exactly the same as @mikeschroder. However, after reading through all the comments on here (I was largely convinced by @pento's data), I think the simple plain video tag is fine with the image fallback.

There is one typo that I saw, just in a comment:

If there was a video an it was removed, fall back to an image.

Should be:

If there was a video and it was removed, fall back to an image.

#35 in reply to: ↑ 27 @obenland
11 months ago

Replying to celloexpressions:

I don't think this should be on by default in header images (due to likely-odd aspect ratio problems for many themes)

Based on your example implementation for Twenty Fourteen it looks like we can't get around changes in style.css? I think it would be worth exploring if inlining height: auto; would make it work for themes with flexible height.

I don't think videos make sense for Ten, Eleven, Twelve, Thirteen, or Fifteen;

Because they have elements preceding the header? I don't think that's an issue, if a default theme has a flexible height header, it should offer support for video headers.

#36 in reply to: ↑ 28 @obenland
11 months ago

Replying to davidakennedy:

I know we talked about some ideas in person recently for the theme API part of this feature, but could you lay out some more specific feedback? That would help immensely here. What would make it more "solid" in your mind, regardless of where we are in the cycle?

@celloexpressions' patch helped quite a bit. get_header_video_url() is still a little wonky though. We don't need to check twice if $id is set, so the latter could probably be all one line. Do we need to absint() in there too? Shouldn't that be done as sanitization when saving a video? The doc block for that function needs some indentation too.

In get_header_video_tag() and the_header_video_tag() the available arguments in $attr need documentation (and while at it, pluralize the variable perhaps?). The is_customize_preview() and ! empty( $image ) checks can be simplified.
Can we add a class in the_header_video_tag() for styling?

#37 in reply to: ↑ 34 @mikeschroder
11 months ago

Replying to aaroncampbell:

After reviewing the patch, my first thoughts were exactly the same as @mikeschroder. However, after reading through all the comments on here (I was largely convinced by @pento's data), I think the simple plain video tag is fine with the image fallback.

I prefer MEjs here for compatibility, but this is less of a sticking point/blocker.

Either way, I think that the video tag, if that's the route chosen, should be generated by common code with wp_video_shortcode() -- even if it's only the video tag generation parts abstracted out into a new function.

Another question:
Is there anything being done to make sure that mobile clients don’t download large files here?

The media team has been doing work to reduce page download sizes across the board. I’m concerned that we might be set to gradually increase total page size across the web if we encourage the use of video headers/make one a default in Twenty Seventeen.

This is something that I would like to see addressed before a merge of this feature.

This ticket was mentioned in Slack in #core-themes by davidakennedy. View the logs.


11 months ago

#39 @mor10
11 months ago

I'm concerned about this feature because it puts the onus on the host to serve up video, and most hosts are not properly equipped to do so.

Even if we limit the upload size of a video file to 8mb, its inclusion on the front page of a site hosted on a standard shared host can easily result in that site going down due to bandwidth overages or excessive data transfers.

If such a feature is to be added, it should rely on dedicated video hosting services to ensure proper support for all users. At present time, the required optimization features can't be offered by WordPress core because they are server-based.

The baseline recommended requirements for video hosting we've operated with for the past year are:

  1. Cross-browser support through OGG and MP4 (and in some cases WebM)
  2. Dynamic scaling based on bandwidth, screen width, and screen resolution
  3. Dynamic streaming based on current active viewport

a requires the admin to generate two different files that are then referenced in the video element separately. WordPress could handle the actual management of the files, but the conversion and compression must be done elsewhere.

b and c both require dedicated server software and service.

Without these features, the video file will put undue strain on both the host and the visitor, and at present I don't think the necessary technologies are in place to get around the issues with WordPress alone.

My suggestion would be to offer this as a feature, but restrict it to oEmbeds from dedicated video hosting providers (Vimeo, WordPress.TV, Brightcove, Akamai, YouTube, etc) to offload the handling to specialized servers. If not I fear a lot of users will experience problems and blame them on WordPress.

#40 @bradyvercher
11 months ago

Here's a thought based on the discussion earlier: What if the the video tag was rendered using JavaScript instead of being printed directly? Some JavaScript is going to be required for the play/pause button anyway.

The header image would be printed in a container to serve as the default display. Then a script would determine if the video file could be played using the native browser API. If so, the image would be replaced with the video when it's ready to play.

Going that route would allow some of the template tags in the patch to be dropped and could potentially allow the environment to be tested to decide whether or not the video should be displayed at all.

#41 @celloexpressions
11 months ago

Actually, the biggest compatibility issue in terms of styling is that most themes (including Twenty Seventeen, for now) don't have max-width: 100% on video. And because so many themes use header images as background images, I don't see a reasonable way to turn it on by default.

For Ten, Eleven, Twelve, Thirteen, or Fifteen, I don't think video makes sense with their designs based on the aspect rations (and in one case, the use as a background image). I'd prefer to see that added by a child theme if desired, because video doesn't seem like something every theme necessarily should support, similarly to how some of the default themes don't support logos. It should be the theme designer's decision.

If we rendered it in JS, if we can prevent trying to load the video based on available screen size/bandwidth, I see benefit there. However, rendering from JS will likely add confusion to the PHP template tags that this introduces.

I'm definitely opposed to making this external-only, or even having external support at all. Besides the implementation challenges, that goes against the notion of hosting and having complete control over your content. Since this is intended for shorter, design-enhancing videos with image fallbacks as opposed to strictly being content, I see this as a good opportunity to help push the web forward in terms of leveraging the fact that all current browsers now support a common file format, and hopefully in the future we'll have responsive video support as well. We can also add a validate_callback to the customizer setting to prevent using videos larger than a certain arbitrary size.

#42 @kwight
11 months ago

The most recent patch, demonstrating support in Twenty Fourteen, looks great. I do find the Customizer UI a little overwhelming though; I imagine users wanting to have just one Header upload control, and if they happen to upload a video, then the second fallback/poster uploader could be shown (not sure if we already have a similar pattern in core yet).

I also wonder about the same thing @mor10 mentioned:

I'm concerned about this feature because it puts the onus on the host to serve up video, and most hosts are not properly equipped to do so. Even if we limit the upload size of a video file to 8mb, its inclusion on the front page of a site hosted on a standard shared host can easily result in that site going down due to bandwidth overages or excessive data transfers.

We don't really set a good example allowing people to throw up MBs of a video on any page load without any consideration of mobile and such. I wouldn't want to lose the feature because of it, but wonder if there is some sort of basic handling that could prioritize the fallback image in some situations.

#43 @pento
11 months ago

I think the bandwidth argument is a reasonable reason to add YouTube fallback, which can totally be copied from this example.

It would also need to be pluggable (so that Vimeo Pro users can add a plugin to use Vimeo).

#44 @celloexpressions
11 months ago

In terms of the customizer side, implementing a YouTube/vimeo/insert from URL option probably isn't feasible for 4.7 at this point. I'd rather keep it simple and expand to include things like that over time. Liberally falling back to images seems like a viable strategy for now.

#45 @bradyvercher
11 months ago

38172.4.diff contains a concept for a JS-based approach to replacing the header image with a dynamically generated video tag. It has support for loading YouTube videos, which would need to be added to the Customizer. It should be possible for a plugin to create handlers for other sources as well.

This approach would also allow for determining which scenarios videos should be embedded using the wp.customHeader.supportsVideo() method.

This ticket was mentioned in Slack in #core by mike. View the logs.


11 months ago

@celloexpressions
11 months ago

Add validation to prevent videos over 8MB from being used, add initial support to Twenty Seventeen (needs front end work).

#47 @celloexpressions
11 months ago

38172.diff (sorry, looks like we didn't start at 0 so that didn't auto-number) adds validation in the customizer that prevents videos over 8MB from being used. It also adds initial support to Twenty Seventeen, although I got a bit lost with the current header logic there so @laurelfulford should probably work on that next. We need to update the header image to use an img instead of a background image so that we can use the new template tag that handles both.

That's based on 38172.3.diff, but could be easily moved over to 38172.4.diff if we decide to go that route. I would prefer the simpler approach for now, and it could be expanded to do the JS part and support YouTube in the future if needed. However, WordPress has supported self-hosted video for over 3 years now. It's time to start pushing for making that a usable experience instead of having to rely on 3rd-party services. Since this feature is intended for short decorative videos, it seems like the ideal place to focus on self-hosted video and try to catalyze broader improvements to that experience.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


11 months ago

@laurelfulford
11 months ago

Update theme styles and markup for proper video placement.

#49 @laurelfulford
11 months ago

In 38172.6.diff, I updated the theme's styles to incorporate the video header. It still needs some testing and work (including a mobile fallback), but should give a good idea what it'll look and act like.

Based on 38172.diff.

Last edited 11 months ago by laurelfulford (previous) (diff)

@celloexpressions
11 months ago

Customizer proof of concept for external support, with JS approach.

#50 @celloexpressions
11 months ago

38172.7.diff merges 38172.4.diff and 38172.6.diff to provide a proof of concept for external support. It's a separate option but the UI feels okay if not phenomenal. More importantly, it's stored as a separate option to avoid making significant changes to WP_Customize_Media_Control and to keep things as flexible as possible for forward compatibility.

I the JS-based initialization broke selective refresh - can you fix that @bradyvercher? I'm not sure what the best approach would be there, or whether we could do it directly with JS via postMessage now. Self-hosted video also would not loop anymore with this approach, even though the video element has an (empty) loop attribute.

See my experience using this below, along with the forthcoming screencast of it. The quality of the actual graphics used in that is of course low since I did the screencast as a gif. Sometimes the YouTube option would show the loading indicator for a while, then start playing, then randomly switch to the image later. That part could use work. I think I'd still prefer self-hosted-only and using a direct video tag, but if the kinks can be worked out I'm open to the more complex approach.


For a more real-world test, I took a video with my phone today to use as a header video. It was about 11 sections long, but I knew I only needed part of that since it would loop. My phone has an option to trim the video, then I emailed it to myself after trimming it to 5 seconds. It was about 10 MB (incidentally, most of the photos I take are 5-8 MB already, and I'm able to upload those to WP with no issues thanks to size generation). I know that my phone takes very high resolution video and I know how to reduce it, and I also wanted to remove the audio and try to stabilize the motion. I have Premiere, so I was able to edit it, and then export it using settings that got the size down to under the 4MB limit. I also decided to slow it down, as the movement was too distracting. It took a couple of tries and adjusting different settings and further trimming the video to get something I was satified with that would loop nicely. Once I had gone trough that process, the final version was still about 5 seconds but with a lower frame rate and getting it under 4MB was no problem.

I also tried adding a YouTube video instead, with the concept of using a longer, more dramatic (higher-motion) video instead. On my extremely slow connection, the experience of the YouTube video loading was pretty awful (note that I was testing on a local site here, so the hosted video didn't have that problem, but would have still worked better). Themes also have less control over how it displays - it could very well be quite grainy and will usually have black bars to account for an incorrect aspect ratio. It works, but the experience is much better with self-hosted video, where the theme has more control over the positioning and we may be able to make a more informed decision about if and when to show the video. With all of that being said, the best thing for core to do is probably to provide both options and leave the decision up to users. Those that are able to get it working with self-hosted can do so, and those that feel the YouTube option works better than an image for their needs can do that. This is not a feature that every user will be able to use right out of the box - even preparing a video that looks good in this content and loops well requires enough knowledge that dealing with the complexities of meeting a file size limit or opting to host externally instead is a reasonable expectation.

@celloexpressions
11 months ago

Testing with 38172.7.diff, note poor image quality due to recording this as a gif.

#51 @celloexpressions
11 months ago

Adding a note that I tried a direct video tag for a self-hosted video playing through my horrible internet connection, and my browser (Chrome on Windows 10) showed a full-quality frame once one loaded and just didn't play quickly/consistently, but still looked decent because the visual was clear and there wasn't an awkward aspect ratio or loading indicator, the movement was just occasional or inconsistent. After loading the whole (2-3 MB) video, it looped normally at full quality.

Don't think I mentioned in the previous comment, but I bumped the file size limit down to 4MB in 38172.7.diff. Considering that that's the equivalent size of maybe up to 10 featured images for a given theme, it's still a lot but is also not exponentially more than might load on a blog page with an immersive layout anyway. My general expectation on the web is for things to take longer to load if they're graphics-heavy and it's usually worth it; not sure how the average user feels about that, though. There are a lot of factors to consider here; we need to decide where the line between the responsibilities of core and users are.

This ticket was mentioned in Slack in #core by helen. View the logs.


11 months ago

@celloexpressions
11 months ago

Additional validation, refresh for Twenty Seventeen structure change.

#53 @celloexpressions
11 months ago

  • Focuses ui accessibility javascript added
  • Keywords ui-feedback ux-feedback dev-feedback has-screenshots added

38172.8.diff adds additional validation for file format and to inform users that their external video won't be used if a local video is set. It also adjusts the size limit back to 8MB per Slack conversation above and refreshes the patch for changes in Twenty Seventeen's directory structure.

I see potential improvements in the theme API functions, fixing selective refresh for the JS-based frontend approach (any ideas here @bradyvercher?), fixing local video not looping with the JS approach (also something @bradyvercher could help with?) improving the inline help text, and potential adjustments in Twenty Seventeen as the outstanding issues here, most of which could be addressed during beta if needed.

Please test and identify any blockers for an initial commit in the next ~36 hours. The JS-based initialization on the frontend needs the most work and is still largely in proof-of-concept stage. If we can't get the JS approach for the front end polished up, we may need to go back to something like 38172.diff for the output and only support self-hosted video for now.

#54 @davidakennedy
11 months ago

I tested the latest patch, 38172.8.diff, from @celloexpressions.

Nice work! It's a solid effort at combining the two approaches, uploaded video and external video via a URL. I did find some issues, some of which are mentioned already.

I made a video showing some of these issues, since that may be easier for debugging. See: https://cloudup.com/cqnz7llIIGF

  • If a file is too big for upload, it still looks like it's uploaded in the Customizer and the Save & Publish button changes state.
  • Putting a external video URL in when an uploaded video is already present is a bit buggy. The preview doesn't update like it should.

Front end issues:

  • There is no visual play/pause button. This is a requirement for basic accessibility, and it should be reachable and operable via the keyboard. Play/pausing does occur if I click the entire video, but that isn't obvious.
  • Looping seemed not to work, even though the loop parameter is on the video tag.

Theme API:

  • It would be nice to have some classes to the #wp-custom-header and #wp-custom-header-video containers.

#55 @bradyvercher
11 months ago

Sorry for the slow response @celloexpressions, I've been out of commission for a few days. 38172.9.diff should add selective refresh support and fix looping.

#56 @celloexpressions
11 months ago

Nice work @bradyvercher, thanks!

@davidakennedy those first two issues are quirks with the way customizer notifications work. When a setting is invalid, you can't save your changes and the preview won't update. I'm thinking an error notification may not be the best approach for the unused-external-video case (we may need to remove that check entirely), but it's probably the best option for the rest of the validation checks.

It looks like the remaining issues are:

  • Adding a minimal play/pause button, which themes can style, and designing a style for that in Twenty Fourteen and Seventeen. Maybe add basic SVG-ified dashicons for those, so the frontend can load the icons directly instead of loading dashicons? Then themes can adjust the styling and colors of the button?
  • Add classes to facilitate theme styling.
  • Consider removing the error validation that prevents having both a local and external video set.
  • Consider elaborating on the section-level help text and moving it behind a help icon (via the new capability to do that in customizer sections).

I'm comfortable with an initial commit without those adjustments if the issues with loop and selective refresh are resolved in 38172.9.diff, I haven't been able to test thoroughly yet.

#57 @joemcgill
11 months ago

Been playing with this feature over the past few days and wanted to add my thoughts. Generally, I think you all have done a nice job of working through the implementation issues here. A few issues I noticed while testing the latest patch, some of which are theme specific.

  • My main concern about making this a core feature is that we're encouraging sites to push more bandwidth at users. Setting a size limit on videos to 8MB is helpful, but I think it's wasteful to load the header image only to immediately swap it for the video HTML once the document loads. I would rather see us lazy load either the video or the image, depending on context and browser support, not both.
  • Related, it looks like this loads both the header image and video regardless of screen size. I know that trying to assume much about someone's connection based on screen size can be problematic, but I'm curious if there is a way to set a minimum width for loading the video.
  • In TwentySeventeen, video headers seem to only work if you also have an image header set. This does not a requirement in TwentyFourteen. Not sure which is the intended functionality, but either way, the UX could be more clear here. If video headers are dependent on setting an image, I would suggest moving those controls beneath the image controls in the customizer.
  • When switching from a local video header to an external video, I had to first remove the local video and save my settings before setting the external URL, otherwise the validation callback still sees the ID for the local video and returns an error.
  • In my testing, YouTube URLs don't automatically loop, so you see the YT play button once the video ends, which is kind of odd. Additionally, YouTube videos don't autoplay on mobile devices and click events don't seem to reach the iframe in order to start them (in TwentySeventeen at least).

Overall, I think these problems can be worked out, though I will note that I still have reservations about making this a core feature because of concerns about performance and developer experience. Seems like this feature still puts a lot of responsibility on theme authors to work out implementation details—for example, TwentySeventeen only displays header videos on the home page, while the latest patch for TwentyFourteen displays these on all pages. I would not like to see us encourage bad video header implementations in themes by rushing this out the door.

This ticket was mentioned in Slack in #core by celloexpressions. View the logs.


11 months ago

#59 @joemcgill
11 months ago

  • Owner set to joemcgill
  • Status changed from new to accepted

#60 @bradyvercher
11 months ago

Thanks for taking the time to review this, @joemcgill!

Setting a size limit on videos to 8MB is helpful, but I think it's wasteful to load the header image only to immediately swap it for the video HTML once the document loads. I would rather see us lazy load either the video or the image, depending on context and browser support, not both.

This is pretty much how the native <video> element with a poster attribute works. If you load that up in a browser and take a look at the Network tab in DevTools, you'll see requests for each.

The image should display when the video can't play, which includes environments without JavaScript support. A theme could use CSS to hide the image for browsers with JS support, which should prevent it from loading, then the script here could make it visible if the video can't be embedded, but that complicates the implementation a bit.

Related, it looks like this loads both the header image and video regardless of screen size. I know that trying to assume much about someone's connection based on screen size can be problematic, but I'm curious if there is a way to set a minimum width for loading the video.

I added a supportsVideo() method with a comment in wp-custom-header.js that the environment could be inspected before deciding to load the video, but I haven't seen any discussion about which environments this should be restricted to. Is there a certain breakpoint that would be preferable?

Aside from screen width, Android and iOS (prior to 10) won't automatically play videos without user interaction, so checking the user agent for those devices would probably make for a good test. Here's some more information about new video policies in iOS 10.

In any case, adding tests should be fairly easy if the requirements can be defined.

In TwentySeventeen, video headers seem to only work if you also have an image header set. This does not a requirement in TwentyFourteen. Not sure which is the intended functionality...

I don't believe that's the intended behavior, so the Twenty Seventeen implementation likely needs to be adjusted.

When switching from a local video header to an external video, I had to first remove the local video and save my settings before setting the external URL, otherwise the validation callback still sees the ID for the local video and returns an error.

I've run into a few weird quirks with validation as well. Consolidating those two controls would be ideal from a UX perspective, but it would likely require a new, unique control.

In my testing, YouTube URLs don't automatically loop, so you see the YT play button once the video ends, which is kind of odd.

The loop setting is set to true for those, but we could also just play the video again when the ended event is triggered. I can add that if necessary.

Additionally, YouTube videos don't autoplay on mobile devices and click events don't seem to reach the iframe in order to start them (in TwentySeventeen at least).

Not autoplaying is likely related to the restrictions I mentioned earlier in Android and iOS.

Click events seem to work fine for me on desktop, but the site title and description do cover the bottom portion of the video, so those would catch click events. Adding the play/pause control mentioned by @davidakennedy and @celloexpressions would help, too.

#61 follow-up: @celloexpressions
11 months ago

38172.10.diff:

  • Fix Twenty Seventeen when no image is set.
  • Remove the validation for there not being a local video when an external video is set.
  • Only play video on screens wider than 900px and taller than 500px (I made these numbers up, they can be changed to whatever).
  • Only show video if is_front_page, otherwise show the image. We should consider making the customizer controls contextual to this behavior.
  • I'm still not seeing local videos loop in Chrome/Windows, but the video tag hass the loop attribute. Not sure why.

@joemcgill please review and @bradyvercher if you have time to take another stab at this before an initial commit tonight, please do. We also need to add a few things later but any bugs would be good to fix before beta. Testing with Twenty Seventeen and Twenty Fourteen it works pretty well now with YouTube or self hosted videos.

#62 @bradyvercher
11 months ago

I'll try to take a look later tonight, but from a quick glance, some of the changes in 38172.9.diff are missing in 38172.10.diff. The header template in Twenty Seventeen also looks way more complicated than it needs to be, so I'll try to simplify that a bit, too.

@joemcgill
11 months ago

#63 @joemcgill
11 months ago

The conditional checks for loading the video, and the validation issues with external videos both look good in 38172.10.diff, but now previews are broken when you change the video.

38172.11.diff fixes a couple of issues in Twenty Seventeen in 38172.10.diff, namely:

  1. The custom header CSS relies on the presence of a .has-header-image class which wasn't being added on front pages when a header video is set, but not a header video.
  2. This simplifies the logic of template-parts/header/header-image.php so custom headers aren't displayed on secondary pages if only a header video is set but no header image.

Mentioned in Slack, but will mention again for posterity— video looping is working fine in Safari 9.1.2 (Mac) and IE9 & IE11 (Win) but not on FF or Chrome. It may be a quirk in how some browsers apply this functionality when partial content requests, see: http://stackoverflow.com/questions/8088364/html5-video-will-not-loop. We might need to fallback to an event listener that manually plays the video when it reaches the end.

#64 @celloexpressions
11 months ago

Confirmed that 38172.12.diff fixes the major issues noted above. I think that's ready for an initial commit, with follow ups to add visible play/pause and iterate on the other issues mentioned for work during beta.

#65 @bradyvercher
11 months ago

38172.12.diff restores selective refresh and video looping.

Twenty Seventeen still feels overly complicated, but it probably needs to do a few more things:

  • Set the header_image and header_image_data settings to use the postMessage transport
  • Toggle the has-header-image body class in customizer.js whenever any of the header settings are changed
  • Trigger whichever functions in global.js reposition the fixed navigation after a video is inserted

Ideally, I would like to see a theme only have to call the_custom_header() without adding any logic to check to see if an image or video exists like Twenty Seventeen currently does.

#66 @joemcgill
11 months ago

Thanks @bradyvercher 38172.12.diff does fix the video looping issues in Chrome and FF, and also fixes the preview issues mentioned above.

I agree with you that the developer experience for including the custom headers in themes is still more complex that I'd hope for, but this may be good for an initial commit.

@joemcgill
11 months ago

#67 @joemcgill
11 months ago

38172.13.diff should be identical to 38172.12.diff but fixes issues with fixed positioning of the navigation in Twenty Fourteen.

@joemcgill
11 months ago

#68 @joemcgill
11 months ago

One thing I noticed that makes the developer experience a bit strange is the mismatch between the new function the_custom_header() which prints the markup for custom headers, and get_custom_header(), which returns an object containing the settings for custom headers. Generally, our the_{name of thing}() functions are wrappers for their get_{name of thing}() counterparts.

In 38172.14.diff I'm proposing the we rename the_custom_header() to the_custom_header_html() which is a wrapper for a new get_the_custom_header_html() function. This also adds a has_custom_header() function that can be used in get_custom_header_html() and also by themes before printing their custom header markup. I've updated both Twenty Seventeen and Twenty Fourteen to use the new function names.

#69 @davidakennedy
11 months ago

@joemcgill I think the function renaming is headed in the right direction. Using html in the function name feels weird to me. But I think it's more important to not break any expectations with the function naming, which we were doing as you pointed out, and instead figure out the best way to do this now. It feels better now than before for sure.

@joemcgill
11 months ago

#70 @joemcgill
11 months ago

Thanks @davidakennedy. In 38172.15.diff I changed the names of the/get_custom_header_html() to the/get_custom_header_markup().

@joemcgill
11 months ago

#71 @joemcgill
11 months ago

38172.16.diff proposes setting default minimum viewport height and widths in get_header_video_settings() and letting those values be filterable so themes can modify their minimum widths based on the layout of the theme. Not sure if this is the best idea in general, but seems there should be a way for themes to override whatever defaults we set.

@joemcgill
11 months ago

#72 @joemcgill
11 months ago

38172.17.diff removes the is_customize_preview() check from the_custom_header_markup() when deciding whether to enqueue the custom header scripts since that made the behavior when navigating away from the front page in the customizer different than what happens outside the customizer.

@joemcgill
11 months ago

Add postMessage support for themes

#73 @joemcgill
11 months ago

38172.18.diff sets transport to postMessage on the custom header settings in Twenty Seventeen and Twenty Fourteen.

@joemcgill
11 months ago

#74 @joemcgill
11 months ago

In 38172.19.diff fires a wp-custom-header-video-loaded event whenever we load the custom header video which themes can listen for to make changes based on the inserted video. For example, in Twenty Fourteen, we recalculate the header height once the video loads so we can fix the navigation to the top of the screen on scroll.

@joemcgill
11 months ago

#75 @joemcgill
11 months ago

38172.20.diff fixes some Jshint issues in wp-custom-header.js.

@joemcgill
11 months ago

#76 @joemcgill
11 months ago

38172.21.diff changes the ID of the selective refresh partial for custom headers to custom_header and fixes the render callback to use the_custom_header_markup(). This also includes some formatting and typo cleanup.

#77 @joemcgill
11 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 38985:

Themes: Enable video in custom headers.

This adds the ability for themes to add support for videos in custom headers
by passing 'video' => true as an argument when adding theme support for
custom headers.

Custom video headers are managed through the “Header Visuals” (i.e. “Header Image”)
panel in the Customizer where you can select a video from the media library or set a
URL to an external video (YouTube for now) for use in custom headers.

This introduces several new functions:

has_header_video() – Check whether a header video is set or not.
get_header_video_url() – Retrieve header video URL for custom header.
the_header_video_url() – Display header video URL.
get_header_video_settings() – Retrieve header video settings.
has_custom_header() – Check whether a custom header is set or not.
get_custom_header_markup() – Retrieve the markup for a custom header.
the_custom_header_markup() – Print the markup for a custom header.

And a new file, wp-includes/js/wp-custom-header.js that handles loading videos
in custom headers.

This also enables video headers in the Twenty Seventeen and Twenty Fourteen themes.

Props davidakennedy, celloexpressions, bradyvercher, laurelfulford, joemcgill.
Fixes #38172.

#78 @laurelfulford
11 months ago

38172.22.diff is a fix for an issue discovered post merge - a background colour style that was added to the front page did not get included in the colors-dark.css, causing this display issue with the dark colour scheme:

https://cldup.com/RNAiJA1Doq.png

This ticket was mentioned in Slack in #core-themes by laurelfulford. View the logs.


11 months ago

#80 @davidakennedy
11 months ago

In 38987:

Twenty Seventeen: Fix issue with missing background color in dark color scheme.

A background color style that was added to the front page during video header implementation did not get included in the colors-dark.css, causing a display issue with the dark color scheme.

Props laurelfulford.

See #38172.

#81 @bradyvercher
11 months ago

38172.23.diff updates the partial id when exporting video settings for selective refresh. It got overlooked when making some last minute updates to the partial's id.

#82 @davidakennedy
11 months ago

In 38988:

Twenty Seventeen: Make border colors for panels match WordPress UI

Also, fixes the visibility of the borders for panels after the header videos commit.

Props celloexpressions.

Fixes #38408.
See #38172.

#83 @joemcgill
11 months ago

In 38989:

Customizer: Fix name of a partial in export_header_video_settings()

Following [38985], this updates a check for partials being set in
export_header_video_settings() to use the correct name.

Props bradyvercher.
See #38172.

This ticket was mentioned in Slack in #core-themes by bradyvercher. View the logs.


11 months ago

#85 in reply to: ↑ 61 @westonruter
10 months ago

Replying to celloexpressions:

  • Only show video if is_front_page, otherwise show the image. We should consider making the customizer controls contextual to this behavior.

I opened #38778 for this.

#86 @westonruter
10 months ago

In 39234:

Customize: Use video-specific labels for buttons in Header Video media control.

See #38172.

#87 @westonruter
9 months ago

In 39560:

Customize: Trim whitespace for URLs supplied for external_header_video to prevent esc_url_raw() from making them invalid.

Props tyxla.
See #38172.
Fixes #39125.

#88 @dd32
9 months ago

In 39573:

Customize: Trim whitespace for URLs supplied for external_header_video to prevent esc_url_raw() from making them invalid.

Props tyxla.
See #38172.
Merges [39560] to the 4.7 branch.
Fixes #39125.

Note: See TracTickets for help on using tickets.