-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Centralize style & support mappings for blocks #25185
Changes from 3 commits
d51a5de
e78fd5a
b4b50c8
7c9560a
cc79a34
6821e59
4b882e4
68f8e6c
9d1b159
391336a
6b635ad
7a44eaa
b6229f7
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 |
---|---|---|
|
@@ -262,26 +262,50 @@ function gutenberg_experimental_global_styles_get_theme() { | |
} | ||
|
||
/** | ||
* Returns the style features a particular block supports. | ||
* Return how the style property is structured. | ||
* | ||
* @param array $supports The block supports array. | ||
* @return array Style property structure. | ||
*/ | ||
function gutenberg_experimental_global_styles_get_style_property() { | ||
return array( | ||
'line-height' => array( 'typography', 'lineHeight' ), | ||
'font-size' => array( 'typography', 'fontSize' ), | ||
'background' => array( 'color', 'gradient' ), | ||
'background-color' => array( 'color', 'background' ), | ||
'color' => array( 'color', 'text' ), | ||
'--wp--style--color--link' => array( 'color', 'link' ), | ||
); | ||
} | ||
|
||
/** | ||
* Return how the support keys are structured. | ||
* | ||
* @return array Style features supported by the block. | ||
* @return array Support keys structure. | ||
*/ | ||
function gutenberg_experimental_global_styles_get_supported_styles( $supports ) { | ||
$style_features = array( | ||
function gutenberg_experimental_global_styles_get_support_keys() { | ||
return array( | ||
'--wp--style--color--link' => array( '__experimentalColor', 'linkColor' ), | ||
'background-color' => array( '__experimentalColor' ), | ||
'backgroundColor' => array( '__experimentalColor' ), | ||
'background' => array( '__experimentalColor', 'gradients' ), | ||
'color' => array( '__experimentalColor' ), | ||
'font-size' => array( '__experimentalFontSize' ), | ||
'line-height' => array( '__experimentalLineHeight' ), | ||
'fontSize' => array( '__experimentalFontSize' ), | ||
'lineHeight' => array( '__experimentalLineHeight' ), | ||
); | ||
} | ||
|
||
/** | ||
* Returns the style features a particular block supports. | ||
* | ||
* @param array $supports The block supports array. | ||
* | ||
* @return array Style features supported by the block. | ||
*/ | ||
function gutenberg_experimental_global_styles_get_supported_styles( $supports ) { | ||
$support_keys = gutenberg_experimental_global_styles_get_support_keys(); | ||
$supported_features = array(); | ||
foreach ( $style_features as $style_feature => $path ) { | ||
foreach ( $support_keys as $key => $path ) { | ||
if ( gutenberg_experimental_get( $supports, $path ) ) { | ||
$supported_features[] = $style_feature; | ||
$supported_features[] = $key; | ||
} | ||
} | ||
|
||
|
@@ -385,17 +409,9 @@ function gutenberg_experimental_global_styles_get_block_data() { | |
* @return array Containing a set of css rules. | ||
*/ | ||
function gutenberg_experimental_global_styles_flatten_styles_tree( $styles ) { | ||
$mappings = array( | ||
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. No changes here, only extracted to a separate function. |
||
'line-height' => array( 'typography', 'lineHeight' ), | ||
'font-size' => array( 'typography', 'fontSize' ), | ||
'background' => array( 'color', 'gradient' ), | ||
'background-color' => array( 'color', 'background' ), | ||
'color' => array( 'color', 'text' ), | ||
'--wp--style--color--link' => array( 'color', 'link' ), | ||
); | ||
$mappings = gutenberg_experimental_global_styles_get_style_property(); | ||
|
||
$result = array(); | ||
|
||
foreach ( $mappings as $key => $path ) { | ||
$value = gutenberg_experimental_get( $styles, $path, null ); | ||
if ( null !== $value ) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,3 +10,24 @@ export const DEPRECATED_ENTRY_KEYS = [ | |
'migrate', | ||
'isEligible', | ||
]; | ||
|
||
export const LINK_COLOR = '--wp--style--color--link'; | ||
|
||
export const STYLE_PROPERTY = { | ||
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. I think this is a good place for this kind of info, as it speaks about how the block attributes are structured. However, I don't feel super strongly about this. If this is a blocker I'm fine moving it to any other place. 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. I guess we should prefix the constant with experimental. |
||
[ LINK_COLOR ]: [ 'color', 'link' ], | ||
background: [ 'color', 'gradient' ], | ||
backgroundColor: [ 'color', 'background' ], | ||
color: [ 'color', 'text' ], | ||
fontSize: [ 'typography', 'fontSize' ], | ||
lineHeight: [ 'typography', 'lineHeight' ], | ||
paddingBottom: [ 'spacing', 'padding', 'bottom' ], | ||
paddingLeft: [ 'spacing', 'padding', 'left' ], | ||
paddingRight: [ 'spacing', 'padding', 'right' ], | ||
paddingTop: [ 'spacing', 'padding', 'top' ], | ||
}; | ||
|
||
/* Block supports */ | ||
export const COLOR_SUPPORT_KEY = '__experimentalColor'; | ||
export const FONT_SIZE_SUPPORT_KEY = '__experimentalFontSize'; | ||
export const LINE_HEIGHT_SUPPORT_KEY = '__experimentalLineHeight'; | ||
export const PADDING_SUPPORT_KEY = '__experimentalPadding'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,3 +67,11 @@ export { | |
} from './templates'; | ||
export { default as children } from './children'; | ||
export { default as node } from './node'; | ||
export { | ||
COLOR_SUPPORT_KEY, | ||
FONT_SIZE_SUPPORT_KEY, | ||
LINE_HEIGHT_SUPPORT_KEY, | ||
LINK_COLOR, | ||
PADDING_SUPPORT_KEY, | ||
STYLE_PROPERTY, | ||
} from './constants'; | ||
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. I'm concerned about this personally, it makes it an API and I don't really know whether these keys will remain as is, be combined... Why do we need this to be an API? (specially the 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. mmm, my rationale was that the support keys are actually part of the API. This is why: if we had specific accessors ( For the style keys, I agree with Jorge that we need a single source of truth, as they're both used in block-editor and edit-site. Exporting them here makes more sense from my POV. 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. in which situation we need to access these keys outside the hooks themselves which are part of block-editor package? 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. I reverted the support keys to block-editor. I still think that we should offer a feature list as I mentioned, but I realized that it doesn't seem a good idea to start with the experimental keys. |
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.
These keys were renamed to use the same conventions as client-side (camelCase over kebab-case), making everything more coherent and less computation-demanding.