[wp-trac] [WordPress Trac] #38391: Twenty Seventeen: Background colour for header

[WordPress Trac]

#38391: Twenty Seventeen: Background colour for header
--------------------------------------+----------------------------
 Reporter:  laurelfulford             |       Owner:  davidakennedy
     Type:  defect (bug)              |      Status:  assigned
 Priority:  normal                    |   Milestone:  4.7
Component:  Bundled Theme             |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  good-first-bug has-patch  |     Focuses:
--------------------------------------+----------------------------

Comment (by Idealien):

 In Slack bug scrub yesterday @ [3:58 PM] celloexpressions wrote
      @davidakennedy: The patch for 38391 looks correct but isn't quite
 there according to @idealien. I think the has_header_video() case in the
 conditional needs to have an && is_front_page(), so that its not shown
 when there's a video but no image and you're on an inner page and it's
 always shown if you have an image regardless of the video.

 I believe there is something else in logic related to header_textcolor
 control presentation causing a larger issue for this patch.

 '''Scenario 1'''
 If I put the same twentyseventeen_is_header_textcolor_visible function as
 active_callback on colorscheme control defined earlier in the customer.php
 it behaves mostly as expected.

 Repeatable steps for issue that the control is not hidden after add +
 remove of image or video without full page reload:

 * [correct] Start with No Image or Video and page refresh, colorscheme is
 hidden
 * [correct] Add Video and colorscheme displays
 * [incorrect] Remove Video, colorscheme stays
 * [correct] Save and force full page refresh and colorscheme is hidden

 '''Scenario 2'''
 Remove the header_textcolor control and re-add it with snippet below. The
 control never changes show / hide behavior. BUT...When adding the new
 control rename it as "header_textcolour" (eh!) and it does work identical
 to Scenario 1.

 {{{#!php

 <?php
         $wp_customize->get_control( 'header_textcolor' )->active_callback
 = 'twentyseventeen_is_header_textcolor_visible';
         $wp_customize->remove_control( 'header_textcolor' );
         $wp_customize->add_control( new WP_Customize_Color_Control(
 $wp_customize, 'header_textcolor', array(
                 'label'       => __( 'Header Text Color',
 'twentyseventeen' ),
                 'description' => __( 'Select Color', 'twentyseventeen' ),
                 'section'     => 'colors',
                 'active_callback' =>
 'twentyseventeen_is_header_textcolor_visible',
         ) ) );
 ?>

 }}}

 Some other logic related to presentation of header_textcolor overlapping
 impact on the callback defined here? Debugging into that depth of JS is
 out of my realm for this release, but hopefully the details above can
 point someone else in a better direction towards right solution.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/38391#comment:9>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform