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

Allow themes to provide empty values for color.duotone and spacing.units #33280

Merged
merged 6 commits into from
Jul 8, 2021
Merged

Allow themes to provide empty values for color.duotone and spacing.units #33280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
22b3bd3
Add tests for empty incoming presets
oandregal Jul 8, 2021
456a7be
Add tests for null incoming presets
oandregal Jul 8, 2021
f62c32a
Empty arrays are valid input data that need to be merged as well
oandregal Jul 8, 2021
05fc7b2
Layout.units were not being considered for merging
oandregal Jul 8, 2021
fd1fc57
Add tests for layout.units
oandregal Jul 8, 2021
2d19440
Substitute layout.units per spacing.units
oandregal Jul 8, 2021
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
4 changes: 2 additions & 2 deletions lib/class-wp-theme-json-gutenberg.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1086,8 +1086,8 @@ public function merge( $incoming ) {
foreach ( $nodes as $metadata ) {
foreach ( $to_replace as $property_path ) {
$path = array_merge( $metadata['path'], $property_path );
$node = _wp_array_get( $incoming_data, $path, array() );
if ( ! empty( $node ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

We already a similar bug before with using empty. I grepped for empty on this file to make sure we don't have additional issues. I found other suspicious case:

			if ( ! empty( $escaped_preset ) ) {
				gutenberg_experimental_set( $output, $preset_metadata['path'], $escaped_preset );
			}

It is related to user escaping does not affect 5.8 I will confirm if this is indeed another bug and if yes fix it in another PR.

$node = _wp_array_get( $incoming_data, $path, null );
if ( isset( $node ) ) {
gutenberg_experimental_set( $this->theme_json, $path, $node );
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/letter-spacing-control/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import useSetting from '../../components/use-setting';
*/
export default function LetterSpacingControl( { value, onChange } ) {
const units = useCustomUnits( {
availableUnits: useSetting( 'layout.units' ) || [ 'px', 'em', 'rem' ],
availableUnits: useSetting( 'spacing.units' ) || [ 'px', 'em', 'rem' ],
defaultValues: { px: '2', em: '.2', rem: '.2' },
} );
return (
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/hooks/border-width.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const BorderWidthEdit = ( props ) => {
};

const units = useCustomUnits( {
availableUnits: useSetting( 'layout.units' ) || [ 'px', 'em', 'rem' ],
availableUnits: useSetting( 'spacing.units' ) || [ 'px', 'em', 'rem' ],
} );

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/hooks/layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function LayoutPanel( { setAttributes, attributes } ) {
}, [] );

const units = useCustomUnits( {
availableUnits: useSetting( 'layout.units' ) || [
availableUnits: useSetting( 'spacing.units' ) || [
'%',
'px',
'em',
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/column/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function ColumnEdit( {
} );

const units = useCustomUnits( {
availableUnits: useSetting( 'layout.units' ) || [
availableUnits: useSetting( 'spacing.units' ) || [
'%',
'px',
'em',
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/column/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function ColumnEdit( {
const [ widthUnit, setWidthUnit ] = useState( valueUnit || '%' );

const units = useCustomUnits( {
availableUnits: useSetting( 'layout.units' ) || [
availableUnits: useSetting( 'spacing.units' ) || [
'%',
'px',
'em',
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/columns/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function ColumnsEditContainer( {
const { width } = sizes || {};

const units = useCustomUnits( {
availableUnits: useSetting( 'layout.units' ) || [
availableUnits: useSetting( 'spacing.units' ) || [
'%',
'px',
'em',
Expand Down
197 changes: 197 additions & 0 deletions phpunit/class-wp-theme-json-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,203 @@ public function test_merge_incoming_data() {
$this->assertEqualSetsWithIndex( $expected, $actual );
}

public function test_merge_incoming_data_empty_presets() {
$theme_json = new WP_Theme_JSON_Gutenberg(
array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'duotone' => array(
array(
'slug' => 'value',
'colors' => array( 'red', 'green' ),
),
),
'gradients' => array(
array(
'slug' => 'gradient',
'gradient' => 'gradient',
),
),
'palette' => array(
array(
'slug' => 'red',
'color' => 'red',
),
),
),
'spacing' => array(
'units' => array( 'px', 'em' ),
),
'typography' => array(
'fontSizes' => array(
array(
'slug' => 'size',
'value' => 'size',
),
),
),
),
)
);

$theme_json->merge(
new WP_Theme_JSON_Gutenberg(
array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'duotone' => array(),
'gradients' => array(),
'palette' => array(),
),
'spacing' => array(
'units' => array(),
),
'typography' => array(
'fontSizes' => array(),
),
),
)
)
);

$actual = $theme_json->get_raw_data();
$expected = array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'duotone' => array(),
'gradients' => array(
'theme' => array(),
),
'palette' => array(
'theme' => array(),
),
),
'spacing' => array(
'units' => array(),
),
'typography' => array(
'fontSizes' => array(
'theme' => array(),
),
),
),
);

$this->assertEqualSetsWithIndex( $expected, $actual );
}

public function test_merge_incoming_data_null_presets() {
$theme_json = new WP_Theme_JSON_Gutenberg(
array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'duotone' => array(
array(
'slug' => 'value',
'colors' => array( 'red', 'green' ),
),
),
'gradients' => array(
array(
'slug' => 'gradient',
'gradient' => 'gradient',
),
),
'palette' => array(
array(
'slug' => 'red',
'color' => 'red',
),
),
),
'spacing' => array(
'units' => array( 'px', 'em' ),
),
'typography' => array(
'fontSizes' => array(
array(
'slug' => 'size',
'value' => 'size',
),
),
),
),
)
);

$theme_json->merge(
new WP_Theme_JSON_Gutenberg(
array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'custom' => false,
),
'spacing' => array(
'customMargin' => false,
),
'typography' => array(
'customLineHeight' => false,
),
),
)
)
);

$actual = $theme_json->get_raw_data();
$expected = array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'custom' => false,
'duotone' => array(
array(
'slug' => 'value',
'colors' => array( 'red', 'green' ),
),
),
'gradients' => array(
'theme' => array(
array(
'slug' => 'gradient',
'gradient' => 'gradient',
),
),
),
'palette' => array(
'theme' => array(
array(
'slug' => 'red',
'color' => 'red',
),
),
),
),
'spacing' => array(
'customMargin' => false,
'units' => array( 'px', 'em' ),
),
'typography' => array(
'customLineHeight' => false,
'fontSizes' => array(
'theme' => array(
array(
'slug' => 'size',
'value' => 'size',
),
),
),
),
),
);

$this->assertEqualSetsWithIndex( $expected, $actual );
}

function test_remove_insecure_properties_removes_unsafe_styles() {
$actual = WP_Theme_JSON_Gutenberg::remove_insecure_properties(
array(
Expand Down