From 75a2463b006781e4097f1d61232b27bbb48be816 Mon Sep 17 00:00:00 2001 From: Jeremy Yip Date: Wed, 25 Aug 2021 03:54:17 -0700 Subject: [PATCH 1/5] Check if block color support have been explicitly opted out BackgroundColor and color are opt-out block supports: they're enabled if there's support for any color unless the block opts-out from them explicitly. Global styles UI panels were't respecting this opt out process, which is why we add logic to validate whether or not a block has turned off background color and color supports. --- packages/blocks/src/api/constants.js | 6 +++-- .../editor/global-styles-provider.js | 23 +++++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/blocks/src/api/constants.js b/packages/blocks/src/api/constants.js index 18342aa916256..d7ce7cafd2526 100644 --- a/packages/blocks/src/api/constants.js +++ b/packages/blocks/src/api/constants.js @@ -24,7 +24,8 @@ export const __EXPERIMENTAL_STYLE_PROPERTY = { }, backgroundColor: { value: [ 'color', 'background' ], - support: [ 'color' ], + support: [ 'color', 'background' ], + requiresOptOut: true, }, borderColor: { value: [ 'border', 'color' ], @@ -50,7 +51,8 @@ export const __EXPERIMENTAL_STYLE_PROPERTY = { }, color: { value: [ 'color', 'text' ], - support: [ 'color' ], + support: [ 'color', 'text' ], + requiresOptOut: true, }, linkColor: { value: [ 'elements', 'link', 'color', 'text' ], diff --git a/packages/edit-site/src/components/editor/global-styles-provider.js b/packages/edit-site/src/components/editor/global-styles-provider.js index 05e672dfa05f1..673930ddb01d1 100644 --- a/packages/edit-site/src/components/editor/global-styles-provider.js +++ b/packages/edit-site/src/components/editor/global-styles-provider.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { set, get, mergeWith, mapValues, setWith, clone } from 'lodash'; +import { set, get, has, mergeWith, mapValues, setWith, clone } from 'lodash'; /** * WordPress dependencies @@ -72,10 +72,29 @@ export const useGlobalStylesReset = () => { ]; }; +const shouldExtractKey = ( name, supports ) => { + // Opting out means that, for certain support keys, blocks have to explicitly + // set the support value false. If the key is unset, we still extract it. + const blockHasNotOptedOut = + STYLE_PROPERTY[ name ].requiresOptOut && + has( supports, STYLE_PROPERTY[ name ].support[ 0 ] ) && + get( supports, STYLE_PROPERTY[ name ].support ) !== false; + + const blockHasAllowedSupportKey = + ! STYLE_PROPERTY[ name ].requiresOptOut && + get( supports, STYLE_PROPERTY[ name ].support, false ); + + return blockHasAllowedSupportKey || blockHasNotOptedOut; +}; + const extractSupportKeys = ( supports ) => { const supportKeys = []; Object.keys( STYLE_PROPERTY ).forEach( ( name ) => { - if ( get( supports, STYLE_PROPERTY[ name ].support, false ) ) { + if ( ! STYLE_PROPERTY[ name ].support ) { + return; + } + + if ( shouldExtractKey( name, supports ) ) { supportKeys.push( name ); } } ); From 4c744492b6fb75cefa272d2f1900aacd9d3b6935 Mon Sep 17 00:00:00 2001 From: Jeremy Yip Date: Thu, 26 Aug 2021 13:59:35 -0700 Subject: [PATCH 2/5] Refactor opt out logic --- .../editor/global-styles-provider.js | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/edit-site/src/components/editor/global-styles-provider.js b/packages/edit-site/src/components/editor/global-styles-provider.js index 673930ddb01d1..d85aecdf251bc 100644 --- a/packages/edit-site/src/components/editor/global-styles-provider.js +++ b/packages/edit-site/src/components/editor/global-styles-provider.js @@ -72,21 +72,6 @@ export const useGlobalStylesReset = () => { ]; }; -const shouldExtractKey = ( name, supports ) => { - // Opting out means that, for certain support keys, blocks have to explicitly - // set the support value false. If the key is unset, we still extract it. - const blockHasNotOptedOut = - STYLE_PROPERTY[ name ].requiresOptOut && - has( supports, STYLE_PROPERTY[ name ].support[ 0 ] ) && - get( supports, STYLE_PROPERTY[ name ].support ) !== false; - - const blockHasAllowedSupportKey = - ! STYLE_PROPERTY[ name ].requiresOptOut && - get( supports, STYLE_PROPERTY[ name ].support, false ); - - return blockHasAllowedSupportKey || blockHasNotOptedOut; -}; - const extractSupportKeys = ( supports ) => { const supportKeys = []; Object.keys( STYLE_PROPERTY ).forEach( ( name ) => { @@ -94,8 +79,20 @@ const extractSupportKeys = ( supports ) => { return; } - if ( shouldExtractKey( name, supports ) ) { - supportKeys.push( name ); + // Opting out means that, for certain support keys like background color, + // blocks have to explicitly set the support value false. If the key is + // unset, we still enable it. + if ( STYLE_PROPERTY[ name ].requiresOptOut ) { + if ( + has( supports, STYLE_PROPERTY[ name ].support[ 0 ] ) && + get( supports, STYLE_PROPERTY[ name ].support ) !== false + ) { + return supportKeys.push( name ); + } + } + + if ( get( supports, STYLE_PROPERTY[ name ].support, false ) ) { + return supportKeys.push( name ); } } ); return supportKeys; From a8c20c1bcf12ebd48cac81745e55bae983245f06 Mon Sep 17 00:00:00 2001 From: Jeremy Yip Date: Thu, 26 Aug 2021 19:35:19 -0700 Subject: [PATCH 3/5] Add specs --- .../editor/test/global-styles-provider.js | 130 ++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 packages/edit-site/src/components/editor/test/global-styles-provider.js diff --git a/packages/edit-site/src/components/editor/test/global-styles-provider.js b/packages/edit-site/src/components/editor/test/global-styles-provider.js new file mode 100644 index 0000000000000..2af15dc4e278d --- /dev/null +++ b/packages/edit-site/src/components/editor/test/global-styles-provider.js @@ -0,0 +1,130 @@ +/** + * WordPress dependencies + */ +import { dispatch } from '@wordpress/data'; + +/** + * External dependencies + */ +import { mount } from 'enzyme'; +import { act } from 'react-dom/test-utils'; + +/** + * Internal dependencies + */ +import GlobalStylesProvider, { + useGlobalStylesContext, +} from '../global-styles-provider'; + +const settings = { + styles: [ + { + css: 'body {\n\tmargin: 0;\n\tpadding: 0;\n}', + baseURL: 'http://localhost:4759/ponyfill.css', + }, + ], + __experimentalGlobalStylesBaseStyles: {}, +}; + +const generateCoverBlockType = ( colorSupports ) => { + return { + name: 'core/cover', + supports: { + color: colorSupports, + }, + }; +}; + +const FakeCmp = () => { + const globalStylesContext = useGlobalStylesContext(); + const coverBlockSupports = + globalStylesContext.blocks[ 'core/cover' ].supports; + + return
; +}; + +const generateWrapper = () => { + return mount( + + + + ); +}; + +describe( 'global styles provider', () => { + beforeAll( () => { + dispatch( 'core/edit-site' ).updateSettings( settings ); + } ); + + // afterEach( () => { + // dispatch( 'core/blocks' ).removeBlockTypes( 'core/cover' ); + // } ); + + describe( 'when a block enables color support', () => { + it( 'automatically enables text color and background color supports', () => { + act( () => { + dispatch( 'core/blocks' ).addBlockTypes( + generateCoverBlockType( { link: true } ) + ); + } ); + + const wrapper = generateWrapper(); + const actual = wrapper + .findWhere( ( ele ) => Boolean( ele.prop( 'supports' ) ) ) + .prop( 'supports' ); + expect( actual ).toContain( 'backgroundColor' ); + expect( actual ).toContain( 'color' ); + + act( () => { + dispatch( 'core/blocks' ).removeBlockTypes( 'core/cover' ); + } ); + } ); + + describe( 'and both text color and background color support are disabled', () => { + it( 'disables text color and background color support', () => { + act( () => { + dispatch( 'core/blocks' ).addBlockTypes( + generateCoverBlockType( { + text: false, + background: false, + } ) + ); + } ); + + const wrapper = generateWrapper(); + const actual = wrapper + .findWhere( ( ele ) => Boolean( ele.prop( 'supports' ) ) ) + .prop( 'supports' ); + expect( actual ).not.toContain( 'backgroundColor' ); + expect( actual ).not.toContain( 'color' ); + + act( () => { + dispatch( 'core/blocks' ).removeBlockTypes( 'core/cover' ); + } ); + } ); + } ); + + describe( 'and text color and background color supports are omitted', () => { + it( 'still enables text color and background color supports', () => { + act( () => { + dispatch( 'core/blocks' ).addBlockTypes( + generateCoverBlockType( { link: true } ) + ); + } ); + + const wrapper = generateWrapper(); + const actual = wrapper + .findWhere( ( ele ) => Boolean( ele.prop( 'supports' ) ) ) + .prop( 'supports' ); + expect( actual ).toContain( 'backgroundColor' ); + expect( actual ).toContain( 'color' ); + + act( () => { + dispatch( 'core/blocks' ).removeBlockTypes( 'core/cover' ); + } ); + } ); + } ); + } ); +} ); From 6769337a9168a069533fe702229643e2eafc424c Mon Sep 17 00:00:00 2001 From: Jeremy Yip Date: Thu, 26 Aug 2021 19:36:43 -0700 Subject: [PATCH 4/5] Remove unnecessary afterEach from spec --- .../src/components/editor/test/global-styles-provider.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/edit-site/src/components/editor/test/global-styles-provider.js b/packages/edit-site/src/components/editor/test/global-styles-provider.js index 2af15dc4e278d..9102e21085938 100644 --- a/packages/edit-site/src/components/editor/test/global-styles-provider.js +++ b/packages/edit-site/src/components/editor/test/global-styles-provider.js @@ -58,10 +58,6 @@ describe( 'global styles provider', () => { dispatch( 'core/edit-site' ).updateSettings( settings ); } ); - // afterEach( () => { - // dispatch( 'core/blocks' ).removeBlockTypes( 'core/cover' ); - // } ); - describe( 'when a block enables color support', () => { it( 'automatically enables text color and background color supports', () => { act( () => { From 98d0c5c51ab1b8eddd36efa4251eafcaadf516ce Mon Sep 17 00:00:00 2001 From: Jeremy Yip Date: Thu, 26 Aug 2021 19:44:28 -0700 Subject: [PATCH 5/5] Update spec descriptions --- .../editor/test/global-styles-provider.js | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/edit-site/src/components/editor/test/global-styles-provider.js b/packages/edit-site/src/components/editor/test/global-styles-provider.js index 9102e21085938..0a31516576e57 100644 --- a/packages/edit-site/src/components/editor/test/global-styles-provider.js +++ b/packages/edit-site/src/components/editor/test/global-styles-provider.js @@ -59,22 +59,27 @@ describe( 'global styles provider', () => { } ); describe( 'when a block enables color support', () => { - it( 'automatically enables text color and background color supports', () => { - act( () => { - dispatch( 'core/blocks' ).addBlockTypes( - generateCoverBlockType( { link: true } ) - ); - } ); + describe( 'and disables background color support', () => { + it( 'still enables text color support', () => { + act( () => { + dispatch( 'core/blocks' ).addBlockTypes( + generateCoverBlockType( { + link: true, + background: false, + } ) + ); + } ); - const wrapper = generateWrapper(); - const actual = wrapper - .findWhere( ( ele ) => Boolean( ele.prop( 'supports' ) ) ) - .prop( 'supports' ); - expect( actual ).toContain( 'backgroundColor' ); - expect( actual ).toContain( 'color' ); + const wrapper = generateWrapper(); + const actual = wrapper + .findWhere( ( ele ) => Boolean( ele.prop( 'supports' ) ) ) + .prop( 'supports' ); + expect( actual ).not.toContain( 'backgroundColor' ); + expect( actual ).toContain( 'color' ); - act( () => { - dispatch( 'core/blocks' ).removeBlockTypes( 'core/cover' ); + act( () => { + dispatch( 'core/blocks' ).removeBlockTypes( 'core/cover' ); + } ); } ); } ); @@ -103,7 +108,7 @@ describe( 'global styles provider', () => { } ); describe( 'and text color and background color supports are omitted', () => { - it( 'still enables text color and background color supports', () => { + it( 'still enables both text color and background color supports', () => { act( () => { dispatch( 'core/blocks' ).addBlockTypes( generateCoverBlockType( { link: true } )