Twenty Fourteen: Let Users Customize the Green "Accent" Color

[celloexpressions]

Continued from #25094.

[celloexpressions]

Refreshed for social links removal - [25214]

[lancewillett]

As discussed in today's office hours (​log) we're going to remove the ability to change the header text color in favor of this more general accent color.

[obenland]

Patch looks good, only some minor formatting issues:

• ​Use spaces to align things in a block of code.
• Comments should always have the first letter capitalized and end with a period.
• Functions should have (at least) arguments and return value documents with PHPDoc (no trailing empty line necessary)

As mentioned during chats, we should remove the priority argument and find better names for accent_1 and accent_2.

[celloexpressions]

Formatting fixes

[celloexpressions]

25220.5.diff​ addresses the formatting fixes. We still need to discuss the naming conventions, though; I think we can do better than accent_lighter and accent_much_lighter.

[obenland]

Patch .5 looks really good, but I agree, accent_* names should be further iterated on.

wp_add_inline_style expects the CSS string not to include any HTML, IIRC.

[SergeyBiryukov]

Replying to obenland:

wp_add_inline_style expects the CSS string not to include any HTML, IIRC.

That was only the case with SCRIPT_DEBUG enabled (until [25202]), see #24813.

[lancewillett]

Tiny formatting things in .6 (just for reference on what I changed):

• Add spaces after if statement word before braces
• Yoda-style comparison for if ( '#24890d' === $accent_color ) and changed to === for strict comparison
• Cleaned up trailing spaces

[lancewillett]

In 25418:

Twenty Fourteen: let authors customize the green "accent" color by changing to a different hex value in the Customizer. Props celloexpressions.

Also add an ID attribute to all style elements.

See #25220.

[celloexpressions]

Next steps:

• Look into applying the primary accent color to links and highlight color in TinyMCE.
• Add selectors to adjust the colors on the mediaelement players and anywhere else they've been added since this initial patch was started.
• Make sure all Twenty Fourteen patches touching non-grayscale colors and/or their selectors also make the necessary edits to the <style> block in customizer.php.
• Fix discrepancies between color patterns with and without a custom accent color; for example, the hover color of top-level header nav items (note that this one also seems to have been overridden with child themes of Further, for example, on ​http://themeshaper.com). I think paging navigation also needed some attention.
• Consider adjusting a couple of color patterns; for example, the header nav sub-menu item hover color is the darkest (original) accent color, displayed against a black background, we should consider changing this to the lightest generated color like the other link hover on black background items. This is the only place where that color is used on a black background, which limits the ability to use darker accent colors. Might be worth having @iamtakashi take a look at it with a few alternate accent colors and provide direction on anywhere we should adjust the use of the different generated colors (or adjust the generation process).
• Remove the ability to change the "header text color" -- which is really only the site title text color. If we want to keep the ability to hide that text, we'll need to add the ability to remove the color control and keep the other header controls to core (and it should definitely be there), which I can look into.
• Come up with better names for accent_lighter and accent_much_lighter. The variables don't matter much, but those labels are also used for the theme_mods.

[celloexpressions]

Apply the primary accent color to the current-time indicator for the mediaelements players (default accent color added there while initial patch was in progress).

[celloexpressions]

Fix several instances of the custom accent color CSS overriding white or black link colors that should have no text-color change on hover by adding link color declarations to additional and more specific selectors.

[celloexpressions]

Remove the text colors from the header nav hover states for sub-menus, and prevent custom accent colors from adding it to top-level items, per @iamtakashi's suggestions; change the background color more on hover for submenus.

[celloexpressions]

Just uploaded several patches that address points from the to-do list. The only one that really needs discussion is the one that changes the hover-state colors in the header nav menu. Particularly, for the sub-menus we decided to try retaining the white text color and adjusting the background color to give it more contrast from the top-level items. Since we couldn't go any darker for that hover color (background was #000), I tried going lighter, slightly lighter than the top-level items go.

I think the best way to make sure we don't miss any colors in customizer.php is to strongly encourage all theme testers/contributors to pick a custom color and leave it enabled. That way, people will notice bugs with that component specifically (like the paging ones patched above) as well as remembering to address it when modifying other things. The theme should be polished regardless of whether this customization is in use, so we might as well test it with custom colors.

[lancewillett]

In 25612:

Twenty Fourteen: fixes for accent color functionality, props celloexpressions. See #25220.

• Apply the primary accent color to the current-time indicator for the mediaelements players.
• Fix instances of the custom accent color CSS overriding white or black link colors that should have no text-color change on hover.
• Remove text color from main navigation hover states for submenus and from main navigation top-level items.
• Change the background color a bit more on hover for main navigation submenus.

[celloexpressions]

The commit message doesn't match the commit :)

You did the first two, but not the header nav stuff, and that needs discussion (or approval from iamtakashi) first anyway.

[iamtakashi]

Replying to celloexpressions:

... the header nav stuff, and that needs discussion (or approval from iamtakashi) first anyway.

We could match the background color of hover state for submenus the same as the top level: #2b2b2b rather than introducing another shade of gray. Then we can also remove line 713 .primary-navigation li li:hover > a.

[lancewillett]

Replying to celloexpressions:

The commit message doesn't match the commit :)

You did the first two, but not the header nav stuff, and that needs discussion (or approval from iamtakashi) first anyway.

You're right, I meant to apply it and let you both iterate in subsequent patches, but didn't end up applying it after I'd written the commit message.

[lancewillett]

In 25623:

Twenty Fourteen: change the background color a bit more on hover for main navigation submenus. See #25220.

[celloexpressions]

Replying to iamtakashi:

Replying to celloexpressions:

... the header nav stuff, and that needs discussion (or approval from iamtakashi) first anyway.

We could match the background color of hover state for submenus the same as the top level: #2b2b2b rather than introducing another shade of gray. Then we can also remove line 713 .primary-navigation li li:hover > a.

I think having something different for the submenus helps guide the visual hierarchy, but maybe we could try accomplishing that by using a darker text color. I'll add another patch which does that, matching the text color of the site description (which should probably be converted from rgba to hex, by the way. It's #8c8c8c I think).

[celloexpressions]

Try a darker text color for header nave submenu items on hover (matching site description color).

[celloexpressions]

Fixes another bad hover color override.

[iamtakashi]

Replying to celloexpressions:

I think having something different for the submenus helps guide the visual hierarchy, but maybe we could try accomplishing that by using a darker text color. I'll add another patch which does that, matching the text color of the site description (which should probably be converted from rgba to hex, by the way. It's #8c8c8c I think).

I've tested 25220.header-submenu-hover.2.diff​ but I don't like the low contrast text color for the hover state with the grey background.

Another idea is that we can match the style with the secondary navigation on the left so that both navigations have the same (or very close) patterns.

[celloexpressions]

Replying to iamtakashi:

Another idea is that we can match the style with the secondary navigation on the left so that both navigations have the same (or very close) patterns.

The only issue with that is that it brings us back to the problem of lessening the distinction of the current page. I agree on the low contrast though.

I think our best option is probably to restore the original behavior but switch to the lightest accent color on hover (matching the sidebar, and reverting to the black background color), or possibly trying the gray text color against the black background (which I believe is the last option we haven't tried yet).

Sidenote: 25220.hover-state-fixes.2.diff​ is ready to go in, and grabs another bad hover state I missed earlier.

[lancewillett]

In 25730:

Twenty Fourteen: accent color fixes, props celloexpressions. See #25220.

• Try a darker text color for header nave submenu items on hover (matching site description color).
• Fix a bad hover color override.

[lancewillett]

@celloexpressions Can we close this ticket and open new ones as needed?

[celloexpressions]

Remaining items left:

• Come up with better names for accent_lighter and accent_much_lighter. The variables don't matter much, but those labels are also used for the theme_mods.
• Look into applying the primary accent color to links and highlight color in TinyMCE.
• Remove the ability to change the "header text color" -- which is really only the site title text color. See #25540

First item should definitely happen here, if we're going to adjust it. Second item could be a new ticket, I suppose, but it's part of the original scope of the feature, I'd say.

[lancewillett]

Replying to celloexpressions:

Remaining items left:

• Come up with better names for accent_lighter and accent_much_lighter. The variables don't matter much, but those labels are also used for the theme_mods.

I'm fine with the variable names. Let's not worry about it.

• Look into applying the primary accent color to links and highlight color in TinyMCE.

Can you make a new ticket?

• Remove the ability to change the "header text color" -- which is really only the site title text color. See #25540

I think it's OK to leave that as-is.