[wp-trac] [WordPress Trac] #38172: Enable Video Headers in Custom Headers
WordPress Trac
noreply at wordpress.org
Fri Oct 14 17:12:07 UTC 2016
#38172: Enable Video Headers in Custom Headers
----------------------------+--------------------
Reporter: davidakennedy | Owner:
Type: task (blessed) | Status: new
Priority: normal | Milestone: 4.7
Component: Themes | Version: trunk
Severity: normal | Resolution:
Keywords: has-patch | Focuses:
----------------------------+--------------------
Comment (by obenland):
Replying to [comment:28 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?
--
Ticket URL: <https://core.trac.wordpress.org/ticket/38172#comment:36>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list