-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
Color Palette support setup with single array argument (backwards compatible) #7619
Changes from all commits
dc0ade2
8e0930a
1465a4b
89e7254
9c5a526
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1147,19 +1147,28 @@ function gutenberg_editor_scripts_and_styles( $hook ) { | |
// Initialize the editor. | ||
$gutenberg_theme_support = get_theme_support( 'gutenberg' ); | ||
$align_wide = get_theme_support( 'align-wide' ); | ||
$color_palette = get_theme_support( 'editor-color-palette' ); | ||
$color_palette = current( (array) get_theme_support( 'editor-color-palette' ) ); | ||
|
||
// Backcompat for Color Palette set as multiple parameters. | ||
if ( is_string( $color_palette ) || isset( $color_palette['color'] ) ) { | ||
$color_palette = get_theme_support( 'editor-color-palette' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a solid implementation of BC, just a minor detail I think we should improve: I don't like calling Then the if clause could check One thing I'm not sure about, why is there an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @felixarntz array(1) {
[0] => bool(false)
} The This code should do the trick though: $color_palette = (array) get_theme_support( 'editor-color-palette' );
// Backcompat for Color Palette set as multiple parameters.
if ( is_string( $color_palette[0] ) || isset( $color_palette[0]['color'] ) ) {
_doing_it_wrong(
'add_theme_support()',
__( 'Setting colors using multiple parameters is deprecated. Please pass a single parameter with an array of colors. See https://wordpress.org/gutenberg/handbook/extensibility/theme-support/ for details.', 'gutenberg' ),
'3.4.0'
);
} else {
$color_palette = current( $color_palette );
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @felixarntz I've went ahead and created #7695 addressing your concerns. Thanks for pointing this out! |
||
_doing_it_wrong( | ||
'add_theme_support()', | ||
__( 'Setting colors using multiple parameters is deprecated. Please pass a single parameter with an array of colors. See https://wordpress.org/gutenberg/handbook/extensibility/theme-support/ for details.', 'gutenberg' ), | ||
'3.4.0' | ||
); | ||
} | ||
|
||
// Backcompat for Color Palette set through `gutenberg` array. | ||
if ( empty( $color_palette ) && ! empty( $gutenberg_theme_support[0]['colors'] ) ) { | ||
$color_palette = $gutenberg_theme_support[0]['colors']; | ||
} | ||
|
||
if ( ! empty( $gutenberg_theme_support ) ) { | ||
wp_add_inline_script( | ||
'wp-edit-post', | ||
'console.warn( "' . | ||
__( 'Adding theme support using the `gutenberg` array is deprecated. See https://wordpress.org/gutenberg/handbook/extensibility/theme-support/ for details.', 'gutenberg' ) . | ||
'");' | ||
_doing_it_wrong( | ||
'add_theme_support()', | ||
__( 'Adding theme support using the `gutenberg` array is deprecated. See https://wordpress.org/gutenberg/handbook/extensibility/theme-support/ for details.', 'gutenberg' ), | ||
'3.4.0' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 for changing this to using |
||
); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I guess the usage right, that you want to create the array somewhat programatically and just pass a variable to
add_theme_support
?All in all I think it is a good method of doing things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Luehrsen I won't speak for @webmandesign but that's what we'd like to do. Also makes it a little easier to re-use the color values in other contexts, e.g. auto-generating inline CSS for the classes, etc.
(something like…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I like the approach, I would probably have done that with
get_theme_support()
. I think that would add consistency throughout the themes and coding standards, as I would just have to search for 'editor-color-palette' to see all code that handles the theme colors.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Luehrsen That works as well! My code was just an example — but having a consistent input and output format makes sense I think, either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Luehrsen Yes, that's correct, I want to create the array programatically and then just pass it into
add_theme_support
. I think that's the best approach (right now I can't think of any otheradd_theme_support
feature being passed with multiple arguments nowadays, actually).