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

Editor: Deprecate layout attribute #11029

Merged
merged 4 commits into from
Oct 29, 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
4 changes: 1 addition & 3 deletions docs/data/data-core-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -930,8 +930,7 @@ be placed. Defaults to the last index.

*Returns*

Insertion point object with `rootClientId`, `layout`,
`index`.
Insertion point object with `rootClientId`, `index`.

### isBlockInsertionPointVisible

Expand Down Expand Up @@ -1512,7 +1511,6 @@ to a new index.
* clientId: The client ID of the block.
* fromRootClientId: Root client ID source.
* toRootClientId: Root client ID destination.
* layout: Layout to move the block into.
* index: The index to move the block into.

### insertBlock
Expand Down
63 changes: 48 additions & 15 deletions packages/block-library/src/columns/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { times, property, omit } from 'lodash';
import { times } from 'lodash';
import classnames from 'classnames';
import memoize from 'memize';

Expand Down Expand Up @@ -39,6 +39,32 @@ const getColumnsTemplate = memoize( ( columns ) => {
return times( columns, () => [ 'core/column' ] );
} );

/**
* Given an HTML string for a deprecated columns inner block, returns the
* column index to which the migrated inner block should be assigned. Returns
* undefined if the inner block was not assigned to a column.
*
* @param {string} originalContent Deprecated Columns inner block HTML.
*
* @return {?number} Column to which inner block is to be assigned.
*/
function getDeprecatedLayoutColumn( originalContent ) {
let { doc } = getDeprecatedLayoutColumn;
if ( ! doc ) {
doc = document.implementation.createHTMLDocument( '' );
getDeprecatedLayoutColumn.doc = doc;
}

let columnMatch;

doc.body.innerHTML = originalContent;
for ( const classListItem of doc.body.firstChild.classList ) {
if ( ( columnMatch = classListItem.match( /^layout-column-(\d+)$/ ) ) ) {
return Number( columnMatch[ 1 ] ) - 1;
}
}
}

export const name = 'core/columns';

export const settings = {
Expand Down Expand Up @@ -70,31 +96,38 @@ export const settings = {
},
},
isEligible( attributes, innerBlocks ) {
return innerBlocks.some( property( [ 'attributes', 'layout' ] ) );
},
migrate( attributes, innerBlocks ) {
function withoutLayout( block ) {
return {
...block,
attributes: omit( block.attributes, [ 'layout' ] ),
};
// Since isEligible is called on every valid instance of the
// Columns block and a deprecation is the unlikely case due to
// its subsequent migration, optimize for the `false` condition
// by performing a naive, inaccurate pass at inner blocks.
const isFastPassEligible = innerBlocks.some( ( innerBlock ) => (
/layout-column-\d+/.test( innerBlock.originalContent )
) );

if ( ! isFastPassEligible ) {
return false;
}

// Only if the fast pass is considered eligible is the more
// accurate, durable, slower condition performed.
return innerBlocks.some( ( innerBlock ) => (
getDeprecatedLayoutColumn( innerBlock.originalContent ) !== undefined
) );
},
migrate( attributes, innerBlocks ) {
const columns = innerBlocks.reduce( ( result, innerBlock ) => {
const { layout } = innerBlock.attributes;
const { originalContent } = innerBlock;

let columnIndex, columnMatch;
if ( layout && ( columnMatch = layout.match( /^column-(\d+)$/ ) ) ) {
columnIndex = Number( columnMatch[ 1 ] ) - 1;
} else {
let columnIndex = getDeprecatedLayoutColumn( originalContent );
if ( columnIndex === undefined ) {
columnIndex = 0;
}

if ( ! result[ columnIndex ] ) {
result[ columnIndex ] = [];
}

result[ columnIndex ].push( withoutLayout( innerBlock ) );
result[ columnIndex ].push( innerBlock );

return result;
}, [] );
Expand Down
18 changes: 3 additions & 15 deletions packages/editor/src/components/block-drop-zone/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* External Dependencies
*/
import { castArray } from 'lodash';
import classnames from 'classnames';

/**
Expand All @@ -10,7 +9,6 @@ import classnames from 'classnames';
import { DropZone } from '@wordpress/components';
import {
rawHandler,
cloneBlock,
getBlockTransforms,
findTransform,
} from '@wordpress/blocks';
Expand Down Expand Up @@ -132,26 +130,16 @@ export default compose(

return {
insertBlocks( blocks, index ) {
const { rootClientId, layout } = ownProps;

if ( layout ) {
// A block's transform function may return a single
// transformed block or an array of blocks, so ensure
// to first coerce to an array before mapping to inject
// the layout attribute.
blocks = castArray( blocks ).map( ( block ) => (
cloneBlock( block, { layout } )
) );
}
const { rootClientId } = ownProps;

insertBlocks( blocks, index, rootClientId );
},
updateBlockAttributes( ...args ) {
updateBlockAttributes( ...args );
},
moveBlockToPosition( srcClientId, srcRootClientId, dstIndex ) {
const { rootClientId: dstRootClientId, layout } = ownProps;
moveBlockToPosition( srcClientId, srcRootClientId, dstRootClientId, layout, dstIndex );
const { rootClientId: dstRootClientId } = ownProps;
moveBlockToPosition( srcClientId, srcRootClientId, dstRootClientId, dstIndex );
},
};
} ),
Expand Down
6 changes: 0 additions & 6 deletions packages/editor/src/components/block-list-appender/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import Inserter from '../inserter';

function BlockListAppender( {
blockClientIds,
layout,
isGroupedByLayout,
rootClientId,
canInsertDefaultBlock,
isLocked,
Expand All @@ -30,15 +28,12 @@ function BlockListAppender( {
return null;
}

const defaultLayout = isGroupedByLayout ? layout : undefined;

if ( canInsertDefaultBlock ) {
return (
<IgnoreNestedEvents childHandledEvents={ [ 'onFocus', 'onClick', 'onKeyDown' ] }>
<DefaultBlockAppender
rootClientId={ rootClientId }
lastBlockClientId={ last( blockClientIds ) }
layout={ defaultLayout }
/>
</IgnoreNestedEvents>
);
Expand All @@ -48,7 +43,6 @@ function BlockListAppender( {
<div className="block-list-appender">
<Inserter
rootClientId={ rootClientId }
layout={ defaultLayout }
renderToggle={ ( { onToggle, disabled, isOpen } ) => (
<Button
aria-label={ __( 'Add block' ) }
Expand Down
14 changes: 2 additions & 12 deletions packages/editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import classnames from 'classnames';
import { get, reduce, size, castArray, first, last } from 'lodash';
import { get, reduce, size, first, last } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -16,7 +16,6 @@ import {
} from '@wordpress/dom';
import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes';
import {
cloneBlock,
getBlockType,
getSaveElement,
isReusableBlock,
Expand Down Expand Up @@ -367,7 +366,6 @@ export class BlockListBlock extends Component {
isLast,
clientId,
rootClientId,
layout,
isSelected,
isPartOfMultiSelection,
isFirstMultiSelected,
Expand Down Expand Up @@ -492,7 +490,6 @@ export class BlockListBlock extends Component {
<BlockInsertionPoint
clientId={ clientId }
rootClientId={ rootClientId }
layout={ layout }
canShowInserter={ canShowInBetweenInserter }
onInsert={ this.hideHoverEffects }
/>
Expand All @@ -501,7 +498,6 @@ export class BlockListBlock extends Component {
index={ order }
clientId={ clientId }
rootClientId={ rootClientId }
layout={ layout }
/>
{ shouldRenderMovers && (
<BlockMover
Expand Down Expand Up @@ -560,7 +556,6 @@ export class BlockListBlock extends Component {
<InserterWithShortcuts
clientId={ clientId }
rootClientId={ rootClientId }
layout={ layout }
onToggle={ this.selectOnOpen }
/>
</div>
Expand Down Expand Up @@ -655,8 +650,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps ) => {
selectBlock( clientId, initialPosition );
},
onInsertBlocks( blocks, index ) {
const { rootClientId, layout } = ownProps;
blocks = blocks.map( ( block ) => cloneBlock( block, { layout } ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

cloneBlock does more than just adding the layout, just want to ensure we don't break anything here. (For example, it clones the inner blocks too etc...)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm fairly certain this was only used to inject the layout, but I'll double-check.

Copy link
Member Author

Choose a reason for hiding this comment

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

The only thing which should change is the client ID, which is not something which should be relied upon anyways, and certainly not so for a block which is created but yet to be inserted.

const { rootClientId } = ownProps;
insertBlocks( blocks, index, rootClientId );
},
onInsertDefaultBlockAfter() {
Expand All @@ -670,10 +664,6 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps ) => {
mergeBlocks( ...args );
},
onReplace( blocks ) {
const { layout } = ownProps;
blocks = castArray( blocks ).map( ( block ) => (
cloneBlock( block, { layout } )
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question as above.

) );
replaceBlocks( [ ownProps.clientId ], blocks );
},
onMetaChange( meta ) {
Expand Down
22 changes: 2 additions & 20 deletions packages/editor/src/components/block-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
sortBy,
throttle,
} from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
Expand Down Expand Up @@ -188,23 +187,12 @@ class BlockList extends Component {
render() {
const {
blockClientIds,
layout,
isGroupedByLayout,
rootClientId,
isDraggable,
} = this.props;

let defaultLayout;
if ( isGroupedByLayout ) {
defaultLayout = layout;
}

const classes = classnames( 'editor-block-list__layout', {
[ `layout-${ layout }` ]: layout,
} );

return (
<div className={ classes }>
<div className="editor-block-list__layout">
{ map( blockClientIds, ( clientId, blockIndex ) => (
<BlockListBlock
key={ 'block-' + clientId }
Expand All @@ -214,18 +202,12 @@ class BlockList extends Component {
onSelectionStart={ this.onSelectionStart }
onShiftSelection={ this.onShiftSelection }
rootClientId={ rootClientId }
layout={ defaultLayout }
isFirst={ blockIndex === 0 }
isLast={ blockIndex === blockClientIds.length - 1 }
isDraggable={ isDraggable }
/>
) ) }

<BlockListAppender
rootClientId={ rootClientId }
layout={ layout }
isGroupedByLayout={ isGroupedByLayout }
/>
<BlockListAppender rootClientId={ rootClientId } />
</div>
);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/editor/src/components/block-list/insertion-point.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ class BlockInsertionPoint extends Component {
}

onClick() {
const { layout, rootClientId, index, ...props } = this.props;
props.insertDefaultBlock( { layout }, rootClientId, index );
const { rootClientId, index, ...props } = this.props;
props.insertDefaultBlock( undefined, rootClientId, index );
props.startTyping();
this.onBlurInserter();
if ( props.onInsert ) {
Expand Down
14 changes: 4 additions & 10 deletions packages/editor/src/components/default-block-appender/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export function DefaultBlockAppender( {
onAppend,
showPrompt,
placeholder,
layout,
rootClientId,
} ) {
if ( isLocked || ! isVisible ) {
Expand All @@ -51,7 +50,7 @@ export function DefaultBlockAppender( {

return (
<div data-root-client-id={ rootClientId || '' } className="wp-block editor-default-block-appender">
<BlockDropZone rootClientId={ rootClientId } layout={ layout } />
<BlockDropZone rootClientId={ rootClientId } />
<input
role="button"
aria-label={ __( 'Add block' ) }
Expand All @@ -61,7 +60,7 @@ export function DefaultBlockAppender( {
onFocus={ onAppend }
value={ showPrompt ? value : '' }
/>
<InserterWithShortcuts rootClientId={ rootClientId } layout={ layout } />
<InserterWithShortcuts rootClientId={ rootClientId } />
<Inserter position="top right" />
</div>
);
Expand Down Expand Up @@ -91,14 +90,9 @@ export default compose(

return {
onAppend() {
const { layout, rootClientId } = ownProps;
const { rootClientId } = ownProps;

let attributes;
if ( layout ) {
attributes = { layout };
}

insertDefaultBlock( attributes, rootClientId );
insertDefaultBlock( undefined, rootClientId );
startTyping();
},
};
Expand Down
Loading