Skip to content
This repository has been archived by the owner on Jan 10, 2018. It is now read-only.

Custom colors first pass #167

Merged
merged 21 commits into from
Oct 14, 2016
Merged

Conversation

celloexpressions
Copy link
Contributor

See #116.

Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a couple of comments for spaces after commas

/* Permalink - use to edit and share this gradient: http://colorzilla.com/gradient-editor/#000000+0,000000+100&0+0,0.3+100 */
background: -moz-linear-gradient(top, rgba(255,255,255,0) 0%, rgba(255,255,255,0.3) 100%); /* FF3.6-15 */
background: -webkit-linear-gradient(top, rgba(255,255,255,0) 0%,rgba(255,255,255,0.3) 100%); /* Chrome10-25,Safari5.1-6 */
background: linear-gradient(to bottom, rgba(255,255,255,0) 0%,rgba(255,255,255,0.3) 100%); /* W3C, IE10+, FF16+, Chrome26+, Opera12+, Safari7+ */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected single space after commas on the above 3 lines

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a direct copy/paste from style.css, but swapping out the colors; it will need to be updated there as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, #169 gets all the coding standards sorted for style.css

/* Permalink - use to edit and share this gradient: http://colorzilla.com/gradient-editor/#000000+0,000000+100&0+0,0.3+75 */
background: -moz-linear-gradient(top, rgba(255,255,255,0) 0%, rgba(255,255,255,0.3) 75%, rgba(255,255,255,0.3) 100%); /* FF3.6-15 */
background: -webkit-linear-gradient(top, rgba(255,255,255,0) 0%,rgba(255,255,255,0.3) 75%,rgba(255,255,255,0.3) 100%); /* Chrome10-25,Safari5.1-6 */
background: linear-gradient(to bottom, rgba(255,255,255,0) 0%,rgba(255,255,255,0.3) 75%,rgba(255,255,255,0.3) 100%); /* W3C, IE10+, FF16+, Chrome26+, Opera12+, Safari7+ */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected single space after commas on the above 3 lines

Note that this was copied from style.css.
function twentyseventeen_sanitize_colorscheme( $input ) {
$valid = array( 'light', 'dark', 'custom' );

if ( array_key_exists( $input, $valid ) ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in_array, since $valid is not a keyed array. Right now this doesn't save colorscheme.

@celloexpressions
Copy link
Contributor Author

The one remaining Travis CI failure is for line endings, which I'm unable to change from here. Someone will need to fix that on or after merge.

This should be ready to go. After the initial merge, someone will need to go through and merge all changes to style.css after October 4th that impact colors in any way back to color-patterns.php and colors-dark.css. All future commits to style.css that touch colors need to consider the color files as well now too. This applies to any color, including white or black.

Per @melchoyce, switching #999 to #bbb. Assuming #aaa will need to go to #ccc in the process so that it doesn't become darker than the other pattern.
The base colorscheme doesn't specify this, so it wasn't being inverted but the text color was. Selectors copied from where the text color is inverted.
Adjust saturation to 50% per @melchoyce, and reduce the saturation on body text (base: WordPress#333) by 20% of the base saturation (should come out to 40%). Needs testing.
@davidakennedy davidakennedy merged commit 62f22e3 into WordPress:master Oct 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants