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.

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

Merged
merged 5 commits into from
Aug 28, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion lib/edit-site-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ function gutenberg_edit_site_init( $hook ) {

$settings = array(
'alignWide' => get_theme_support( 'align-wide' ),
'disableCustomColors' => get_theme_support( 'disable-custom-colors' ),
'disableCustomFontSizes' => get_theme_support( 'disable-custom-font-sizes' ),
'imageSizes' => $available_image_sizes,
'isRTL' => is_rtl(),
Expand All @@ -144,6 +143,7 @@ function gutenberg_edit_site_init( $hook ) {
$settings['fontSizes'] = $font_sizes;
}
$settings['styles'] = gutenberg_get_editor_styles();
$settings = gutenberg_experimental_global_styles_settings( $settings );

// This is so other parts of the code can hook their own settings.
// Example: Global Styles.
Expand Down
3 changes: 3 additions & 0 deletions lib/experimental-default-theme.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@
"features": {
"typography": {
"dropCap": false
},
"colors": {
"allowCustom": true
Copy link
Contributor Author

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).

Copy link
Contributor Author

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:

const colorsPreset = useEditorFeature( 'colors.preset' );
const allowCustomColors = useEditorFeature( 'colors.allowCustom' );

// Render palette with these options.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the prefix.

Copy link
Member

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 existing useEditorFeature('color.preset').

}
}
}
Expand Down
5 changes: 5 additions & 0 deletions lib/global-styles.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

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

If the global features config is empty the previous condition will return:

	if (
		empty( $config['global']['features'] ) ||
		! is_array( $config['global']['features'] )
	) {
		return array();
	}

And in that case, we will not pass the flag value. Should we handle this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed.


return $config['global']['features'];
}

Expand Down
3 changes: 2 additions & 1 deletion lib/navigation-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ function gutenberg_navigation_init( $hook ) {
}

$settings = array(
'disableCustomColors' => get_theme_support( 'disable-custom-colors' ),
'disableCustomFontSizes' => get_theme_support( 'disable-custom-font-sizes' ),
'imageSizes' => $available_image_sizes,
'isRTL' => is_rtl(),
Expand All @@ -76,6 +75,8 @@ function gutenberg_navigation_init( $hook ) {
if ( false !== $font_sizes ) {
$settings['fontSizes'] = $font_sizes;
}

$settings = gutenberg_experimental_global_styles_settings( $settings );

wp_add_inline_script(
'wp-edit-navigation',
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ function gutenberg_widgets_init( $hook ) {

$settings = array_merge(
array(
'disableCustomColors' => get_theme_support( 'disable-custom-colors' ),
'disableCustomFontSizes' => get_theme_support( 'disable-custom-font-sizes' ),
'imageSizes' => $available_image_sizes,
'isRTL' => is_rtl(),
Expand All @@ -90,6 +89,7 @@ function gutenberg_widgets_init( $hook ) {
if ( false !== $font_sizes ) {
$settings['fontSizes'] = $font_sizes;
}
$settings = gutenberg_experimental_global_styles_settings( $settings );

wp_add_inline_script(
'wp-edit-widgets',
Expand Down
1 change: 0 additions & 1 deletion packages/block-editor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,6 @@ _Properties_
- _alignWide_ `boolean`: Enable/Disable Wide/Full Alignments
- _availableLegacyWidgets_ `Array`: Array of objects representing the legacy widgets available.
- _colors_ `Array`: Palette colors
- _disableCustomColors_ `boolean`: Whether or not the custom colors are disabled
- _fontSizes_ `Array`: Available font sizes
- _disableCustomFontSizes_ `boolean`: Whether or not the custom font sizes are disabled
- _imageEditing_ `boolean`: Image Editing settings set to false to disable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,50 @@ import { isEmpty } from 'lodash';
/**
* WordPress dependencies
*/
import { createHigherOrderComponent } from '@wordpress/compose';
import { createHigherOrderComponent, compose } from '@wordpress/compose';
import { withSelect } from '@wordpress/data';

export default createHigherOrderComponent(
withSelect( ( select, ownProps ) => {
const settings = select( 'core/block-editor' ).getSettings();
const colors =
ownProps.colors === undefined ? settings.colors : ownProps.colors;
/**
* Internal dependencies
*/
import useEditorFeature from '../use-editor-feature';

const withDisableCustomColors = createHigherOrderComponent(
( WrappedComponent ) => {
return ( props ) => {
const disableCustomColors = ! useEditorFeature(
'colors.allowCustom'
);

const disableCustomColors =
ownProps.disableCustomColors === undefined
? settings.disableCustomColors
: ownProps.disableCustomColors;
return {
colors,
disableCustomColors,
hasColorsToChoose: ! isEmpty( colors ) || ! disableCustomColors,
return (
<WrappedComponent
{ ...props }
disableCustomColors={
props.disableCustomColors || disableCustomColors
}
/>
);
};
} ),
},
'withDisableCustomColors'
);

export default createHigherOrderComponent(
compose(
withDisableCustomColors,
withSelect( ( select, ownProps ) => {
const settings = select( 'core/block-editor' ).getSettings();
const colors =
ownProps.colors === undefined
? settings.colors
: ownProps.colors;

return {
colors,
hasColorsToChoose:
! isEmpty( colors ) || ! ownProps.disableCustomColors,
};
} )
),
'withColorContext'
);
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { useSelect } from '@wordpress/data';
*/
import { getColorObjectByColorValue } from '../colors';
import { __experimentalGetGradientObjectByGradientValue } from '../gradients';
import useEditorFeature from '../use-editor-feature';

// translators: first %s: the color name or value (e.g. red or #ff0000)
const colorIndicatorAriaLabel = __( '(Color: %s)' );
Expand Down Expand Up @@ -177,6 +178,10 @@ function ColorGradientControlSelect( props ) {
const settings = select( 'core/block-editor' ).getSettings();
return pick( settings, colorsAndGradientKeys );
} );
colorGradientSettings.disableCustomColors = ! useEditorFeature(
'colors.allowCustom'
);

return (
<ColorGradientControlInner
{ ...{ ...colorGradientSettings, ...props } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { useSelect } from '@wordpress/data';
import ColorGradientControl from './control';
import { getColorObjectByColorValue } from '../colors';
import { __experimentalGetGradientObjectByGradientValue } from '../gradients';
import useEditorFeature from '../use-editor-feature';

// translators: first %s: The type of color or gradient (e.g. background, overlay...), second %s: the color name or value (e.g. red or #ff0000)
const colorIndicatorAriaLabel = __( '(%s: color %s)' );
Expand Down Expand Up @@ -151,6 +152,9 @@ const PanelColorGradientSettingsSelect = ( props ) => {
const settings = select( 'core/block-editor' ).getSettings();
return pick( settings, colorsAndGradientKeys );
} );
colorGradientSettings.disableCustomColors = ! useEditorFeature(
'colors.allowCustom'
);
return (
<PanelColorGradientSettingsInner
{ ...{ ...colorGradientSettings, ...props } }
Expand Down
28 changes: 18 additions & 10 deletions packages/block-editor/src/components/use-editor-feature/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
Expand All @@ -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;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this hook should concern itself with getBlockSupport. getBlockSupport is only used to say that a block support a given feature and should be used directly in the "hooks" and not in useEditorFeature which responsibility is more related to block-editor settings and theme.json.

Copy link
Member

Choose a reason for hiding this comment

The 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;
Expand Down
1 change: 0 additions & 1 deletion packages/block-editor/src/store/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export const PREFERENCES_DEFAULTS = {
* @property {boolean} alignWide Enable/Disable Wide/Full Alignments
* @property {Array} availableLegacyWidgets Array of objects representing the legacy widgets available.
* @property {Array} colors Palette colors
* @property {boolean} disableCustomColors Whether or not the custom colors are disabled
* @property {Array} fontSizes Available font sizes
* @property {boolean} disableCustomFontSizes Whether or not the custom font sizes are disabled
* @property {boolean} imageEditing Image Editing settings set to false to disable.
Expand Down
12 changes: 7 additions & 5 deletions packages/format-library/src/text-color/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import { get, isEmpty } from 'lodash';
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import { useCallback, useMemo, useState } from '@wordpress/element';
import { RichTextToolbarButton } from '@wordpress/block-editor';
import {
RichTextToolbarButton,
__experimentalUseEditorFeature as useEditorFeature,
} from '@wordpress/block-editor';
import { Icon, textColor as textColorIcon } from '@wordpress/icons';
import { removeFormat } from '@wordpress/rich-text';

Expand All @@ -24,7 +27,8 @@ const title = __( 'Text Color' );
const EMPTY_ARRAY = [];

function TextColorEdit( { value, onChange, isActive, activeAttributes } ) {
const { colors, disableCustomColors } = useSelect( ( select ) => {
const allowCustomControl = useEditorFeature( 'colors.allowCustom' );
const { colors } = useSelect( ( select ) => {
const blockEditorSelect = select( 'core/block-editor' );
let settings;
if ( blockEditorSelect && blockEditorSelect.getSettings ) {
Expand All @@ -34,7 +38,6 @@ function TextColorEdit( { value, onChange, isActive, activeAttributes } ) {
}
return {
colors: get( settings, [ 'colors' ], EMPTY_ARRAY ),
disableCustomColors: settings.disableCustomColors,
};
} );
const [ isAddingColor, setIsAddingColor ] = useState( false );
Expand All @@ -54,8 +57,7 @@ function TextColorEdit( { value, onChange, isActive, activeAttributes } ) {
};
}, [ value, colors ] );

const hasColorsToChoose =
! isEmpty( colors ) || disableCustomColors !== true;
const hasColorsToChoose = ! isEmpty( colors ) || ! allowCustomControl;
if ( ! hasColorsToChoose && ! isActive ) {
return null;
}
Expand Down