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.

Sign up for GitHub

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

Fix: the description text of block movers for horizontal movement #42393

Merged
merged 9 commits into from
Jul 18, 2022
Merged

Fix: the description text of block movers for horizontal movement #42393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3439c47
Fixed the description text of block movers for horizontal movement
thisissandip May 30, 2021
f3a3ab8
Fix unit test
t-hamano Jul 13, 2022
4dddf73
Remove singular / plural translate variation
t-hamano Jul 13, 2022
48ee4f7
Add unit tests for getMultiBlockMoverDescription
t-hamano Jul 13, 2022
1a78eab
Fix wrong quote
t-hamano Jul 14, 2022
15f13d4
Add RTL support
t-hamano Jul 14, 2022
1e22dfb
Add condition when all blocks are selected
t-hamano Jul 14, 2022
fab4eb6
Fix unit test
t-hamano Jul 14, 2022
0ee7ee3
Update packages/block-editor/src/components/block-mover/mover-descrip…
t-hamano Jul 14, 2022
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
179 changes: 131 additions & 48 deletions packages/block-editor/src/components/block-mover/mover-description.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
/**
* WordPress dependencies
*/
import { __, _n, sprintf, isRTL } from '@wordpress/i18n';
import { __, sprintf, isRTL } from '@wordpress/i18n';

const getMovementDirection = ( moveDirection, orientation ) => {
if ( moveDirection === 'up' ) {
if ( orientation === 'horizontal' ) {
return isRTL() ? 'right' : 'left';
}
return 'up';
} else if ( moveDirection === 'down' ) {
if ( orientation === 'horizontal' ) {
return isRTL() ? 'left' : 'right';
}
return 'down';
}
return null;
};

/**
* Return a label for the block movement controls depending on block position.
Expand Down Expand Up @@ -30,28 +45,14 @@ export function getBlockMoverDescription(
) {
const position = firstIndex + 1;

const getMovementDirection = ( moveDirection ) => {
if ( moveDirection === 'up' ) {
if ( orientation === 'horizontal' ) {
return isRTL() ? 'right' : 'left';
}
return 'up';
} else if ( moveDirection === 'down' ) {
if ( orientation === 'horizontal' ) {
return isRTL() ? 'left' : 'right';
}
return 'down';
}
return null;
};

if ( selectedCount > 1 ) {
return getMultiBlockMoverDescription(
selectedCount,
firstIndex,
isFirst,
isLast,
dir
dir,
orientation
);
}

Expand All @@ -65,7 +66,7 @@ export function getBlockMoverDescription(

if ( dir > 0 && ! isLast ) {
// Moving down.
const movementDirection = getMovementDirection( 'down' );
const movementDirection = getMovementDirection( 'down', orientation );

if ( movementDirection === 'down' ) {
return sprintf(
Expand Down Expand Up @@ -106,7 +107,7 @@ export function getBlockMoverDescription(

if ( dir > 0 && isLast ) {
// Moving down, and is the last item.
const movementDirection = getMovementDirection( 'down' );
const movementDirection = getMovementDirection( 'down', orientation );

if ( movementDirection === 'down' ) {
return sprintf(
Expand Down Expand Up @@ -141,7 +142,7 @@ export function getBlockMoverDescription(

if ( dir < 0 && ! isFirst ) {
// Moving up.
const movementDirection = getMovementDirection( 'up' );
const movementDirection = getMovementDirection( 'up', orientation );

if ( movementDirection === 'up' ) {
return sprintf(
Expand Down Expand Up @@ -180,7 +181,7 @@ export function getBlockMoverDescription(

if ( dir < 0 && isFirst ) {
// Moving up, and is the first item.
const movementDirection = getMovementDirection( 'up' );
const movementDirection = getMovementDirection( 'up', orientation );

if ( movementDirection === 'up' ) {
return sprintf(
Expand Down Expand Up @@ -223,6 +224,8 @@ export function getBlockMoverDescription(
* @param {boolean} isLast This is the last block.
* @param {number} dir Direction of movement (> 0 is considered to be going
* down, < 0 is up).
* @param {string} orientation The orientation of the block movers, vertical or
* horizontal.
*
* @return {string} Label for the block movement controls.
*/
Expand All @@ -231,43 +234,123 @@ export function getMultiBlockMoverDescription(
firstIndex,
isFirst,
isLast,
dir
dir,
orientation
) {
const position = firstIndex + 1;

if ( dir < 0 && isFirst ) {
return __( 'Blocks cannot be moved up as they are already at the top' );
if ( isFirst && isLast ) {
// All blocks are selected
return __( 'All blocks are selected, and cannot be moved' );
}

if ( dir > 0 && ! isLast ) {
// moving down
const movementDirection = getMovementDirection( 'down', orientation );

if ( movementDirection === 'down' ) {
return sprintf(
// translators: 1: Number of selected blocks, 2: Position of selected blocks
__( 'Move %1$d blocks from position %2$d down by one place' ),
selectedCount,
position
);
}

if ( movementDirection === 'left' ) {
return sprintf(
// translators: 1: Number of selected blocks, 2: Position of selected blocks
__( 'Move %1$d blocks from position %2$d left by one place' ),
selectedCount,
position
);
}

if ( movementDirection === 'right' ) {
return sprintf(
// translators: 1: Number of selected blocks, 2: Position of selected blocks
__( 'Move %1$d blocks from position %2$d right by one place' ),
selectedCount,
position
);
}
}

if ( dir > 0 && isLast ) {
return __(
'Blocks cannot be moved down as they are already at the bottom'
);
// moving down, and the selected blocks are the last item
const movementDirection = getMovementDirection( 'down', orientation );

if ( movementDirection === 'down' ) {
return __(
'Blocks cannot be moved down as they are already at the bottom'
);
}

if ( movementDirection === 'left' ) {
return __(
'Blocks cannot be moved left as they are already are at the leftmost position'
);
}

if ( movementDirection === 'right' ) {
return __(
'Blocks cannot be moved right as they are already are at the rightmost position'
);
}
}

if ( dir < 0 && ! isFirst ) {
return sprintf(
// translators: 1: Number of selected blocks, 2: Position of selected blocks
_n(
'Move %1$d block from position %2$d up by one place',
'Move %1$d blocks from position %2$d up by one place',
selectedCount
),
selectedCount,
position
);
// moving up
const movementDirection = getMovementDirection( 'up', orientation );

if ( movementDirection === 'up' ) {
return sprintf(
// translators: 1: Number of selected blocks, 2: Position of selected blocks
__( 'Move %1$d blocks from position %2$d up by one place' ),
selectedCount,
position
);
}

if ( movementDirection === 'left' ) {
return sprintf(
// translators: 1: Number of selected blocks, 2: Position of selected blocks
__( 'Move %1$d blocks from position %2$d left by one place' ),
selectedCount,
position
);
}

if ( movementDirection === 'right' ) {
return sprintf(
// translators: 1: Number of selected blocks, 2: Position of selected blocks
__( 'Move %1$d blocks from position %2$d right by one place' ),
selectedCount,
position
);
}
}

if ( dir > 0 && ! isLast ) {
return sprintf(
// translators: 1: Number of selected blocks, 2: Position of selected blocks
_n(
'Move %1$d block from position %2$d down by one place',
'Move %1$d blocks from position %2$d down by one place',
selectedCount
),
selectedCount,
position
);
if ( dir < 0 && isFirst ) {
// moving up, and the selected blocks are the first item
const movementDirection = getMovementDirection( 'up', orientation );

if ( movementDirection === 'up' ) {
return __(
'Blocks cannot be moved up as they are already at the top'
);
}

if ( movementDirection === 'left' ) {
return __(
'Blocks cannot be moved left as they are already are at the leftmost position'
);
}

if ( movementDirection === 'right' ) {
return __(
'Blocks cannot be moved right as they are already are at the rightmost position'
);
}
}
}
58 changes: 55 additions & 3 deletions packages/block-editor/src/components/block-mover/test/mover-description.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ describe( 'block mover', () => {
true,
negativeDirection
)
).toBe(
'Blocks cannot be moved up as they are already at the top'
);
).toBe( 'All blocks are selected, and cannot be moved' );
} );

it( 'generates a title for a selection of blocks at the bottom', () => {
Expand All @@ -201,5 +199,59 @@ describe( 'block mover', () => {
'Blocks cannot be moved down as they are already at the bottom'
);
} );

it( 'indicates that blocks can be moved left when the orientation is horizontal and the direction is negative', () => {
expect(
getMultiBlockMoverDescription(
4,
1,
false,
true,
negativeDirection,
'horizontal'
)
).toBe( 'Move 4 blocks from position 2 left by one place' );
} );

it( 'indicates that blocks can be moved right when the orientation is horizontal and the direction is positive', () => {
expect(
getMultiBlockMoverDescription(
4,
0,
true,
false,
positiveDirection,
'horizontal'
)
).toBe( 'Move 4 blocks from position 1 right by one place' );
} );

it( 'indicates that blocks cannot be moved left when the orientation is horizontal and a selection of blocks at the left', () => {
expect(
getMultiBlockMoverDescription(
4,
1,
true,
true,
negativeDirection,
'horizontal'
)
).toBe( 'All blocks are selected, and cannot be moved' );
} );

it( 'indicates that blocks cannot be moved right when the orientation is horizontal and the block is the last block', () => {
expect(
getMultiBlockMoverDescription(
4,
2,
false,
true,
positiveDirection,
'horizontal'
)
).toBe(
'Blocks cannot be moved right as they are already are at the rightmost position'
);
} );
} );
} );