Added custom header support for the Theme #133
Conversation
It added the checkbox to display the title/tagline as well. WordPress.org Username: mmaumio
| * @uses twentynineteen_header_style() | ||
| */ | ||
| function twentynineteen_custom_header_setup() { | ||
| add_theme_support( 'custom-header', apply_filters( 'twentynineteen_custom_header_args', array( |
There was a problem hiding this comment.
Add the following line to enable header video support. See #243:
'video' => true,
There was a problem hiding this comment.
General comments and additional information for implementing video support per #243.
| * Sample implementation of the Custom Header feature | ||
| * http://codex.wordpress.org/Custom_Headers | ||
| * | ||
| * You can add an optional custom header image to header.php like so ... |
There was a problem hiding this comment.
This comment should probably be removed
| @@ -0,0 +1,74 @@ | |||
| <?php | |||
| /** | |||
| * Sample implementation of the Custom Header feature | |||
| * @see twentynineteen_custom_header_setup(). | ||
| */ | ||
| function twentynineteen_header_style() { | ||
| $header_text_color = get_header_textcolor(); |
There was a problem hiding this comment.
I recommend turning off support for the header text color. An overlay is provided to ensure readability. See also the discussions for Twenty Seventeen.
If header text color is a feature, it should be coordinated with the other color customization options instead of being implemented here.
| @@ -22,6 +22,11 @@ function twentynineteen_body_classes( $classes ) { | |||
| $classes[] = 'hfeed'; | |||
| } | |||
|
|
|||
| // Add a class if there is a custom header. | |||
| if ( has_header_image() ) { | |||
There was a problem hiding this comment.
Revise to:
if ( has_header_image() || has_header_video() ) {
| @@ -22,6 +22,11 @@ function twentynineteen_body_classes( $classes ) { | |||
| $classes[] = 'hfeed'; | |||
| } | |||
|
|
|||
| // Add a class if there is a custom header. | |||
| if ( has_header_image() ) { | |||
| $classes[] = 'has-header-image'; | |||
There was a problem hiding this comment.
Revise to:
$classes[] = 'has-header-media';
| @@ -7,5 +7,37 @@ | |||
| */ | |||
|
|
|||
| ( function( $ ) { | |||
|
|
|||
There was a problem hiding this comment.
I recommend leaving any color customization or other changes to this file out of this PR, and coordinating those with the proposed color customization options.
| height: auto; | ||
| } | ||
|
|
||
| .has-header-image .custom-header .wp-custom-header img { |
There was a problem hiding this comment.
Add the selector for header video.
There was a problem hiding this comment.
Which is probably:
.has-header-media .custom-header .wp-custom-header video
| @@ -1259,14 +1259,36 @@ body.page .main-navigation { | |||
| /*-------------------------------------------------------------- | |||
| ## Header | |||
| --------------------------------------------------------------*/ | |||
There was a problem hiding this comment.
Add this first:
.wp-custom-header iframe,
.wp-custom-header img,
.wp-custom-header video {
display: block;
height: auto;
max-width: 100%;
}
There was a problem hiding this comment.
Because this theme is primarily focused on Gutenberg, we don’t really want to add too many content-related customizer options. Instead we want to promote the Gutenberg editor as much as possible, so we’re utilizing featured images to achieve the header image behavior.
As always, we’re open to further discussion on this :-)
|
I think this is good to have, regardless of the editor used. |
|
So, are we keeping the custom header support off for now? |
Yes — as per @allancole's notes on focusing on Gutenberg, let's leave this out for now. I'll go ahead and close this. Thank you for the PR in any case! |
It added the checkbox to display the title/tagline as well.
Allowed users to upload a header image from the customer.
Allowed the option to change the Site Title and Description.
As It was present on the other Bundled themes.
WordPress.org Username: mmaumio