[wp-trac] [WordPress Trac] #38995: Twenty Seventeen: Custom headers incorrect on mobile when no image is set.

[WordPress Trac]

#38995: Twenty Seventeen: Custom headers incorrect on mobile when no image is set.
-------------------------------------+------------------------
 Reporter:  joemcgill                |       Owner:  joemcgill
     Type:  defect (bug)             |      Status:  accepted
 Priority:  normal                   |   Milestone:  4.7
Component:  Bundled Theme            |     Version:  trunk
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+------------------------

Comment (by laurelfulford):

 Thanks for this, @joemcgill!


 These changes look good overall (and fixed a couple ugly issues I
 introduced - thanks!!)

 I made a couple tweaks in [attachment:38995.2.patch]:

 1. In a couple cases, I re-added styles for `img` in `.has-header-video`
 because the images are inside of a `.custom-logo-link` container and
 aren’t custom header images.

 2. I had totally missed updating the ie8.css styles - thanks for catching
 that! I tested IE8, and you’re right - it doesn’t look like we need video
 styles here at all. The video doesn’t appear regardless if it’s mp4 or
 from YouTube. I’ve updated these styles to remove video references all
 together.

 3. Made similar updates to `.has-header-video` class in custom-header.php
 from fix for #38980.

 4. I re-added a change to adjustScrollClass() in global.js from
 [attachment:38995.patch]:[[br]][[br]]Originally we were checking for the
 presence of `.custom-header-image` (now `.custom-header-media`) to decide
 when to make the menu ‘sticky’. But there’s an edge-case issue: it happens
 when a site has a video but no header image, and you view the site on a
 screen size just above the tablet breakpoint (769px - 899px wide). The
 video isn’t visible at this screen size, but the JavaScript thinks it is
 because `.custom-header-media` is there, causing it to miscalculate where
 the menu should be stuck.[[br]][[br]]I’ve switched it to check for the
 `.has-header-video` or `.has-header-image` classes instead.[[br]][[br]]I
 didn’t comment on this specifically with the last patch, so it wasn’t
 clear why I had made that change - sorry about that!

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