From 6775d9284628710bef8ff535c686a7135769e855 Mon Sep 17 00:00:00 2001 From: Jorge Date: Wed, 30 Oct 2019 19:32:19 +0000 Subject: [PATCH 1/2] Fix: setting a preset color on pullquote default style makes the block invalid. --- .../block-library/src/pullquote/deprecated.js | 61 +++++++++++++++++++ packages/block-library/src/pullquote/edit.js | 29 ++++++++- packages/block-library/src/pullquote/save.js | 14 +---- .../e2e-tests/fixtures/block-transforms.js | 7 +++ .../blocks/core__pullquote__deprecated-3.html | 3 + .../blocks/core__pullquote__deprecated-3.json | 15 +++++ .../core__pullquote__deprecated-3.parsed.json | 23 +++++++ ...e__pullquote__deprecated-3.serialized.html | 3 + .../blocks/core__pullquote__main-color.html | 3 + .../blocks/core__pullquote__main-color.json | 16 +++++ .../core__pullquote__main-color.parsed.json | 24 ++++++++ ...ore__pullquote__main-color.serialized.html | 3 + .../block-transforms.test.js.snap | 6 ++ 13 files changed, 192 insertions(+), 15 deletions(-) create mode 100644 packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.html create mode 100644 packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.json create mode 100644 packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.parsed.json create mode 100644 packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html create mode 100644 packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.html create mode 100644 packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.json create mode 100644 packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.parsed.json create mode 100644 packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.serialized.html diff --git a/packages/block-library/src/pullquote/deprecated.js b/packages/block-library/src/pullquote/deprecated.js index dc323edf32ca..1261b788a974 100644 --- a/packages/block-library/src/pullquote/deprecated.js +++ b/packages/block-library/src/pullquote/deprecated.js @@ -49,6 +49,67 @@ const blockAttributes = { }; const deprecated = [ + { + attributes: blockAttributes, + save( { attributes } ) { + const { + mainColor, + customMainColor, + textColor, + customTextColor, + value, + citation, + className, + } = attributes; + + const isSolidColorStyle = includes( className, SOLID_COLOR_CLASS ); + + let figureClasses, figureStyles; + + // Is solid color style + if ( isSolidColorStyle ) { + const backgroundClass = getColorClassName( 'background-color', mainColor ); + + figureClasses = classnames( { + 'has-background': ( backgroundClass || customMainColor ), + [ backgroundClass ]: backgroundClass, + } ); + + figureStyles = { + backgroundColor: backgroundClass ? undefined : customMainColor, + }; + // Is normal style and a custom color is being used ( we can set a style directly with its value) + } else if ( customMainColor ) { + figureStyles = { + borderColor: customMainColor, + }; + // If normal style and a named color are being used, we need to retrieve the color value to set the style, + // as there is no expectation that themes create classes that set border colors. + } else if ( mainColor ) { + const colors = get( select( 'core/block-editor' ).getSettings(), [ 'colors' ], [] ); + const colorObject = getColorObjectByAttributeValues( colors, mainColor ); + figureStyles = { + borderColor: colorObject.color, + }; + } + + const blockquoteTextColorClass = getColorClassName( 'color', textColor ); + const blockquoteClasses = ( textColor || customTextColor ) && classnames( 'has-text-color', { + [ blockquoteTextColorClass ]: blockquoteTextColorClass, + } ); + + const blockquoteStyles = blockquoteTextColorClass ? undefined : { color: customTextColor }; + + return ( +
+
+ + { ! RichText.isEmpty( citation ) && } +
+
+ ); + }, + }, { attributes: blockAttributes, save( { attributes } ) { diff --git a/packages/block-library/src/pullquote/edit.js b/packages/block-library/src/pullquote/edit.js index 010ac8f5078d..5b37164535a2 100644 --- a/packages/block-library/src/pullquote/edit.js +++ b/packages/block-library/src/pullquote/edit.js @@ -32,12 +32,20 @@ class PullQuoteEdit extends Component { } pullQuoteMainColorSetter( colorValue ) { - const { colorUtils, textColor, setTextColor, setMainColor, className } = this.props; + const { colorUtils, textColor, setAttributes, setTextColor, setMainColor, className } = this.props; const isSolidColorStyle = includes( className, SOLID_COLOR_CLASS ); const needTextColor = ! textColor.color || this.wasTextColorAutomaticallyComputed; const shouldSetTextColor = isSolidColorStyle && needTextColor && colorValue; - setMainColor( colorValue ); + if ( isSolidColorStyle ) { + // If we use the solid color style, set the color using the normal mechanism. + setMainColor( colorValue ); + } else { + // If we use the default style, set the color as a custom color to force the usage of an inline style. + // Default style uses a border color for which classes are not available. + setAttributes( { customMainColor: colorValue } ); + } + if ( shouldSetTextColor ) { this.wasTextColorAutomaticallyComputed = true; setTextColor( colorUtils.getMostReadableColor( colorValue ) ); @@ -50,6 +58,23 @@ class PullQuoteEdit extends Component { this.wasTextColorAutomaticallyComputed = false; } + componentDidUpdate( prevProps ) { + const { + attributes, + className, + mainColor, + setAttributes, + } = this.props; + // If the block includes a named color and we switched from the + // solid color style to the default style. + if ( attributes.mainColor && ! includes( className, SOLID_COLOR_CLASS ) && includes( prevProps.className, SOLID_COLOR_CLASS ) ) { + // Remove the named color, and set the color as a custom color. + // This is done because named colors use classes, in the default style we use a border color, + // and themes don't set classes for border colors. + setAttributes( { mainColor: undefined, customMainColor: mainColor.color } ); + } + } + render() { const { attributes, diff --git a/packages/block-library/src/pullquote/save.js b/packages/block-library/src/pullquote/save.js index bb07a42d6814..c2f9833046fa 100644 --- a/packages/block-library/src/pullquote/save.js +++ b/packages/block-library/src/pullquote/save.js @@ -2,7 +2,7 @@ * External dependencies */ import classnames from 'classnames'; -import { get, includes } from 'lodash'; +import { includes } from 'lodash'; /** * WordPress dependencies @@ -10,11 +10,7 @@ import { get, includes } from 'lodash'; import { getColorClassName, RichText, - getColorObjectByAttributeValues, } from '@wordpress/block-editor'; -import { - select, -} from '@wordpress/data'; /** * Internal dependencies @@ -53,14 +49,6 @@ export default function save( { attributes } ) { figureStyles = { borderColor: customMainColor, }; - // If normal style and a named color are being used, we need to retrieve the color value to set the style, - // as there is no expectation that themes create classes that set border colors. - } else if ( mainColor ) { - const colors = get( select( 'core/block-editor' ).getSettings(), [ 'colors' ], [] ); - const colorObject = getColorObjectByAttributeValues( colors, mainColor ); - figureStyles = { - borderColor: colorObject.color, - }; } const blockquoteTextColorClass = getColorClassName( 'color', textColor ); diff --git a/packages/e2e-tests/fixtures/block-transforms.js b/packages/e2e-tests/fixtures/block-transforms.js index 7a85f462d54c..3a44a8b2e817 100644 --- a/packages/e2e-tests/fixtures/block-transforms.js +++ b/packages/e2e-tests/fixtures/block-transforms.js @@ -390,6 +390,13 @@ export const EXPECTED_TRANSFORMS = { 'Group', ], }, + 'core__pullquote__main-color': { + originalBlock: 'Pullquote', + availableTransforms: [ + 'Quote', + 'Group', + ], + }, 'core__pullquote__multi-paragraph': { originalBlock: 'Pullquote', availableTransforms: [ diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.html b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.html new file mode 100644 index 000000000000..b3fd2c37a633 --- /dev/null +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.html @@ -0,0 +1,3 @@ + +

pullquote

+ diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.json b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.json new file mode 100644 index 000000000000..c3e12273ea37 --- /dev/null +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.json @@ -0,0 +1,15 @@ +[ + { + "clientId": "_clientId_0", + "name": "core/pullquote", + "isValid": false, + "attributes": { + "value": "

pullquote

", + "citation": "", + "mainColor": "accent", + "textColor": "secondary" + }, + "innerBlocks": [], + "originalContent": "

pullquote

" + } +] diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.parsed.json b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.parsed.json new file mode 100644 index 000000000000..61459172c1eb --- /dev/null +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.parsed.json @@ -0,0 +1,23 @@ +[ + { + "blockName": "core/pullquote", + "attrs": { + "mainColor": "accent", + "textColor": "secondary" + }, + "innerBlocks": [], + "innerHTML": "\n

pullquote

\n", + "innerContent": [ + "\n

pullquote

\n" + ] + }, + { + "blockName": null, + "attrs": {}, + "innerBlocks": [], + "innerHTML": "\n", + "innerContent": [ + "\n" + ] + } +] diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html new file mode 100644 index 000000000000..b3fd2c37a633 --- /dev/null +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html @@ -0,0 +1,3 @@ + +

pullquote

+ diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.html b/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.html new file mode 100644 index 000000000000..473e6ecefd48 --- /dev/null +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.html @@ -0,0 +1,3 @@ + +

Pullquote custom color

my citation
+ diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.json b/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.json new file mode 100644 index 000000000000..a5e6479043d1 --- /dev/null +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.json @@ -0,0 +1,16 @@ +[ + { + "clientId": "_clientId_0", + "name": "core/pullquote", + "isValid": true, + "attributes": { + "value": "

Pullquote custom color

", + "citation": "my citation", + "customMainColor": "#2207d0", + "textColor": "subtle-background", + "className": "has-background is-style-default" + }, + "innerBlocks": [], + "originalContent": "

Pullquote custom color

my citation
" + } +] diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.parsed.json b/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.parsed.json new file mode 100644 index 000000000000..56004374d156 --- /dev/null +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.parsed.json @@ -0,0 +1,24 @@ +[ + { + "blockName": "core/pullquote", + "attrs": { + "customMainColor": "#2207d0", + "textColor": "subtle-background", + "className": "has-background is-style-default" + }, + "innerBlocks": [], + "innerHTML": "\n

Pullquote custom color

my citation
\n", + "innerContent": [ + "\n

Pullquote custom color

my citation
\n" + ] + }, + { + "blockName": null, + "attrs": {}, + "innerBlocks": [], + "innerHTML": "\n", + "innerContent": [ + "\n" + ] + } +] diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.serialized.html b/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.serialized.html new file mode 100644 index 000000000000..473e6ecefd48 --- /dev/null +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.serialized.html @@ -0,0 +1,3 @@ + +

Pullquote custom color

my citation
+ diff --git a/packages/e2e-tests/specs/experimental/__snapshots__/block-transforms.test.js.snap b/packages/e2e-tests/specs/experimental/__snapshots__/block-transforms.test.js.snap index f461a1a4412d..bf5ae4c96744 100644 --- a/packages/e2e-tests/specs/experimental/__snapshots__/block-transforms.test.js.snap +++ b/packages/e2e-tests/specs/experimental/__snapshots__/block-transforms.test.js.snap @@ -456,6 +456,12 @@ exports[`Block transforms correctly transform block Pullquote in fixture core__p " `; +exports[`Block transforms correctly transform block Pullquote in fixture core__pullquote__main-color into the Quote block 1`] = ` +" +

Pullquote custom color

my citation
+" +`; + exports[`Block transforms correctly transform block Pullquote in fixture core__pullquote__multi-paragraph into the Quote block 1`] = ` "

Paragraph one

Paragraph two

by whomever
From 7f6eccfd7fca5be2bbac52372a200af66858160f Mon Sep 17 00:00:00 2001 From: Jorge Date: Mon, 4 Nov 2019 11:40:40 +0000 Subject: [PATCH 2/2] Updated deprecation to keep the colors --- .../block-library/src/pullquote/deprecated.js | 53 +++++++++++++++++-- .../blocks/core__pullquote__deprecated-3.json | 4 +- ...e__pullquote__deprecated-3.serialized.html | 2 +- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/pullquote/deprecated.js b/packages/block-library/src/pullquote/deprecated.js index 1261b788a974..150bd1eb615c 100644 --- a/packages/block-library/src/pullquote/deprecated.js +++ b/packages/block-library/src/pullquote/deprecated.js @@ -48,9 +48,28 @@ const blockAttributes = { }, }; +function parseBorderColor( styleString ) { + if ( ! styleString ) { + return; + } + const matches = styleString.match( /border-color:([^;]+)[;]?/ ); + if ( matches && matches[ 1 ] ) { + return matches[ 1 ]; + } +} + const deprecated = [ { - attributes: blockAttributes, + attributes: { + ...blockAttributes, + // figureStyle is an attribute that never existed. + // We are using it as a way to access the styles previously applied to the figure. + figureStyle: { + source: 'attribute', + selector: 'figure', + attribute: 'style', + }, + }, save( { attributes } ) { const { mainColor, @@ -60,6 +79,7 @@ const deprecated = [ value, citation, className, + figureStyle, } = attributes; const isSolidColorStyle = includes( className, SOLID_COLOR_CLASS ); @@ -86,10 +106,14 @@ const deprecated = [ // If normal style and a named color are being used, we need to retrieve the color value to set the style, // as there is no expectation that themes create classes that set border colors. } else if ( mainColor ) { - const colors = get( select( 'core/block-editor' ).getSettings(), [ 'colors' ], [] ); - const colorObject = getColorObjectByAttributeValues( colors, mainColor ); + // Previously here we queried the color settings to know the color value + // of a named color. This made the save function impure and the block was refactored, + // because meanwhile a change in the editor made it impossible to query color settings in the save function. + // Here instead of querying the color settings to know the color value, we retrieve the value + // directly from the style previously serialized. + const borderColor = parseBorderColor( figureStyle ); figureStyles = { - borderColor: colorObject.color, + borderColor, }; } @@ -109,6 +133,27 @@ const deprecated = [ ); }, + migrate( { className, figureStyle, mainColor, ...attributes } ) { + const isSolidColorStyle = includes( className, SOLID_COLOR_CLASS ); + // If is the default style, and a main color is set, + // migrate the main color value into a custom color. + // The custom color value is retrived by parsing the figure styles. + if ( ! isSolidColorStyle && mainColor && figureStyle ) { + const borderColor = parseBorderColor( figureStyle ); + if ( borderColor ) { + return { + ...attributes, + className, + customMainColor: borderColor, + }; + } + } + return { + className, + mainColor, + ...attributes, + }; + }, }, { attributes: blockAttributes, diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.json b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.json index c3e12273ea37..00948d585075 100644 --- a/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.json +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.json @@ -2,11 +2,11 @@ { "clientId": "_clientId_0", "name": "core/pullquote", - "isValid": false, + "isValid": true, "attributes": { "value": "

pullquote

", "citation": "", - "mainColor": "accent", + "customMainColor": "#cd2653", "textColor": "secondary" }, "innerBlocks": [], diff --git a/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html index b3fd2c37a633..786f15f7b429 100644 --- a/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html +++ b/packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html @@ -1,3 +1,3 @@ - +

pullquote