Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extensibility: Define customClassname behavior as filtered blocks support #3472

Merged
merged 7 commits into from
Nov 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions blocks/api/factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 0 additions & 5 deletions blocks/api/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
19 changes: 10 additions & 9 deletions blocks/api/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}

/**
Expand Down
4 changes: 3 additions & 1 deletion blocks/api/test/factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
25 changes: 0 additions & 25 deletions blocks/api/test/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,31 +155,6 @@ describe( 'block parser', () => {
topic: 'none',
} );
} );

it( 'should parse the className if the block supports it', () => {
const blockType = {
attributes: {},
};

const innerHTML = '<div class="chicken">Ribs</div>';
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 = '<div class="chicken">Ribs</div>';
const attrs = { className: 'chicken' };

expect( getBlockAttributes( blockType, innerHTML, attrs ) ).toEqual( {} );
} );
} );

describe( 'createBlockWithFallback', () => {
Expand Down
75 changes: 57 additions & 18 deletions blocks/api/test/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ describe( 'blocks', () => {
save: noop,
category: 'common',
title: 'block title',
attributes: {},
attributes: {
className: {
type: 'string',
},
},
} );
} );

Expand Down Expand Up @@ -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',
},
},
} );
} );

Expand All @@ -177,7 +186,11 @@ describe( 'blocks', () => {
category: 'common',
title: 'block title',
icon: 'block-default',
attributes: {},
attributes: {
className: {
type: 'string',
},
},
} );
} );
} );
Expand All @@ -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( {
Expand All @@ -207,7 +226,11 @@ describe( 'blocks', () => {
category: 'common',
title: 'block title',
icon: 'block-default',
attributes: {},
attributes: {
className: {
type: 'string',
},
},
} );
expect( getBlockTypes() ).toEqual( [] );
} );
Expand Down Expand Up @@ -250,7 +273,11 @@ describe( 'blocks', () => {
category: 'common',
title: 'block title',
icon: 'block-default',
attributes: {},
attributes: {
className: {
type: 'string',
},
},
} );
} );

Expand All @@ -264,7 +291,11 @@ describe( 'blocks', () => {
category: 'common',
title: 'block title',
icon: 'block-default',
attributes: {},
attributes: {
className: {
type: 'string',
},
},
} );
} );
} );
Expand All @@ -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',
Expand All @@ -294,7 +329,11 @@ describe( 'blocks', () => {
category: 'common',
title: 'block title',
icon: 'block-default',
attributes: {},
attributes: {
className: {
type: 'string',
},
},
},
] );
} );
Expand Down
2 changes: 1 addition & 1 deletion blocks/block-edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function BlockEdit( props ) {
Edit = blockType.edit || blockType.save;
}

return applyFilters( 'BlockEdit', <Edit { ...editProps } />, props );
return applyFilters( 'BlockEdit', <Edit key="edit" { ...editProps } />, props );
}

export default BlockEdit;
5 changes: 2 additions & 3 deletions blocks/hooks/anchor.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { assign } from 'lodash';
/**
* WordPress dependencies
*/
import { cloneElement } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
Expand Down Expand Up @@ -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' } ),
<InspectorControls key="inspector">
element,
<InspectorControls key="inspector-anchor">
<InspectorControls.TextControl
label={ __( 'HTML Anchor' ) }
help={ __( 'Anchors lets you link directly to a section on a page.' ) }
Expand Down
89 changes: 89 additions & 0 deletions blocks/hooks/custom-class-name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/**
* External dependencies
*/
import { assign } from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { hasBlockSupport } from '../api';
import InspectorControls from '../inspector-controls';

/**
* Filters registered block settings, extending attributes with anchor using ID
* of the first node
*
* @param {Object} settings Original block settings
* @return {Object} Filtered block settings
*/
export function addAttribute( settings ) {
if ( hasBlockSupport( settings, 'customClassName', true ) ) {
// Use Lodash's assign to gracefully handle if attributes are undefined
settings.attributes = assign( settings.attributes, {
className: {
type: 'string',
},
} );
}

return settings;
}

/**
* Override the default edit UI to include a new block inspector control for
* assigning the anchor ID, if block supports anchor
*
* @param {Element} element Original edit element
* @param {Object} props Props passed to BlockEdit
* @return {Element} Filtered edit element
*/
export function addInspectorControl( element, props ) {
if ( hasBlockSupport( props.name, 'customClassName', true ) && props.focus ) {
element = [
element,
<InspectorControls key="inspector-custom-class-name">
<InspectorControls.TextControl
label={ __( 'Additional CSS Class' ) }
value={ props.attributes.className || '' }
onChange={ ( nextValue ) => {
props.setAttributes( {
className: nextValue,
} );
} }
/>
</InspectorControls>,
];
}

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 );
}
2 changes: 2 additions & 0 deletions blocks/hooks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import createHooks from '@wordpress/hooks';
* Internal dependencies
*/
import anchor from './anchor';
import customClassName from './custom-class-name';

const hooks = createHooks();

Expand Down Expand Up @@ -45,3 +46,4 @@ export {
};

anchor( hooks );
customClassName( hooks );
2 changes: 1 addition & 1 deletion blocks/hooks/test/anchor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Copy link
Member

@gziolo gziolo Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, this is the issue with testing only for non-existence. It is very easy to satisfy 😃


expect( extraProps ).not.toHaveProperty( 'id' );
} );
Expand Down
Loading