Skip to content

Commit

Permalink
Use block label for movers
Browse files Browse the repository at this point in the history
  • Loading branch information
@talldan
talldan committed Oct 28, 2019
1 parent e60a265 commit a5b655a
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 37 deletions.
50 changes: 43 additions & 7 deletions packages/block-editor/src/components/block-mover/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import classnames from 'classnames';
*/
import { __ } from '@wordpress/i18n';
import { IconButton } from '@wordpress/components';
import { getBlockType } from '@wordpress/blocks';
import { getBlockType, __experimentalGetBlockLabel } from '@wordpress/blocks';
import { Component } from '@wordpress/element';
import { withSelect, withDispatch } from '@wordpress/data';
import { withInstanceId, compose } from '@wordpress/compose';
import deprecated from '@wordpress/deprecated';

/**
* Internal dependencies
Expand Down Expand Up @@ -44,13 +45,38 @@ export class BlockMover extends Component {
}

render() {
const { onMoveUp, onMoveDown, isFirst, isLast, isDraggable, onDragStart, onDragEnd, clientIds, blockElementId, blockType, firstIndex, isLocked, instanceId, isHidden, rootClientId } = this.props;
const {
onMoveUp,
onMoveDown,
isFirst,
isLast,
isDraggable,
onDragStart,
onDragEnd,
clientIds,
blockElementId,
firstIndex,
isLocked,
instanceId,
isHidden,
rootClientId,
// `blockType` is now deprecated. If it's defined use it as the default
// for its replacement, `blockLabel`.
blockType,
blockLabel = ( blockType ? blockType.title : undefined ),
} = this.props;
const { isFocused } = this.state;
const blocksCount = castArray( clientIds ).length;
if ( isLocked || ( isFirst && isLast && ! rootClientId ) ) {
return null;
}

if ( blockType ) {
deprecated( 'wp.blockEditor.BlockMover blockType prop', {
alternative: 'blockLabel prop',
} );
}

// We emulate a disabled state because forcefully applying the `disabled`
// attribute on the button while it has focus causes the screen to change
// to an unfocused state (body as active element) without firing blur on,
Expand Down Expand Up @@ -90,7 +116,7 @@ export class BlockMover extends Component {
{
getBlockMoverDescription(
blocksCount,
blockType && blockType.title,
blockLabel,
firstIndex,
isFirst,
isLast,
Expand All @@ -102,7 +128,7 @@ export class BlockMover extends Component {
{
getBlockMoverDescription(
blocksCount,
blockType && blockType.title,
blockLabel,
firstIndex,
isFirst,
isLast,
Expand All @@ -117,17 +143,27 @@ export class BlockMover extends Component {

export default compose(
withSelect( ( select, { clientIds } ) => {
const { getBlock, getBlockIndex, getTemplateLock, getBlockRootClientId, getBlockOrder } = select( 'core/block-editor' );
const {
getBlock,
getBlockIndex,
getTemplateLock,
getBlockRootClientId,
getBlockOrder,
getBlockAttributes,
} = select( 'core/block-editor' );

const normalizedClientIds = castArray( clientIds );
const firstClientId = first( normalizedClientIds );
const block = getBlock( firstClientId );
const rootClientId = getBlockRootClientId( first( normalizedClientIds ) );
const rootClientId = getBlockRootClientId( firstClientId );
const blockOrder = getBlockOrder( rootClientId );
const firstIndex = getBlockIndex( firstClientId, rootClientId );
const lastIndex = getBlockIndex( last( normalizedClientIds ), rootClientId );
const blockType = getBlockType( block.name );
const blockAttributes = getBlockAttributes( firstClientId );

return {
blockType: block ? getBlockType( block.name ) : null,
blockLabel: __experimentalGetBlockLabel( blockType, blockAttributes ),
isLocked: getTemplateLock( rootClientId ) === 'all',
rootClientId,
firstIndex,
Expand Down
20 changes: 10 additions & 10 deletions packages/block-editor/src/components/block-mover/mover-description.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { __, _n, sprintf } from '@wordpress/i18n';
* Return a label for the block movement controls depending on block position.
*
* @param {number} selectedCount Number of blocks selected.
* @param {string} type Block type - in the case of a single block, should
* define its 'type'. I.e. 'Text', 'Heading', 'Image' etc.
* @param {string} blockLabel Block label, generally the block type, but sometimes this
* also includes display text (e.g. 'Header: My Life Story')
* @param {number} firstIndex The index (position - 1) of the first block selected.
* @param {boolean} isFirst This is the first block.
* @param {boolean} isLast This is the last block.
Expand All @@ -17,7 +17,7 @@ import { __, _n, sprintf } from '@wordpress/i18n';
*
* @return {string} Label for the block movement controls.
*/
export function getBlockMoverDescription( selectedCount, type, firstIndex, isFirst, isLast, dir ) {
export function getBlockMoverDescription( selectedCount, blockLabel, firstIndex, isFirst, isLast, dir ) {
const position = ( firstIndex + 1 );

if ( selectedCount > 1 ) {
Expand All @@ -26,15 +26,15 @@ export function getBlockMoverDescription( selectedCount, type, firstIndex, isFir

if ( isFirst && isLast ) {
// translators: %s: Type of block (i.e. Text, Image etc)
return sprintf( __( 'Block %s is the only block, and cannot be moved' ), type );
return sprintf( __( "Block '%s' is the only block, and cannot be moved" ), blockLabel );
}

if ( dir > 0 && ! isLast ) {
// moving down
return sprintf(
// translators: 1: Type of block (i.e. Text, Image etc), 2: Position of selected block, 3: New position
__( 'Move %1$s block from position %2$d down to position %3$d' ),
type,
__( "Move '%1$s' block from position %2$d down to position %3$d" ),
blockLabel,
position,
( position + 1 )
);
Expand All @@ -43,15 +43,15 @@ export function getBlockMoverDescription( selectedCount, type, firstIndex, isFir
if ( dir > 0 && isLast ) {
// moving down, and is the last item
// translators: %s: Type of block (i.e. Text, Image etc)
return sprintf( __( 'Block %s is at the end of the content and can’t be moved down' ), type );
return sprintf( __( "Block '%s' is at the end of the content and can’t be moved down" ), blockLabel );
}

if ( dir < 0 && ! isFirst ) {
// moving up
return sprintf(
// translators: 1: Type of block (i.e. Text, Image etc), 2: Position of selected block, 3: New position
__( 'Move %1$s block from position %2$d up to position %3$d' ),
type,
__( "Move '%1$s' block from position %2$d up to position %3$d" ),
blockLabel,
position,
( position - 1 )
);
Expand All @@ -60,7 +60,7 @@ export function getBlockMoverDescription( selectedCount, type, firstIndex, isFir
if ( dir < 0 && isFirst ) {
// moving up, and is the first item
// translators: %s: Type of block (i.e. Text, Image etc)
return sprintf( __( 'Block %s is at the beginning of the content and can’t be moved up' ), type );
return sprintf( __( "Block '%s' is at the beginning of the content and can’t be moved up" ), blockLabel );
}
}

Expand Down
31 changes: 22 additions & 9 deletions packages/block-editor/src/components/block-mover/test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,33 @@ describe( 'BlockMover', () => {
describe( 'basic rendering', () => {
const selectedClientIds = [ 'IisClientId', 'IisOtherClientId' ];

const blockType = {
title: 'yolo-block',
};
const blockLabel = 'Header: Test Header';

it( 'should not render if the editor is locked', () => {
const wrapper = shallow( <BlockMover isLocked /> );
expect( wrapper.type() ).toBe( null );
} );

it( 'should log a deprecation warning when the blockType prop is used and to still be backwards compatible', () => {
const blockMover = shallow(
<BlockMover
clientIds={ [ 'singleClientId' ] }
blockType={ { title: 'Header' } }
firstIndex={ 0 }
instanceId={ 1 } />
);

expect( console ).toHaveWarned();

const moveUpDesc = blockMover.childAt( 3 );
expect( moveUpDesc.text() ).toBe( "Move 'Header' block from position 1 up to position 0" );
} );

it( 'should render three icons with the following props', () => {
const blockMover = shallow(
<BlockMover
clientIds={ selectedClientIds }
blockType={ blockType }
blockLabel={ blockLabel }
firstIndex={ 0 }
instanceId={ 1 } />
);
Expand Down Expand Up @@ -69,7 +82,7 @@ describe( 'BlockMover', () => {
const blockMover = shallow(
<BlockMover
clientIds={ selectedClientIds }
blockType={ blockType }
blockLabel={ blockLabel }
onMoveUp={ onMoveUp }
firstIndex={ 0 }
/>
Expand All @@ -84,7 +97,7 @@ describe( 'BlockMover', () => {
const blockMover = shallow(
<BlockMover
clientIds={ selectedClientIds }
blockType={ blockType }
blockLabel={ blockLabel }
onDragStart={ onDragStart }
onDragEnd={ onDragEnd }
firstIndex={ 0 }
Expand All @@ -100,7 +113,7 @@ describe( 'BlockMover', () => {
const blockMover = shallow(
<BlockMover
clientIds={ selectedClientIds }
blockType={ blockType }
blockLabel={ blockLabel }
onMoveDown={ onMoveDown }
firstIndex={ 0 }
/>
Expand All @@ -113,7 +126,7 @@ describe( 'BlockMover', () => {
const blockMover = shallow(
<BlockMover
clientIds={ selectedClientIds }
blockType={ blockType }
blockLabel={ blockLabel }
isDraggable={ false }
/>
);
Expand All @@ -127,7 +140,7 @@ describe( 'BlockMover', () => {
const blockMover = shallow(
<BlockMover
clientIds={ selectedClientIds }
blockType={ blockType }
blockLabel={ blockLabel }
onMoveDown={ onMoveDown }
isLast
firstIndex={ 0 }
Expand Down
22 changes: 11 additions & 11 deletions packages/block-editor/src/components/block-mover/test/mover-description.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,63 +8,63 @@ describe( 'block mover', () => {
dirDown = 1;

describe( 'getBlockMoverDescription', () => {
const type = 'TestType';
const label = 'Header: Writing Tests Considered Harmful';

it( 'Should generate a title for the first item moving up', () => {
expect( getBlockMoverDescription(
1,
type,
label,
0,
true,
false,
dirUp,
) ).toBe(
`Block ${ type } is at the beginning of the content and can’t be moved up`
`Block '${ label }' is at the beginning of the content and can’t be moved up`
);
} );

it( 'Should generate a title for the last item moving down', () => {
expect( getBlockMoverDescription(
1,
type,
label,
3,
false,
true,
dirDown,
) ).toBe( `Block ${ type } is at the end of the content and can’t be moved down` );
) ).toBe( `Block '${ label }' is at the end of the content and can’t be moved down` );
} );

it( 'Should generate a title for the second item moving up', () => {
expect( getBlockMoverDescription(
1,
type,
label,
1,
false,
false,
dirUp,
) ).toBe( `Move ${ type } block from position 2 up to position 1` );
) ).toBe( `Move '${ label }' block from position 2 up to position 1` );
} );

it( 'Should generate a title for the second item moving down', () => {
expect( getBlockMoverDescription(
1,
type,
label,
1,
false,
false,
dirDown,
) ).toBe( `Move ${ type } block from position 2 down to position 3` );
) ).toBe( `Move '${ label }' block from position 2 down to position 3` );
} );

it( 'Should generate a title for the only item in the list', () => {
expect( getBlockMoverDescription(
1,
type,
label,
0,
true,
true,
dirDown,
) ).toBe( `Block ${ type } is the only block, and cannot be moved` );
) ).toBe( `Block '${ label }' is the only block, and cannot be moved` );
} );
} );

Expand Down

0 comments on commit a5b655a

Please sign in to comment.