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

Parser: Parse superfluous classes as custom classes #7538

Merged
merged 2 commits into from
Jun 28, 2018
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
1 change: 1 addition & 0 deletions blocks/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export {
getBlockDefaultClassName,
getBlockMenuDefaultClassName,
getSaveElement,
getSaveContent,
} from './serializer';
export { isValidBlock } from './validation';
export { getCategories } from './categories';
Expand Down
9 changes: 8 additions & 1 deletion blocks/api/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { castArray, mapValues, omit } from 'lodash';
* WordPress dependencies
*/
import { autop } from '@wordpress/autop';
import { applyFilters } from '@wordpress/hooks';

/**
* Internal dependencies
Expand Down Expand Up @@ -148,7 +149,13 @@ export function getBlockAttributes( blockType, innerHTML, attributes ) {
return getBlockAttribute( attributeKey, attributeSchema, innerHTML, attributes );
} );

return blockAttributes;
return applyFilters(
'blocks.getBlockAttributes',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to document this or keep it "secret" for now :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to document this or keep it "secret" for now :)

I'm fine to document. Will add.

blockAttributes,
blockType,
innerHTML,
attributes
);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions docs/extensibility/extending-blocks.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ Used internally by the default block (paragraph) to exclude the attributes from

Used to filters an individual transform result from block transformation. All of the original blocks are passed, since transformations are many-to-many, not one-to-one.

#### `blocks.getBlockAttributes`

Called immediately after the default parsing of a block's attributes and before validation to allow a plugin to manipulate attribute values in time for validation and/or the initial values rendering of the block in the editor.

#### `editor.BlockEdit`

Used to modify the block's `edit` component. It receives the original block `BlockEdit` component and returns a new wrapped component.
Expand Down
67 changes: 65 additions & 2 deletions editor/hooks/custom-class-name.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { assign } from 'lodash';
import { assign, difference, compact } from 'lodash';
import classnames from 'classnames';

/**
Expand All @@ -11,7 +11,11 @@ import { createHigherOrderComponent, Fragment } from '@wordpress/element';
import { addFilter } from '@wordpress/hooks';
import { TextControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { hasBlockSupport } from '@wordpress/blocks';
import {
hasBlockSupport,
parseWithAttributeSchema,
getSaveContent,
} from '@wordpress/blocks';

/**
* Internal dependencies
Expand Down Expand Up @@ -93,6 +97,65 @@ export function addSaveProps( extraProps, blockType, attributes ) {
return extraProps;
}

/**
* Given an HTML string, returns an array of class names assigned to the root
* element in the markup.
*
* @param {string} innerHTML Markup string from which to extract classes.
*
* @return {string[]} Array of class names assigned to the root element.
*/
export function getHTMLRootElementClasses( innerHTML ) {
innerHTML = `<div data-custom-class-name>${ innerHTML }</div>`;

const parsed = parseWithAttributeSchema( innerHTML, {
type: 'string',
source: 'attribute',
selector: '[data-custom-class-name] > *',
attribute: 'class',
} );

return parsed ? parsed.trim().split( /\s+/ ) : [];
}

/**
* Given a parsed set of block attributes, if the block supports custom class
* names and an unknown class (per the block's serialization behavior) is
* found, the unknown classes are treated as custom classes. This prevents the
* block from being considered as invalid.
*
* @param {Object} blockAttributes Original block attributes.
* @param {Object} blockType Block type settings.
* @param {string} innerHTML Original block markup.
*
* @return {Object} Filtered block attributes.
*/
export function addParsedDifference( blockAttributes, blockType, innerHTML ) {
if ( hasBlockSupport( blockType, 'customClassName', true ) ) {
// To determine difference, serialize block given the known set of
// attributes. If there are classes which are mismatched with the
// incoming HTML of the block, add to filtered result.
const serialized = getSaveContent( blockType, blockAttributes );
const classes = getHTMLRootElementClasses( serialized );
const parsedClasses = getHTMLRootElementClasses( innerHTML );
const customClasses = difference( parsedClasses, classes );

const filteredClassName = compact( [
blockAttributes.className,
...customClasses,
] ).join( ' ' );

if ( filteredClassName ) {
blockAttributes.className = filteredClassName;
} else {
delete blockAttributes.className;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not certain why we're deleting the className attribute in this case? This creates this bug #9991. Can you clarify a bit?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh actually the filsterClassName variable have been removed at some point which is causing the bug.

Copy link
Contributor

Choose a reason for hiding this comment

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

}
}

return blockAttributes;
}

addFilter( 'blocks.registerBlockType', 'core/custom-class-name/attribute', addAttribute );
addFilter( 'editor.BlockEdit', 'core/editor/custom-class-name/with-inspector-control', withInspectorControl );
addFilter( 'blocks.getSaveContent.extraProps', 'core/custom-class-name/save-props', addSaveProps );
addFilter( 'blocks.getBlockAttributes', 'core/custom-class-name/addParsedDifference', addParsedDifference );
62 changes: 55 additions & 7 deletions editor/hooks/test/custom-class-name.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { noop } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -11,11 +6,11 @@ import { applyFilters } from '@wordpress/hooks';
/**
* Internal dependencies
*/
import '../custom-class-name';
import { getHTMLRootElementClasses } from '../custom-class-name';

describe( 'custom className', () => {
const blockSettings = {
save: noop,
save: () => <div />,
category: 'common',
title: 'block title',
};
Expand Down Expand Up @@ -63,4 +58,57 @@ describe( 'custom className', () => {
expect( extraProps.className ).toBe( 'foo bar' );
} );
} );

describe( 'getHTMLRootElementClasses', () => {
it( 'should return an empty array if there are no classes', () => {
const classes = getHTMLRootElementClasses( '<div></div>' );

expect( classes ).toEqual( [] );
} );

it( 'return an array of parsed classes from inner HTML', () => {
const classes = getHTMLRootElementClasses( '<div class=" foo bar "></div>' );

expect( classes ).toEqual( [ 'foo', 'bar' ] );
} );
} );

describe( 'addParsedDifference', () => {
const addParsedDifference = applyFilters.bind( null, 'blocks.getBlockAttributes' );

it( 'should do nothing if the block settings do not define custom className support', () => {
const attributes = addParsedDifference(
{ className: 'foo' },
{
...blockSettings,
supports: {
customClassName: false,
},
},
'<div class="bar baz"></div>'
);

expect( attributes.className ).toBe( 'foo' );
} );

it( 'should inject the className differences from parsed attributes', () => {
const attributes = addParsedDifference(
{ className: 'foo' },
blockSettings,
'<div class="foo bar baz"></div>'
);

expect( attributes.className ).toBe( 'foo bar baz' );
} );

it( 'should assign as undefined if there are no classes', () => {
const attributes = addParsedDifference(
{},
blockSettings,
'<div class=""></div>'
);

expect( attributes.className ).toBeUndefined();
} );
} );
} );