Skip to content

Commit

Permalink
Fix deletion of custom block classes (#8232)
Browse files Browse the repository at this point in the history
* Fix deletion of custom block classes

If you add a custom class to a block and then remove that custom class it would flag the block as invalid. The reason is that the custom class became part of the blocks ‘default’ attributes, and it would then add it back and this then the expected HTML wouldn’t match the current HTML.

This change looks at the difference between the default class and the inner HTML.

The unit tests have been beefed up to cope with several edge case situations
  • Loading branch information
johngodley authored Aug 11, 2018
1 parent 97dd134 commit b5490a6
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 17 deletions.
27 changes: 12 additions & 15 deletions packages/editor/src/hooks/custom-class-name.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { assign, difference, compact } from 'lodash';
import { assign, difference, omit } from 'lodash';
import classnames from 'classnames';

/**
Expand Down Expand Up @@ -134,20 +134,17 @@ export function getHTMLRootElementClasses( innerHTML ) {
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;
// attributes, with the exception of `className`. This will determine
// the default set of classes. From there, any difference in innerHTML
// can be considered as custom classes.
const attributesSansClassName = omit( blockAttributes, [ 'className' ] );
const serialized = getSaveContent( blockType, attributesSansClassName );
const defaultClasses = getHTMLRootElementClasses( serialized );
const actualClasses = getHTMLRootElementClasses( innerHTML );
const customClasses = difference( actualClasses, defaultClasses );

if ( customClasses.length ) {
blockAttributes.className = customClasses.join( ' ' );
} else {
delete blockAttributes.className;
}
Expand Down
44 changes: 42 additions & 2 deletions packages/editor/src/hooks/test/custom-class-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getHTMLRootElementClasses } from '../custom-class-name';

describe( 'custom className', () => {
const blockSettings = {
save: () => <div />,
save: () => <div className="default" />,
category: 'common',
title: 'block title',
};
Expand Down Expand Up @@ -95,7 +95,7 @@ describe( 'custom className', () => {
const attributes = addParsedDifference(
{ className: 'foo' },
blockSettings,
'<div class="foo bar baz"></div>'
'<div class="default foo bar baz"></div>'
);

expect( attributes.className ).toBe( 'foo bar baz' );
Expand All @@ -110,5 +110,45 @@ describe( 'custom className', () => {

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

it( 'should add a custom class name to an element without a class', () => {
const attributes = addParsedDifference(
{},
blockSettings,
'<div class="default foo"></div>'
);

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

it( 'should remove the custom class and retain default class', () => {
const attributes = addParsedDifference(
{ className: 'custom1 custom2' },
blockSettings,
'<div class="default custom1"></div>'
);

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

it( 'should remove the custom class from an element originally without a class', () => {
const attributes = addParsedDifference(
{ className: 'foo' },
blockSettings,
'<div></div>'
);

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

it( 'should remove the custom classes and retain default and other custom class', () => {
const attributes = addParsedDifference(
{ className: 'custom1 custom2 custom3' },
blockSettings,
'<div class="default custom1 custom3"></div>'
);

expect( attributes.className ).toBe( 'custom1 custom3' );
} );
} );
} );

0 comments on commit b5490a6

Please sign in to comment.