From d1727d207787fa9af0711c8c456bd1d5f7a2bc83 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 5 Apr 2023 18:40:34 +1000 Subject: [PATCH 1/4] Fix typo throwing error --- packages/block-editor/src/hooks/border.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/hooks/border.js b/packages/block-editor/src/hooks/border.js index de6aa349898645..2460a47710699e 100644 --- a/packages/block-editor/src/hooks/border.js +++ b/packages/block-editor/src/hooks/border.js @@ -84,7 +84,7 @@ function styleToAttributes( style ) { const borderColorValue = style?.border?.color; const borderColorSlug = borderColorValue?.startsWith( 'var:preset|color|' ) - ? borderColorSlug.substring( 'var:preset|color|'.length ) + ? borderColorValue.substring( 'var:preset|color|'.length ) : undefined; const updatedStyle = { ...style }; updatedStyle.border = { From 59654511f9b934c5087a8cb40da533bc01499058 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 5 Apr 2023 18:41:00 +1000 Subject: [PATCH 2/4] Fix on border change handling in border panel --- .../components/global-styles/border-panel.js | 68 +++++++++++-------- .../components/global-styles/border-panel.js | 1 + 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/packages/block-editor/src/components/global-styles/border-panel.js b/packages/block-editor/src/components/global-styles/border-panel.js index 83979b2845c4be..0549e213e44894 100644 --- a/packages/block-editor/src/components/global-styles/border-panel.js +++ b/packages/block-editor/src/components/global-styles/border-panel.js @@ -111,6 +111,7 @@ export default function BorderPanel( { settings, panelId, defaultControls = DEFAULT_CONTROLS, + forceSplitBorders = false, } ) { const colors = useColorsPerOrigin( settings ); const decodeValue = ( rawValue ) => @@ -187,38 +188,45 @@ export default function BorderPanel( { // Ensure we have a visible border style when a border width or // color is being selected. const newBorderWithStyle = applyAllFallbackStyles( newBorder ); + let updatedBorder = newBorderWithStyle; - // As we can't conditionally generate styles based on if other - // style properties have been set we need to force split border - // definitions for user set border styles. Border radius is derived - // from the same property i.e. `border.radius` if it is a string - // that is used. The longhand border radii styles are only generated - // if that property is an object. - // - // For borders (color, style, and width) those are all properties on - // the `border` style property. This means if the theme.json defined - // split borders and the user condenses them into a flat border or - // vice-versa we'd get both sets of styles which would conflict. - const updatedBorder = ! hasSplitBorders( newBorderWithStyle ) - ? { - top: newBorderWithStyle, - right: newBorderWithStyle, - bottom: newBorderWithStyle, - left: newBorderWithStyle, - } - : { - color: null, - style: null, - width: null, - ...newBorderWithStyle, - }; + if ( forceSplitBorders ) { + // As Global Styles can't conditionally generate styles based on if + // other style properties have been set, we need to force split + // border definitions for user set global border styles. Border + // radius is derived from the same property i.e. `border.radius` if + // it is a string that is used. The longhand border radii styles are + // only generated if that property is an object. + // + // For borders (color, style, and width) those are all properties on + // the `border` style property. This means if the theme.json defined + // split borders and the user condenses them into a flat border or + // vice-versa we'd get both sets of styles which would conflict. + updatedBorder = ! hasSplitBorders( newBorderWithStyle ) + ? { + top: newBorderWithStyle, + right: newBorderWithStyle, + bottom: newBorderWithStyle, + left: newBorderWithStyle, + } + : { + color: null, + style: null, + width: null, + ...newBorderWithStyle, + }; + } - [ 'top', 'right', 'bottom', 'left' ].forEach( ( side ) => { - updatedBorder[ side ] = { - ...updatedBorder[ side ], - color: encodeColorValue( updatedBorder[ side ]?.color ), - }; - } ); + if ( hasSplitBorders( updatedBorder ) ) { + [ 'top', 'right', 'bottom', 'left' ].forEach( ( side ) => { + updatedBorder[ side ] = { + ...updatedBorder[ side ], + color: encodeColorValue( updatedBorder[ side ]?.color ), + }; + } ); + } else { + updatedBorder.color = encodeColorValue( updatedBorder.color ); + } // As radius is maintained separately to color, style, and width // maintain its value. Undefined values here will be cleaned when diff --git a/packages/edit-site/src/components/global-styles/border-panel.js b/packages/edit-site/src/components/global-styles/border-panel.js index 0926e6edd03386..d90a12f0b03d33 100644 --- a/packages/edit-site/src/components/global-styles/border-panel.js +++ b/packages/edit-site/src/components/global-styles/border-panel.js @@ -35,6 +35,7 @@ export default function BorderPanel( { name, variation = '' } ) { value={ style } onChange={ setStyle } settings={ settings } + forceSplitBorders /> ); } From 854ab4be06c3cc21ebf8a13c6f8fd348c2314914 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 6 Apr 2023 12:03:04 +1000 Subject: [PATCH 3/4] Avoid using prop to modify border change behaviour --- .../components/global-styles/border-panel.js | 31 +---------------- .../components/global-styles/border-panel.js | 34 +++++++++++++++++-- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/packages/block-editor/src/components/global-styles/border-panel.js b/packages/block-editor/src/components/global-styles/border-panel.js index 0549e213e44894..cab4158fe9a908 100644 --- a/packages/block-editor/src/components/global-styles/border-panel.js +++ b/packages/block-editor/src/components/global-styles/border-panel.js @@ -111,7 +111,6 @@ export default function BorderPanel( { settings, panelId, defaultControls = DEFAULT_CONTROLS, - forceSplitBorders = false, } ) { const colors = useColorsPerOrigin( settings ); const decodeValue = ( rawValue ) => @@ -187,35 +186,7 @@ export default function BorderPanel( { const onBorderChange = ( newBorder ) => { // Ensure we have a visible border style when a border width or // color is being selected. - const newBorderWithStyle = applyAllFallbackStyles( newBorder ); - let updatedBorder = newBorderWithStyle; - - if ( forceSplitBorders ) { - // As Global Styles can't conditionally generate styles based on if - // other style properties have been set, we need to force split - // border definitions for user set global border styles. Border - // radius is derived from the same property i.e. `border.radius` if - // it is a string that is used. The longhand border radii styles are - // only generated if that property is an object. - // - // For borders (color, style, and width) those are all properties on - // the `border` style property. This means if the theme.json defined - // split borders and the user condenses them into a flat border or - // vice-versa we'd get both sets of styles which would conflict. - updatedBorder = ! hasSplitBorders( newBorderWithStyle ) - ? { - top: newBorderWithStyle, - right: newBorderWithStyle, - bottom: newBorderWithStyle, - left: newBorderWithStyle, - } - : { - color: null, - style: null, - width: null, - ...newBorderWithStyle, - }; - } + const updatedBorder = applyAllFallbackStyles( newBorder ); if ( hasSplitBorders( updatedBorder ) ) { [ 'top', 'right', 'bottom', 'left' ].forEach( ( side ) => { diff --git a/packages/edit-site/src/components/global-styles/border-panel.js b/packages/edit-site/src/components/global-styles/border-panel.js index d90a12f0b03d33..4d95757e2e9dcd 100644 --- a/packages/edit-site/src/components/global-styles/border-panel.js +++ b/packages/edit-site/src/components/global-styles/border-panel.js @@ -2,6 +2,7 @@ * WordPress dependencies */ import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; +import { __experimentalHasSplitBorders as hasSplitBorders } from '@wordpress/components'; /** * Internal dependencies @@ -29,13 +30,42 @@ export default function BorderPanel( { name, variation = '' } ) { const [ rawSettings ] = useGlobalSetting( '', name ); const settings = useSettingsForBlockElement( rawSettings, name ); + const onChange = ( newStyle ) => { + // As Global Styles can't conditionally generate styles based on if + // other style properties have been set, we need to force split + // border definitions for user set global border styles. Border + // radius is derived from the same property i.e. `border.radius` if + // it is a string that is used. The longhand border radii styles are + // only generated if that property is an object. + // + // For borders (color, style, and width) those are all properties on + // the `border` style property. This means if the theme.json defined + // split borders and the user condenses them into a flat border or + // vice-versa we'd get both sets of styles which would conflict. + const { border } = newStyle; + const updatedBorder = ! hasSplitBorders( border ) + ? { + top: border, + right: border, + bottom: border, + left: border, + } + : { + color: null, + style: null, + width: null, + ...border, + }; + + setStyle( { ...newStyle, border: updatedBorder } ); + }; + return ( ); } From 6aadf4cbf985bcb98df12272854f246da5a326bd Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 6 Apr 2023 18:26:41 +1000 Subject: [PATCH 4/4] Prevent encoding colors for sides that aren't set Also prevents attempting to encode flat color for border object that is undefined. --- .../src/components/global-styles/border-panel.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/global-styles/border-panel.js b/packages/block-editor/src/components/global-styles/border-panel.js index cab4158fe9a908..b4d47c03efe21a 100644 --- a/packages/block-editor/src/components/global-styles/border-panel.js +++ b/packages/block-editor/src/components/global-styles/border-panel.js @@ -190,12 +190,14 @@ export default function BorderPanel( { if ( hasSplitBorders( updatedBorder ) ) { [ 'top', 'right', 'bottom', 'left' ].forEach( ( side ) => { - updatedBorder[ side ] = { - ...updatedBorder[ side ], - color: encodeColorValue( updatedBorder[ side ]?.color ), - }; + if ( updatedBorder[ side ] ) { + updatedBorder[ side ] = { + ...updatedBorder[ side ], + color: encodeColorValue( updatedBorder[ side ]?.color ), + }; + } } ); - } else { + } else if ( updatedBorder ) { updatedBorder.color = encodeColorValue( updatedBorder.color ); }