From f134ea8323020f53e418557369513d6d4307f665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 2 Dec 2021 15:04:20 +0200 Subject: [PATCH 1/4] Format library: fix unsetting highlight color --- packages/format-library/src/text-color/index.js | 6 ++++-- packages/format-library/src/text-color/inline.js | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/format-library/src/text-color/index.js b/packages/format-library/src/text-color/index.js index 347b5d6662e86..24df51b91d2b1 100644 --- a/packages/format-library/src/text-color/index.js +++ b/packages/format-library/src/text-color/index.js @@ -17,6 +17,8 @@ import { removeFormat } from '@wordpress/rich-text'; */ import { default as InlineColorUI, getActiveColors } from './inline'; +export const transparentValue = 'rgba(0, 0, 0, 0)'; + const name = 'core/text-color'; const title = __( 'Highlight' ); @@ -30,7 +32,7 @@ function getComputedStyleProperty( element, property ) { if ( property === 'background-color' && - value === 'rgba(0, 0, 0, 0)' && + value === transparentValue && element.parentElement ) { return getComputedStyleProperty( element.parentElement, property ); @@ -47,7 +49,7 @@ function fillComputedColors( element, { color, backgroundColor } ) { return { color: color || getComputedStyleProperty( element, 'color' ), backgroundColor: - backgroundColor === 'rgba(0, 0, 0, 0)' + backgroundColor === transparentValue ? getComputedStyleProperty( element, 'background-color' ) : backgroundColor, }; diff --git a/packages/format-library/src/text-color/inline.js b/packages/format-library/src/text-color/inline.js index abe1f2162bb85..504584c176de6 100644 --- a/packages/format-library/src/text-color/inline.js +++ b/packages/format-library/src/text-color/inline.js @@ -28,14 +28,14 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { textColor as settings } from './index'; +import { textColor as settings, transparentValue } from './index'; function parseCSS( css = '' ) { return css.split( ';' ).reduce( ( accumulator, rule ) => { if ( rule ) { const [ property, value ] = rule.split( ':' ); if ( property === 'color' ) accumulator.color = value; - if ( property === 'background-color' ) + if ( property === 'background-color' && value !== transparentValue ) accumulator.backgroundColor = value; } return accumulator; From 1903775709ca9063d0f4744eb27efa49a1d1c33e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 6 Dec 2021 12:05:29 +0200 Subject: [PATCH 2/4] Fix for named colors and add e2e test --- .../__snapshots__/text-color.test.js.snap | 13 +++++ .../various/format-library/text-color.test.js | 47 +++++++++++++++++++ .../format-library/src/text-color/inline.js | 7 +-- 3 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 packages/e2e-tests/specs/editor/various/format-library/__snapshots__/text-color.test.js.snap create mode 100644 packages/e2e-tests/specs/editor/various/format-library/text-color.test.js diff --git a/packages/e2e-tests/specs/editor/various/format-library/__snapshots__/text-color.test.js.snap b/packages/e2e-tests/specs/editor/various/format-library/__snapshots__/text-color.test.js.snap new file mode 100644 index 0000000000000..05823957322a1 --- /dev/null +++ b/packages/e2e-tests/specs/editor/various/format-library/__snapshots__/text-color.test.js.snap @@ -0,0 +1,13 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`RichText should remove highlighting element 1`] = ` +" +

1

+" +`; + +exports[`RichText should remove highlighting element 2`] = ` +" +

1

+" +`; diff --git a/packages/e2e-tests/specs/editor/various/format-library/text-color.test.js b/packages/e2e-tests/specs/editor/various/format-library/text-color.test.js new file mode 100644 index 0000000000000..ea3cf38a6d5de --- /dev/null +++ b/packages/e2e-tests/specs/editor/various/format-library/text-color.test.js @@ -0,0 +1,47 @@ +/** + * WordPress dependencies + */ +import { + createNewPost, + getEditedPostContent, + clickBlockAppender, + pressKeyWithModifier, + clickBlockToolbarButton, +} from '@wordpress/e2e-test-utils'; + +describe( 'RichText', () => { + beforeEach( async () => { + await createNewPost(); + } ); + + it( 'should remove highlighting element', async () => { + await clickBlockAppender(); + + // Add text and select to color. + await page.keyboard.type( '1' ); + await pressKeyWithModifier( 'primary', 'a' ); + await clickBlockToolbarButton( 'More' ); + + const button = await page.waitForXPath( + `//button[text()='Highlight']` + ); + // Clicks may fail if the button is out of view. Assure it is before click. + await button.evaluate( ( element ) => element.scrollIntoView() ); + await button.click(); + + // Tab to the "Text" tab. + await page.keyboard.press( 'Tab' ); + // Tab to black. + await page.keyboard.press( 'Tab' ); + // Select color other than black. + await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Space' ); + + expect( await getEditedPostContent() ).toMatchSnapshot(); + + await page.keyboard.press( 'Space' ); + + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); +} ); diff --git a/packages/format-library/src/text-color/inline.js b/packages/format-library/src/text-color/inline.js index 504584c176de6..c2f12b2cc57bc 100644 --- a/packages/format-library/src/text-color/inline.js +++ b/packages/format-library/src/text-color/inline.js @@ -44,9 +44,10 @@ function parseCSS( css = '' ) { function parseClassName( className = '', colorSettings ) { return className.split( ' ' ).reduce( ( accumulator, name ) => { - const match = name.match( /^has-([^-]+)-color$/ ); - if ( match ) { - const [ , colorSlug ] = name.match( /^has-([^-]+)-color$/ ); + if ( name.startsWith( 'has-' ) && name.endsWith( '-color' ) ) { + const colorSlug = name + .replace( /^has-/, '' ) + .replace( /-color$/, '' ); const colorObject = getColorObjectByAttributeValues( colorSettings, colorSlug From 9635057c21faa288c27c573efee92ec0f6e21987 Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Mon, 6 Dec 2021 10:24:56 +0000 Subject: [PATCH 3/4] Use new constant `transparentValue` throughout --- packages/format-library/src/text-color/index.js | 2 +- packages/format-library/src/text-color/inline.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/format-library/src/text-color/index.js b/packages/format-library/src/text-color/index.js index 24df51b91d2b1..a79129d1eb554 100644 --- a/packages/format-library/src/text-color/index.js +++ b/packages/format-library/src/text-color/index.js @@ -141,7 +141,7 @@ export const textColor = { if ( key !== 'style' ) return value; // We should not add a background-color if it's already set if ( value && value.includes( 'background-color' ) ) return value; - const addedCSS = [ 'background-color', 'rgba(0, 0, 0, 0)' ].join( ':' ); + const addedCSS = [ 'background-color', transparentValue ].join( ':' ); // Prepend `addedCSS` to avoid a double `;;` as any the existing CSS // rules will already include a `;`. return value ? [ addedCSS, value ].join( ';' ) : addedCSS; diff --git a/packages/format-library/src/text-color/inline.js b/packages/format-library/src/text-color/inline.js index c2f12b2cc57bc..296f30f65c1ea 100644 --- a/packages/format-library/src/text-color/inline.js +++ b/packages/format-library/src/text-color/inline.js @@ -89,7 +89,7 @@ function setColors( value, name, colorSettings, colors ) { styles.push( [ 'background-color', backgroundColor ].join( ':' ) ); } else { // Override default browser color for mark element. - styles.push( [ 'background-color', 'rgba(0, 0, 0, 0)' ].join( ':' ) ); + styles.push( [ 'background-color', transparentValue ].join( ':' ) ); } if ( color ) { From afec558c15571ff9605a78d71df22f597c285bbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 6 Dec 2021 21:59:47 +0200 Subject: [PATCH 4/4] Address feedback --- .../various/format-library/text-color.test.js | 17 ++++++++--------- .../format-library/src/text-color/inline.js | 1 + 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/format-library/text-color.test.js b/packages/e2e-tests/specs/editor/various/format-library/text-color.test.js index ea3cf38a6d5de..ba6a8bda06180 100644 --- a/packages/e2e-tests/specs/editor/various/format-library/text-color.test.js +++ b/packages/e2e-tests/specs/editor/various/format-library/text-color.test.js @@ -29,18 +29,17 @@ describe( 'RichText', () => { await button.evaluate( ( element ) => element.scrollIntoView() ); await button.click(); - // Tab to the "Text" tab. - await page.keyboard.press( 'Tab' ); - // Tab to black. - await page.keyboard.press( 'Tab' ); - // Select color other than black. - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Space' ); + // Use a color name with multiple words to ensure that it becomes + // active. Previously we had a broken regular expression. + const option = await page.waitForSelector( + '[aria-label="Color: Cyan bluish gray"]' + ); + + await option.click(); expect( await getEditedPostContent() ).toMatchSnapshot(); - await page.keyboard.press( 'Space' ); + await option.click(); expect( await getEditedPostContent() ).toMatchSnapshot(); } ); diff --git a/packages/format-library/src/text-color/inline.js b/packages/format-library/src/text-color/inline.js index 296f30f65c1ea..37f2d6f9401f6 100644 --- a/packages/format-library/src/text-color/inline.js +++ b/packages/format-library/src/text-color/inline.js @@ -44,6 +44,7 @@ function parseCSS( css = '' ) { function parseClassName( className = '', colorSettings ) { return className.split( ' ' ).reduce( ( accumulator, name ) => { + // `colorSlug` could contain dashes, so simply match the start and end. if ( name.startsWith( 'has-' ) && name.endsWith( '-color' ) ) { const colorSlug = name .replace( /^has-/, '' )