diff --git a/blocks/api/factory.js b/blocks/api/factory.js index 7beaf4b596163..310fcae6c0708 100644 --- a/blocks/api/factory.js +++ b/blocks/api/factory.js @@ -46,11 +46,6 @@ export function createBlock( name, blockAttributes = {} ) { return result; }, {} ); - // Keep the className if the block supports it - if ( blockType.className !== false && blockAttributes.className ) { - attributes.className = blockAttributes.className; - } - // Blocks are stored with a unique ID, the assigned type name, // and the block attributes. return { diff --git a/blocks/api/parser.js b/blocks/api/parser.js index f04d4c23b1ce3..211379bbc2680 100644 --- a/blocks/api/parser.js +++ b/blocks/api/parser.js @@ -126,11 +126,6 @@ export function getBlockAttributes( blockType, innerHTML, attributes ) { return getBlockAttribute( attributeKey, attributeSchema, innerHTML, attributes ); } ); - // If the block supports a custom className parse it - if ( blockType.className !== false && attributes && attributes.className ) { - blockAttributes.className = attributes.className; - } - return blockAttributes; } diff --git a/blocks/api/serializer.js b/blocks/api/serializer.js index 4e3628147904d..f3b9d8b436582 100644 --- a/blocks/api/serializer.js +++ b/blocks/api/serializer.js @@ -50,28 +50,29 @@ export function getSaveContent( blockType, attributes ) { } } - // Adding a generic classname - const addAdvancedAttributes = ( element ) => { + const addExtraContainerProps = ( element ) => { if ( ! element || ! isObject( element ) ) { return element; } - const extraProps = applyFilters( 'getSaveContent.extraProps', {}, blockType, attributes ); + // Applying the filters adding extra props + const props = applyFilters( 'getSaveContent.extraProps', { ...element.props }, blockType, attributes ); + + // Adding the generated className if ( !! className ) { const updatedClassName = classnames( className, - element.props.className, - attributes.className + props.className, ); - extraProps.className = updatedClassName; + props.className = updatedClassName; } - return cloneElement( element, extraProps ); + return cloneElement( element, props ); }; - const contentWithClassname = Children.map( saveContent, addAdvancedAttributes ); + const contentWithClassName = Children.map( saveContent, addExtraContainerProps ); // Otherwise, infer as element - return renderToString( contentWithClassname ); + return renderToString( contentWithClassName ); } /** diff --git a/blocks/api/test/factory.js b/blocks/api/test/factory.js index 893d1ec59e74f..f2df17e628e98 100644 --- a/blocks/api/test/factory.js +++ b/blocks/api/test/factory.js @@ -80,7 +80,9 @@ describe( 'block factory', () => { save: noop, category: 'common', title: 'test block', - className: false, + supports: { + customClassName: false, + }, } ); const block = createBlock( 'core/test-block', { className: 'chicken', diff --git a/blocks/api/test/parser.js b/blocks/api/test/parser.js index 577fc93b9e161..3eb16a2878e59 100644 --- a/blocks/api/test/parser.js +++ b/blocks/api/test/parser.js @@ -155,31 +155,6 @@ describe( 'block parser', () => { topic: 'none', } ); } ); - - it( 'should parse the className if the block supports it', () => { - const blockType = { - attributes: {}, - }; - - const innerHTML = '
Ribs
'; - const attrs = { className: 'chicken' }; - - expect( getBlockAttributes( blockType, innerHTML, attrs ) ).toEqual( { - className: 'chicken', - } ); - } ); - - it( 'should not parse the className if the block supports it', () => { - const blockType = { - attributes: {}, - className: false, - }; - - const innerHTML = '
Ribs
'; - const attrs = { className: 'chicken' }; - - expect( getBlockAttributes( blockType, innerHTML, attrs ) ).toEqual( {} ); - } ); } ); describe( 'createBlockWithFallback', () => { diff --git a/blocks/api/test/registration.js b/blocks/api/test/registration.js index 6f4d1c2df161c..ba92666124158 100644 --- a/blocks/api/test/registration.js +++ b/blocks/api/test/registration.js @@ -79,7 +79,11 @@ describe( 'blocks', () => { save: noop, category: 'common', title: 'block title', - attributes: {}, + attributes: { + className: { + type: 'string', + }, + }, } ); } ); @@ -160,9 +164,14 @@ describe( 'blocks', () => { category: 'common', title: 'block title', icon: 'block-default', - attributes: { ok: { - type: 'boolean', - } }, + attributes: { + ok: { + type: 'boolean', + }, + className: { + type: 'string', + }, + }, } ); } ); @@ -177,7 +186,11 @@ describe( 'blocks', () => { category: 'common', title: 'block title', icon: 'block-default', - attributes: {}, + attributes: { + className: { + type: 'string', + }, + }, } ); } ); } ); @@ -191,14 +204,20 @@ describe( 'blocks', () => { it( 'should unregister existing blocks', () => { registerBlockType( 'core/test-block', defaultBlockSettings ); - expect( getBlockTypes() ).toEqual( [ { - name: 'core/test-block', - save: noop, - category: 'common', - title: 'block title', - icon: 'block-default', - attributes: {}, - } ] ); + expect( getBlockTypes() ).toEqual( [ + { + name: 'core/test-block', + save: noop, + category: 'common', + title: 'block title', + icon: 'block-default', + attributes: { + className: { + type: 'string', + }, + }, + }, + ] ); const oldBlock = unregisterBlockType( 'core/test-block' ); expect( console.error ).not.toHaveBeenCalled(); expect( oldBlock ).toEqual( { @@ -207,7 +226,11 @@ describe( 'blocks', () => { category: 'common', title: 'block title', icon: 'block-default', - attributes: {}, + attributes: { + className: { + type: 'string', + }, + }, } ); expect( getBlockTypes() ).toEqual( [] ); } ); @@ -250,7 +273,11 @@ describe( 'blocks', () => { category: 'common', title: 'block title', icon: 'block-default', - attributes: {}, + attributes: { + className: { + type: 'string', + }, + }, } ); } ); @@ -264,7 +291,11 @@ describe( 'blocks', () => { category: 'common', title: 'block title', icon: 'block-default', - attributes: {}, + attributes: { + className: { + type: 'string', + }, + }, } ); } ); } ); @@ -285,7 +316,11 @@ describe( 'blocks', () => { category: 'common', title: 'block title', icon: 'block-default', - attributes: {}, + attributes: { + className: { + type: 'string', + }, + }, }, { name: 'core/test-block-with-settings', @@ -294,7 +329,11 @@ describe( 'blocks', () => { category: 'common', title: 'block title', icon: 'block-default', - attributes: {}, + attributes: { + className: { + type: 'string', + }, + }, }, ] ); } ); diff --git a/blocks/block-edit/index.js b/blocks/block-edit/index.js index ee1cb389cb47b..97362d85c1308 100644 --- a/blocks/block-edit/index.js +++ b/blocks/block-edit/index.js @@ -20,7 +20,7 @@ function BlockEdit( props ) { Edit = blockType.edit || blockType.save; } - return applyFilters( 'BlockEdit', , props ); + return applyFilters( 'BlockEdit', , props ); } export default BlockEdit; diff --git a/blocks/hooks/anchor.js b/blocks/hooks/anchor.js index a674a6ba57bf5..88a8bb275dd4a 100644 --- a/blocks/hooks/anchor.js +++ b/blocks/hooks/anchor.js @@ -6,7 +6,6 @@ import { assign } from 'lodash'; /** * WordPress dependencies */ -import { cloneElement } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; /** @@ -56,8 +55,8 @@ export function addAttribute( settings ) { export function addInspectorControl( element, props ) { if ( hasBlockSupport( props.name, 'anchor' ) && props.focus ) { element = [ - cloneElement( element, { key: 'edit' } ), - + element, + + { + props.setAttributes( { + className: nextValue, + } ); + } } + /> + , + ]; + } + + return element; +} + +/** + * Override props assigned to save component to inject anchor ID, if block + * supports anchor. This is only applied if the block's save result is an + * element and not a markup string. + * + * @param {Object} extraProps Additional props applied to save element + * @param {Object} blockType Block type + * @param {Object} attributes Current block attributes + * @return {Object} Filtered props applied to save element + */ +export function addSaveProps( extraProps, blockType, attributes ) { + if ( hasBlockSupport( blockType, 'customClassName', true ) && attributes.className ) { + extraProps.className = classnames( extraProps.className, attributes.className ); + } + + return extraProps; +} + +export default function customClassName( { addFilter } ) { + addFilter( 'registerBlockType', 'core\custom-class-name-attribute', addAttribute ); + addFilter( 'BlockEdit', 'core\custom-class-name-inspector-control', addInspectorControl ); + addFilter( 'getSaveContent.extraProps', 'core\custom-class-name-save-props', addSaveProps ); +} diff --git a/blocks/hooks/index.js b/blocks/hooks/index.js index 1a61cfddb51f1..e725627bd21c0 100644 --- a/blocks/hooks/index.js +++ b/blocks/hooks/index.js @@ -7,6 +7,7 @@ import createHooks from '@wordpress/hooks'; * Internal dependencies */ import anchor from './anchor'; +import customClassName from './custom-class-name'; const hooks = createHooks(); @@ -45,3 +46,4 @@ export { }; anchor( hooks ); +customClassName( hooks ); diff --git a/blocks/hooks/test/anchor.js b/blocks/hooks/test/anchor.js index 1fcecf44d6a0d..6df21c279563a 100644 --- a/blocks/hooks/test/anchor.js +++ b/blocks/hooks/test/anchor.js @@ -58,7 +58,7 @@ describe( 'anchor', () => { it( 'should do nothing if the block settings do not define anchor support', () => { const attributes = { anchor: 'foo' }; - const extraProps = addSaveProps( blockSettings, attributes ); + const extraProps = addSaveProps( {}, blockSettings, attributes ); expect( extraProps ).not.toHaveProperty( 'id' ); } ); diff --git a/blocks/hooks/test/custom-class-name.js b/blocks/hooks/test/custom-class-name.js new file mode 100644 index 0000000000000..9eadb54b44fe4 --- /dev/null +++ b/blocks/hooks/test/custom-class-name.js @@ -0,0 +1,78 @@ +/** + * External dependencies + */ +import { noop } from 'lodash'; + +/** + * External dependencies + */ +import createHooks from '@wordpress/hooks'; + +/** + * Internal dependencies + */ +import customClassName from '../custom-class-name'; + +describe( 'custom className', () => { + const hooks = createHooks(); + + let blockSettings; + beforeEach( () => { + customClassName( hooks ); + + blockSettings = { + save: noop, + category: 'common', + title: 'block title', + }; + } ); + + afterEach( () => { + hooks.removeAllFilters( 'registerBlockType' ); + hooks.removeAllFilters( 'getSaveContent.extraProps' ); + } ); + + describe( 'addAttribute()', () => { + const addAttribute = hooks.applyFilters.bind( null, 'registerBlockType' ); + + it( 'should do nothing if the block settings disable custom className support', () => { + const settings = addAttribute( { + ...blockSettings, + supports: { + customClassName: false, + }, + } ); + + expect( settings.attributes ).toBe( undefined ); + } ); + + it( 'should assign a new custom className attribute', () => { + const settings = addAttribute( blockSettings ); + + expect( settings.attributes ).toHaveProperty( 'className' ); + } ); + } ); + + describe( 'addSaveProps', () => { + const addSaveProps = hooks.applyFilters.bind( null, 'getSaveContent.extraProps' ); + + it( 'should do nothing if the block settings do not define anchor support', () => { + const attributes = { className: 'foo' }; + const extraProps = addSaveProps( {}, { + ...blockSettings, + supports: { + customClassName: false, + }, + }, attributes ); + + expect( extraProps ).not.toHaveProperty( 'className' ); + } ); + + it( 'should inject anchor attribute ID', () => { + const attributes = { className: 'bar' }; + const extraProps = addSaveProps( { className: 'foo' }, blockSettings, attributes ); + + expect( extraProps.className ).toBe( 'foo bar' ); + } ); + } ); +} ); diff --git a/blocks/library/html/index.js b/blocks/library/html/index.js index 38ff029c7df5b..a5f6278dee089 100644 --- a/blocks/library/html/index.js +++ b/blocks/library/html/index.js @@ -31,6 +31,10 @@ registerBlockType( 'core/html', { supportHTML: false, + supports: { + customClassName: false, + }, + attributes: { content: { type: 'string', diff --git a/blocks/library/more/index.js b/blocks/library/more/index.js index 44932a97f336e..5adc1148553cd 100644 --- a/blocks/library/more/index.js +++ b/blocks/library/more/index.js @@ -25,6 +25,10 @@ registerBlockType( 'core/more', { supportHTML: false, + supports: { + customClassName: false, + }, + attributes: { customText: { type: 'string', diff --git a/blocks/library/shortcode/index.js b/blocks/library/shortcode/index.js index f2f67db907d6d..c3c2f858efd02 100644 --- a/blocks/library/shortcode/index.js +++ b/blocks/library/shortcode/index.js @@ -35,6 +35,10 @@ registerBlockType( 'core/shortcode', { supportHTML: false, + supports: { + customClassName: false, + }, + edit: withInstanceId( ( { attributes, setAttributes, instanceId, focus } ) => { const inputId = `blocks-shortcode-input-${ instanceId }`; diff --git a/components/slot-fill/fill.js b/components/slot-fill/fill.js index 91824106fe277..628e434bbf27d 100644 --- a/components/slot-fill/fill.js +++ b/components/slot-fill/fill.js @@ -8,18 +8,25 @@ import { noop } from 'lodash'; */ import { Component, createPortal } from '@wordpress/element'; -/** - * Internal dependencies - */ -import withInstanceId from '../higher-order/with-instance-id'; +let occurrences = 0; class Fill extends Component { + componentWillMount() { + this.occurrence = ++occurrences; + } + componentDidMount() { const { registerFill = noop } = this.context; registerFill( this.props.name, this ); } + componentWillUpdate() { + if ( ! this.occurrence ) { + this.occurrence = ++occurrences; + } + } + componentWillUnmount() { const { unregisterFill = noop } = this.context; @@ -47,6 +54,10 @@ class Fill extends Component { } } + resetOccurrence() { + this.occurrence = null; + } + render() { const { getSlot = noop } = this.context; const { name, children } = this.props; @@ -65,4 +76,4 @@ Fill.contextTypes = { unregisterFill: noop, }; -export default withInstanceId( Fill ); +export default Fill; diff --git a/components/slot-fill/provider.js b/components/slot-fill/provider.js index f746421e2986a..55a3fa8be2442 100644 --- a/components/slot-fill/provider.js +++ b/components/slot-fill/provider.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { pick, without, noop } from 'lodash'; +import { pick, sortBy, forEach, without, noop } from 'lodash'; /** * WordPress dependencies @@ -61,6 +61,7 @@ class SlotFillProvider extends Component { this.fills[ name ], instance ); + this.resetFillOccurrence( name ); this.forceUpdateSlot( name ); } @@ -69,15 +70,19 @@ class SlotFillProvider extends Component { } getFills( name ) { - return this.fills[ name ]; + return sortBy( this.fills[ name ], 'occurrence' ); + } + + resetFillOccurrence( name ) { + forEach( this.fills[ name ], ( instance ) => { + instance.resetOccurrence(); + } ); } forceUpdateFills( name ) { - if ( this.fills.hasOwnProperty( name ) ) { - this.fills[ name ].forEach( ( instance ) => { - instance.forceUpdate(); - } ); - } + forEach( this.fills[ name ], ( instance ) => { + instance.forceUpdate(); + } ); } forceUpdateSlot( name ) { diff --git a/components/slot-fill/slot.js b/components/slot-fill/slot.js index 4214e41756ea8..8dc373509a1d1 100644 --- a/components/slot-fill/slot.js +++ b/components/slot-fill/slot.js @@ -55,7 +55,7 @@ class Slot extends Component { return (
{ map( getFills( name ), ( fill ) => { - const fillKey = fill.props.instanceId; + const fillKey = fill.occurrence; return Children.map( fill.props.children, ( child, childIndex ) => { if ( ! child || isString( child ) ) { return child; diff --git a/components/slot-fill/test/slot.js b/components/slot-fill/test/slot.js index a2253a3427bb1..95b6023375836 100644 --- a/components/slot-fill/test/slot.js +++ b/components/slot-fill/test/slot.js @@ -26,7 +26,7 @@ class Filler extends Component { render() { return [