Skip to content

Commit

Permalink
RichText: fix RTL keyboard interactions (#15496)
Browse files Browse the repository at this point in the history
  • Loading branch information
ellatrix committed May 15, 2019
1 parent 705257d commit 5cc5660
Show file tree
Hide file tree
Showing 7 changed files with 225 additions and 8 deletions.
6 changes: 4 additions & 2 deletions packages/block-editor/src/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ export class RichText extends Component {
range,
multilineTag: this.multilineTag,
multilineWrapperTags: this.multilineWrapperTags,
prepareEditableTree: this.props.prepareEditableTree,
__unstableIsEditableTree: true,
} );
}
Expand Down Expand Up @@ -731,7 +730,10 @@ export class RichText extends Component {
const { formats, text, start, end } = value;
const { activeFormats = [] } = this.state;
const collapsed = isCollapsed( value );
const isReverse = event.keyCode === LEFT;
// To do: ideally, we should look at visual position instead.
const { direction } = getComputedStyle( this.editableRef );
const reverseKey = direction === 'rtl' ? RIGHT : LEFT;
const isReverse = event.keyCode === reverseKey;

// If the selection is collapsed and at the very start, do nothing if
// navigating backward.
Expand Down
13 changes: 9 additions & 4 deletions packages/block-editor/src/components/writing-flow/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
* Browser constants
*/

const { getSelection } = window;
const { getSelection, getComputedStyle } = window;

/**
* Given an element, returns true if the element is a tabbable text field, or
Expand Down Expand Up @@ -287,6 +287,11 @@ class WritingFlow extends Component {
this.verticalRect = computeCaretRect( target );
}

// In the case of RTL scripts, right means previous and left means next,
// which is the exact reverse of LTR.
const { direction } = getComputedStyle( target );
const isReverseDir = direction === 'rtl' ? ( ! isReverse ) : isReverse;

if ( isShift ) {
if (
(
Expand Down Expand Up @@ -316,9 +321,9 @@ class WritingFlow extends Component {
placeCaretAtVerticalEdge( closestTabbable, isReverse, this.verticalRect );
event.preventDefault();
}
} else if ( isHorizontal && getSelection().isCollapsed && isHorizontalEdge( target, isReverse ) ) {
const closestTabbable = this.getClosestTabbable( target, isReverse );
placeCaretAtHorizontalEdge( closestTabbable, isReverse );
} else if ( isHorizontal && getSelection().isCollapsed && isHorizontalEdge( target, isReverseDir ) ) {
const closestTabbable = this.getClosestTabbable( target, isReverseDir );
placeCaretAtHorizontalEdge( closestTabbable, isReverseDir );
event.preventDefault();
}
}
Expand Down
8 changes: 6 additions & 2 deletions packages/dom/src/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,20 +143,24 @@ function isEdge( container, isReverse, onlyVertical ) {
return true;
}

// In the case of RTL scripts, the horizontal edge is at the opposite side.
const { direction } = computedStyle;
const isReverseDir = direction === 'rtl' ? ( ! isReverse ) : isReverse;

// To calculate the horizontal position, we insert a test range and see if
// this test range has the same horizontal position. This method proves to
// be better than a DOM-based calculation, because it ignores empty text
// nodes and a trailing line break element. In other words, we need to check
// visual positioning, not DOM positioning.
const x = isReverse ? containerRect.left + 1 : containerRect.right - 1;
const x = isReverseDir ? containerRect.left + 1 : containerRect.right - 1;
const y = isReverse ? containerRect.top + buffer : containerRect.bottom - buffer;
const testRange = hiddenCaretRangeFromPoint( document, x, y, container );

if ( ! testRange ) {
return false;
}

const side = isReverse ? 'left' : 'right';
const side = isReverseDir ? 'left' : 'right';
const testRect = getRectangleFromRange( testRange );

return Math.round( testRect[ side ] ) === Math.round( rangeRect[ side ] );
Expand Down
6 changes: 6 additions & 0 deletions packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ exports[`RichText should apply multiple formats when selection is collapsed 1`]
<!-- /wp:paragraph -->"
`;

exports[`RichText should handle Home and End keys 1`] = `
"<!-- wp:paragraph -->
<p>-<strong>12</strong>+</p>
<!-- /wp:paragraph -->"
`;

exports[`RichText should handle change in tag name gracefully 1`] = `
"<!-- wp:heading {\\"level\\":3} -->
<h3></h3>
Expand Down
63 changes: 63 additions & 0 deletions packages/e2e-tests/specs/__snapshots__/rtl.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`RTL should arrow navigate 1`] = `
"<!-- wp:paragraph -->
<p>٠١٢</p>
<!-- /wp:paragraph -->"
`;

exports[`RTL should arrow navigate between blocks 1`] = `
"<!-- wp:paragraph -->
<p>٠<br>١</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph -->
<p>٠<br>١<br>٢</p>
<!-- /wp:paragraph -->"
`;
exports[`RTL should merge backward 1`] = `
"<!-- wp:paragraph -->
<p>٠١</p>
<!-- /wp:paragraph -->"
`;
exports[`RTL should merge forward 1`] = `
"<!-- wp:paragraph -->
<p>٠١</p>
<!-- /wp:paragraph -->"
`;
exports[`RTL should navigate inline boundaries 1`] = `
"<!-- wp:paragraph -->
<p><strong>١</strong>٠٢</p>
<!-- /wp:paragraph -->"
`;
exports[`RTL should navigate inline boundaries 2`] = `
"<!-- wp:paragraph -->
<p><strong>١٠</strong>٢</p>
<!-- /wp:paragraph -->"
`;
exports[`RTL should navigate inline boundaries 3`] = `
"<!-- wp:paragraph -->
<p><strong>٠١</strong>٢</p>
<!-- /wp:paragraph -->"
`;
exports[`RTL should navigate inline boundaries 4`] = `
"<!-- wp:paragraph -->
<p>٠<strong>١</strong>٢</p>
<!-- /wp:paragraph -->"
`;
exports[`RTL should split 1`] = `
"<!-- wp:paragraph -->
<p>٠</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph -->
<p>١</p>
<!-- /wp:paragraph -->"
`;
15 changes: 15 additions & 0 deletions packages/e2e-tests/specs/rich-text.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,4 +208,19 @@ describe( 'RichText', () => {

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should handle Home and End keys', async () => {
await page.keyboard.press( 'Enter' );

await pressKeyWithModifier( 'primary', 'b' );
await page.keyboard.type( '12' );
await pressKeyWithModifier( 'primary', 'b' );

await page.keyboard.press( 'Home' );
await page.keyboard.type( '-' );
await page.keyboard.press( 'End' );
await page.keyboard.type( '+' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );
} );
122 changes: 122 additions & 0 deletions packages/e2e-tests/specs/rtl.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/**
* WordPress dependencies
*/
import {
createNewPost,
getEditedPostContent,
pressKeyWithModifier,
} from '@wordpress/e2e-test-utils';

// Avoid using three, as it looks too much like two with some fonts.
const ARABIC_ZERO = '٠';
const ARABIC_ONE = '١';
const ARABIC_TWO = '٢';

describe( 'RTL', () => {
beforeEach( async () => {
await createNewPost();
} );

it( 'should arrow navigate', async () => {
await page.evaluate( () => document.dir = 'rtl' );
await page.keyboard.press( 'Enter' );

// We need at least three characters as arrow navigation *from* the
// edges might be handled differently.
await page.keyboard.type( ARABIC_ONE );
await page.keyboard.type( ARABIC_TWO );
await page.keyboard.press( 'ArrowRight' );
// This is the important key press: arrow nav *from* the middle.
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.type( ARABIC_ZERO );

// Expect: ARABIC_ZERO + ARABIC_ONE + ARABIC_TWO (<p>٠١٢</p>).
// N.b.: HTML is LTR, so direction will be reversed!
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should split', async () => {
await page.evaluate( () => document.dir = 'rtl' );
await page.keyboard.press( 'Enter' );

await page.keyboard.type( ARABIC_ZERO );
await page.keyboard.type( ARABIC_ONE );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'Enter' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should merge backward', async () => {
await page.evaluate( () => document.dir = 'rtl' );
await page.keyboard.press( 'Enter' );

await page.keyboard.type( ARABIC_ZERO );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( ARABIC_ONE );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'Backspace' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should merge forward', async () => {
await page.evaluate( () => document.dir = 'rtl' );
await page.keyboard.press( 'Enter' );

await page.keyboard.type( ARABIC_ZERO );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( ARABIC_ONE );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'Delete' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should arrow navigate between blocks', async () => {
await page.evaluate( () => document.dir = 'rtl' );
await page.keyboard.press( 'Enter' );

await page.keyboard.type( ARABIC_ZERO );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( ARABIC_ONE );
await pressKeyWithModifier( 'shift', 'Enter' );
await page.keyboard.type( ARABIC_TWO );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'ArrowRight' );

// Move to the previous block with two lines in the current block.
await page.keyboard.press( 'ArrowRight' );
await pressKeyWithModifier( 'shift', 'Enter' );
await page.keyboard.type( ARABIC_ONE );

// Move to the next block with two lines in the current block.
await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.type( ARABIC_ZERO );
await pressKeyWithModifier( 'shift', 'Enter' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should navigate inline boundaries', async () => {
await page.evaluate( () => document.dir = 'rtl' );
await page.keyboard.press( 'Enter' );

await pressKeyWithModifier( 'primary', 'b' );
await page.keyboard.type( ARABIC_ONE );
await pressKeyWithModifier( 'primary', 'b' );
await page.keyboard.type( ARABIC_TWO );

// Insert a character at each boundary position.
for ( let i = 4; i > 0; i-- ) {
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.type( ARABIC_ZERO );

expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.press( 'Backspace' );
}
} );
} );

0 comments on commit 5cc5660

Please sign in to comment.