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

Jump to bottom

Fix link color style property name in global styles #24296

Merged
merged 2 commits into from
Aug 2, 2020
Merged

Fix link color style property name in global styles #24296

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ab57ad
Fix link color style property name
oandregal Jul 30, 2020
91d20c7
Update prop name used to check support
oandregal Aug 2, 2020
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
14 changes: 7 additions & 7 deletions lib/global-styles.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,13 @@ function gutenberg_experimental_global_styles_get_theme() {
*/
function gutenberg_experimental_global_styles_get_supported_styles( $supports ) {
$style_features = array(
'color' => array( '__experimentalColor' ),
'background-color' => array( '__experimentalColor' ),
'background' => array( '__experimentalColor', 'gradients' ),
'link-color' => array( '__experimentalColor', 'linkColor' ),
'line-height' => array( '__experimentalLineHeight' ),
'font-size' => array( '__experimentalFontSize' ),
'block-align' => array( 'align' ),
'--wp--style--color--link' => array( '__experimentalColor', 'linkColor' ),
'background-color' => array( '__experimentalColor' ),
'background' => array( '__experimentalColor', 'gradients' ),
'block-align' => array( 'align' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't make sense for me, though it's not introduced in this particular PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've noticed it as well, made a note to review later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Heya! Sorry, that was my doing... does block align not belong in this function in general?

Its being used for checking if a block as align hook support here:

gutenberg/lib/blocks.php

Line 421 in 1087121

if ( in_array( 'block-align', $supports, true ) ) {

but I can remove it from this function and just check if the block_type has the align flag there instead if that makes more sense? I currently have #24300 open in this area to add support for default values and can update this there if needed. (Also I wouldn't mind any input/review on that PR, I didn't realize we missed the case for default values when we originally added those functions in #23007.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Riad already proposed an alternative at #24351

'color' => array( '__experimentalColor' ),
'font-size' => array( '__experimentalFontSize' ),
'line-height' => array( '__experimentalLineHeight' ),
);

$supported_features = array();
Expand Down