-
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
Migrate existing theme supports to configure the editor to theme.json configs #24761
Changes from 2 commits
442b296
eafbb8d
2c911ae
cc0465e
cb501c1
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 |
---|---|---|
|
@@ -127,6 +127,9 @@ | |
"features": { | ||
"typography": { | ||
"dropCap": false | ||
}, | ||
"colors": { | ||
"allowCustom": true | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -603,6 +603,11 @@ function gutenberg_experimental_global_styles_get_editor_features( $config ) { | |
return array(); | ||
} | ||
|
||
// Deprecated theme supports. | ||
if ( null !== get_theme_support( 'disable-custom-colors' ) ) { | ||
$config['global']['features']['colors']['allowCustom'] = ! get_theme_support( 'disable-custom-colors' ); | ||
} | ||
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 where we added the backward compatibility support for the deprecated theme support flags. 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. If the global features config is empty the previous condition will return:
And in that case, we will not pass the flag value. Should we handle this case? 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. Should be fixed. |
||
|
||
return $config['global']['features']; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,13 @@ import { useSelect } from '@wordpress/data'; | |
*/ | ||
import { useBlockEditContext } from '../block-edit'; | ||
|
||
const deprecatedFlags = { | ||
'colors.allowCustom': ( settings ) => | ||
settings.disableCustomColors === undefined | ||
? undefined | ||
: ! settings.disableCustomColors, | ||
}; | ||
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 where we say which existing block setting maps to which feature path (backward compatibility for block editor settings) |
||
|
||
/** | ||
* Hook that retrieves the setting for the given editor feature. | ||
* It works with nested objects using by finding the value at path. | ||
|
@@ -28,22 +35,23 @@ import { useBlockEditContext } from '../block-edit'; | |
*/ | ||
export default function useEditorFeature( featurePath ) { | ||
const { name: blockName } = useBlockEditContext(); | ||
const path = `__experimentalFeatures.${ featurePath }`; | ||
|
||
const setting = useSelect( | ||
( select ) => { | ||
const { getBlockSupport } = select( 'core/blocks' ); | ||
|
||
const blockSupportValue = getBlockSupport( blockName, path ); | ||
if ( blockSupportValue !== undefined ) { | ||
return blockSupportValue; | ||
} | ||
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 don't think this hook should concern itself with 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 has introduced a regression by which the dropCap control is no longer shown for paragraph #24930 This is also related to this conversation. I think we should merge the fix above so it makes it to this week's release, and then we can figure out how to settle the info coming from block.json, core's theme.json, and theme's theme.json. |
||
|
||
const path = `__experimentalFeatures.${ featurePath }`; | ||
const { getSettings } = select( 'core/block-editor' ); | ||
const settings = getSettings(); | ||
|
||
const deprecatedSettingsValue = deprecatedFlags[ featurePath ] | ||
? deprecatedFlags[ featurePath ]( settings ) | ||
: undefined; | ||
|
||
return get( getSettings(), path ); | ||
if ( deprecatedSettingsValue !== undefined ) { | ||
return deprecatedSettingsValue; | ||
} | ||
return get( settings, path ); | ||
}, | ||
[ blockName, path ] | ||
[ blockName, featurePath ] | ||
); | ||
|
||
return setting; | ||
|
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.
I'm assuming this file is where we define if the feature is opt-in or opt-out. (Default value provided by Core).
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.
Also by reading this, I wonder if it makes more sense to avoid the separation between the top-level "presets" and "features" and just combine them because these are used in conjunction in general.
Components will do things 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.
Alternatively we can provide a separate hook
useEditorPreset( 'colors' )
but it's a bit weird.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.
It's a bit weird to me to see the prefix "allow" given cases like "dropCap" when it is implied. I think just
custom
would be fine.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.
Removed the prefix.
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.
Updated #20588 by adding a task to look into either creating a
usePreset('colors')
hook or integrate into the existinguseEditorFeature('color.preset')
.