#38993 closed defect (bug) (fixed)
Twenty Seventeen: Clearing header text color doesn't update color in preview
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 4.7 | Priority: | low |
| Severity: | normal | Version: | 4.7 |
| Component: | Bundled Theme | Keywords: | has-patch needs-testing commit dev-reviewed |
| Focuses: | Cc: |
Description
Steps to reproduce:
- Clear existing theme mods (
wp eval "remove_theme_mods();") - Go to the customizer and the colors section
- Choose a color
- Click "Save & Publish"
- Reload customizer (CMD/CTRL+R)
- Go to the customizer and the colors section
- Click the "Clear" button
- Bug: The preview still shows the previous color
Attachments (3)
Change History (15)
This ticket was mentioned in Slack in #core by helen. View the logs.
8 years ago
#3
@
8 years ago
In 38993.2.patch, I bounce off the good work from @sstoqnov but do it a bit differently. I just remove the styles from the <head> since the theme stylesheet can take care of things. This avoids the extra logic in the Customizer preview JS.
@laurelfulford Want to test this one out and provide feedback?
#4
@
8 years ago
This looks good to me, @davidakennedy!
I tested it with a variety of combinations - with and without both video and header image (and with both); on subpages and on the front page, and with the dark and custom colour schemes. The header colour consistently returned to the default, whatever that was based on the other settings (white, dark grey, or custom).
This ticket was mentioned in Slack in #core by davidakennedy. View the logs.
8 years ago
#7
follow-up:
↓ 8
@
8 years ago
@davidakennedy
- What about the situation when the header text color is added? This patch removes the
#twentyseventeen-custom-header-stylesstylesheet entirely. When the header text is added shouldn't thestyleelement be restored? - Likewise, when
is_customize_preview(), thetwentyseventeen_header_style()should output an emptystyleelement when short-circuiting, even when there are no styles. This is so that the the style can be restored. - The JS logic assumes that
default-text-coloris going to be an empty string. What if a child theme sets the default color?
#8
in reply to:
↑ 7
@
8 years ago
Replying to westonruter:
Good questions @westonruter!
- What about the situation when the header text color is added? This patch removes the
#twentyseventeen-custom-header-stylesstylesheet entirely. When the header text is added shouldn't thestyleelement be restored?
That's covered here:
$( '.site-branding, .site-branding a, .site-description, .site-description a' ).css({
color: to
});
This code only comes into play if to is empty. It's easier to remove the styles than to figure out what the default text color should be, override it, etc. – the color is different based on whether a header image or video is in place.
- Likewise, when
is_customize_preview(), thetwentyseventeen_header_style()should output an emptystyleelement when short-circuiting, even when there are no styles. This is so that the the style can be restored.
The styles are added in the other parts of the code... this use case only covers the preview when you press the "Clear" button.
- The JS logic assumes that
default-text-coloris going to be an empty string. What if a child theme sets the default color?
If Twenty Seventeen or a child theme where to add a default text color, everything should work fine – unless I'm missing something. I tested this by adding a default text color to Twenty Seventeen. The added logic never is run because the button now applies the "Default" color instead of clearing it.
If I'm missing something, let me know.
#9
@
8 years ago
- Keywords dev-feedback added
@davidakennedy thanks for sitting down to explain it with me. It looks good to me.
+1 needs dev-reviewed
As @ocean90 mentioned in Slack discussion, the color doesn't remove because the styles are still in header.
Since we are not able to remove that styles I made a check if there is a color, and if not revert to original colors.
I am not sure that this is the best way to add the text color, when there is custom header image.
And I think that there is another problem with color picker.