Opened 11 years ago
Closed 11 years ago
#23722 closed defect (bug) (fixed)
Twenty Thirteen: toggling Display Header Text in Theme Customizer not working
Reported by: | rachelbaker | Owned by: | lancewillett |
---|---|---|---|
Milestone: | 3.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Bundled Theme | Keywords: | |
Focuses: | Cc: |
Description
Selecting or deselecting the "Display Header Text" checkbox does not result in expected behavior of hiding or displaying the title and description in the theme's header.
The theme customizer control for this setting appears to be for setting a color for the title and description text, no displaying or hiding the text.
Attachments (3)
Change History (15)
#1
@
11 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 3.6
- Severity changed from minor to normal
- Summary changed from Twenty Thirteen: Theme Customizer for Site Title & Description is inconsistent to Twenty Thirteen: toggling Display Header Text in Theme Customizer not working
@
11 years ago
Accounts for header image not being set and for cases where header text is hidden to begin with
#5
@
11 years ago
- Owner set to lancewillett
- Resolution set to fixed
- Status changed from new to closed
In 23670:
#6
follow-up:
↓ 11
@
11 years ago
In style.css
, min-height
is set for .site-header hgroup
:
http://core.trac.wordpress.org/browser/trunk/wp-content/themes/twentythirteen/style.css?rev=23680#L802
[23670] sets it for #masthead hgroup
. Shouldn't the selector be the same for consistency?
#7
@
11 years ago
- Keywords has-patch commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
Found another issue:
- Go to Appearance > Header, disable header text.
- Go to Customizer, enable header text.
- Go to Appearance > Header again. You'll see the header with a wrong color (see the screenshot).
Code inspection shows that get_header_textcolor()
returns an empty value.
#8
follow-up:
↓ 9
@
11 years ago
Twenty Twelve also has an empty color in the style attribute (as in the screenshot above), but it still shows the correct color due to the declaration in twentytwelve_admin_header_style()
:
http://core.trac.wordpress.org/browser/tags/3.5.1/wp-content/themes/twentytwelve/inc/custom-header.php#L109
Twenty Eleven doesn't display the header on Appearance > Header screen at all after those steps, so there might be an underlying core issue.
#9
in reply to:
↑ 8
@
11 years ago
Replying to SergeyBiryukov:
Twenty Eleven doesn't display the header on Appearance > Header screen at all after those steps
It specifically checks that get_header_textcolor()
is not empty:
http://core.trac.wordpress.org/browser/tags/3.5.1/wp-content/themes/twentyeleven/functions.php#L313
#10
follow-up:
↓ 12
@
11 years ago
I can reproduce this broken color issue with both Twenty Eleven and Twenty Thirteen and I think it's not related to the theme at all. The trick here is that both fields (text-color and display-header-text) are actually one single setting. In wp-admin/custom-header.php, we check the value for a color, and if a color is not found, we set it to blank
, so setting an empty text-color results in blank
, which is fine, but do note that we never set the header_textcolor
mod to an empty string.
In the Customizer, on the other hand, both sanitize_hex_color_no_hash
and sanitize_hex_color
are allowed to return an empty string, which is then passed on to the header_textcolor
theme mod.
I feel that the broken part is in wp-admin/js/customize-controls.js when juggling the two controls -- the setting isn't reset to its default color when the checkbox is checked and the value is blank
. I can't think of a good way to fix this yet.
#11
in reply to:
↑ 6
@
11 years ago
Replying to SergeyBiryukov:
In
style.css
,min-height
is set for.site-header hgroup
:
[23670] sets it for#masthead hgroup
. Shouldn't the selector be the same for consistency?
I chose to use #masthead
in the jQuery selector because I thought it might be faster to look for an ID rather than a class. In the end it will always be an inline style added to the hgroup
element, so changing the jQuery selector will have no effect on the priority of the added CSS rule.
Thanks for the report, good eye. Looks like we missed this.