Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dark black header and footer variation #307

Closed
wants to merge 5 commits into from
Closed

Add dark black header and footer variation #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51e63d1
Add 'white on dark black' variation to header and footer to support c…
Dec 8, 2022
c91fa86
Change wp-block-navigation container to inherit background color to e…
Dec 8, 2022
80a6d29
Fix dark black value typo
Dec 8, 2022
129155f
Relabel variations to be the style name for consistency. The values f…
Dec 9, 2022
c053aa6
Remove extra characters
Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions mu-plugins/blocks/global-header-footer/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,9 @@ function get_container_classes( $color_scheme ) {
case 'black-on-white':
$classes .= ' has-charcoal-2-color has-white-background-color';
break;
case 'white-on-dark-black':
$classes .= ' has-white-color has-charcoal-1-background-color';
break;
case 'white-on-black':
default:
$classes .= ' has-white-color has-charcoal-2-background-color';
Expand Down
2 changes: 1 addition & 1 deletion mu-plugins/blocks/global-header-footer/postcss/header/menu.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@
& .wp-block-navigation__container {

@media (--tablet) {
background: var(--wp-global-header--background-color);
background: inherit;
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than changing this, you can add another "case" to _common.pcss to override --wp-global-header--background-color. See how it's done for the other styles here:

wporg-mu-plugins/mu-plugins/blocks/global-header-footer/postcss/_common.pcss

Lines 67 to 90 in 2ca5e5b

--wp-global-header--background-color: var(--wp--preset--color--charcoal-2);
--wp-global-header--background-color--hover: var(--wp--preset--color--charcoal-5);
--wp-global-header--link-color: var(--wp--preset--color--white);
--wp-global-header--link-color--active: var(--wp--preset--color--blueberry-2);
--wp-global-header--text-color: var(--wp--preset--color--white);
}
.has-charcoal-2-color {
--wp-global-header--link-color: var(--wp--preset--color--charcoal-2);
--wp-global-header--link-color--active: var(--wp--preset--color--deep-blueberry);
--wp-global-header--text-color: var(--wp--preset--color--charcoal-2);
}
.has-blueberry-1-background-color {
--wp-global-header--background-color: var(--wp--preset--color--blueberry-1);
--wp-global-header--background-color--hover: var(--wp--preset--color--deep-blueberry);
--wp-global-header--link-color--active: var(--wp--preset--color--white);
}
.has-white-background-color {
--wp-global-header--background-color: var(--wp--preset--color--white);
--wp-global-header--background-color--hover: var(--wp--preset--color--light-grey-2);
}

}
}
}
Expand Down
7 changes: 6 additions & 1 deletion mu-plugins/blocks/global-header-footer/src/footer/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
"style": {
"type": "string",
"default": "white-on-black",
"enum": [ "white-on-black", "black-on-white", "white-on-blue" ]
"enum": [
"white-on-black",
"white-on-dark-black",
"black-on-white",
"white-on-blue"
]
}
},
"textdomain": "wporg",
Expand Down
5 changes: 3 additions & 2 deletions mu-plugins/blocks/global-header-footer/src/footer/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import { useBlockProps } from '@wordpress/block-editor';
import metadata from './block.json';

const variations = [
{ label: __( 'White on black (default)', 'wporg' ), value: 'white-on-black' },
{ label: __( 'White on charcoal-1', 'wporg' ), value: 'white-on-dark-black' },
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing is using this value yet, correct? So it's not too late to try to improve the naming? For me the range of values is where it's going to get messy if/when we add more variations...

Suggested change
{ label: __( 'White on charcoal-1', 'wporg' ), value: 'white-on-dark-black' },
{ label: __( 'White on charcoal-1', 'wporg' ), value: 'white-on-charcoal-1' },

Also I wonder if it's still manageable to migrate the existing usages to new values while we are still relatively early in the overall redesign. All the pages are saved as patterns in source control in the various repos, so we could replace eg. white-on-black with white-on-charcoal-2, and deploy all at once.

Or maybe I'm worrying too much about this... feel free to give me a reality check 😂

Copy link
Contributor

Choose a reason for hiding this comment

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

For white-on-black
1 usage in wporg-main-2022
2 usages in wporg-showcase-2022
2 usages in wporg-parent-2021
Several in this repo

Copy link
Contributor

Choose a reason for hiding this comment

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

Usages are even better for white-on-blue

Copy link
Author

Choose a reason for hiding this comment

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

@adamwoodnz Perhaps for a migration strategy we could do the following:

  1. Add both the legacy and new values to the switch statement so we can support both the legacy and new new names.
  2. Create an issue so we can update the content someday.
  3. Eventually remove the support from the switch statements.

What do you think? I'm going to hold off until we find out whether we are going to use charcoal-1 or charcoal-2, but I think it's a good idea to implement someday.

Copy link
Contributor

@adamwoodnz adamwoodnz Dec 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I was a bit hesitant to suggest we reuse the same color. I think that makes sense for simplicity's sake.

Having both values in the switch could work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would a deprecation work? It might only apply to JS blocks.

If so, I think the proposed migration strategy sounds good.

{ label: __( 'White on charcoal-2 (default)', 'wporg' ), value: 'white-on-black' },
{ label: __( 'White on blueberry-1', 'wporg' ), value: 'white-on-blue' },
{ label: __( 'Black on white', 'wporg' ), value: 'black-on-white' },
{ label: __( 'White on blue', 'wporg' ), value: 'white-on-blue' },
];

const Edit = ( { attributes } ) => (
Expand Down
7 changes: 6 additions & 1 deletion mu-plugins/blocks/global-header-footer/src/header/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
"style": {
"type": "string",
"default": "white-on-black",
"enum": [ "white-on-black", "black-on-white", "white-on-blue" ]
"enum": [
"white-on-black",
"white-on-dark-black",
"black-on-white",
"white-on-blue"
]
}
},
"textdomain": "wporg",
Expand Down
5 changes: 3 additions & 2 deletions mu-plugins/blocks/global-header-footer/src/header/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import { useBlockProps } from '@wordpress/block-editor';
import metadata from './block.json';

const variations = [
{ label: __( 'White on black (default)', 'wporg' ), value: 'white-on-black' },
{ label: __( 'White on charcoal-1', 'wporg' ), value: 'white-on-dark-black' },
{ label: __( 'White on charcoal-2 (default)', 'wporg' ), value: 'white-on-black' },
{ label: __( 'White on blueberry-1', 'wporg' ), value: 'white-on-blue' },
{ label: __( 'Black on white', 'wporg' ), value: 'black-on-white' },
{ label: __( 'White on blue', 'wporg' ), value: 'white-on-blue' },
];

const Edit = ( { attributes } ) => (
Expand Down