From 5cc56602494f22260b2d0ca112111549e5be7edb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Tue, 14 May 2019 13:13:31 +0200 Subject: [PATCH] RichText: fix RTL keyboard interactions (#15496) --- .../src/components/rich-text/index.js | 6 +- .../src/components/writing-flow/index.js | 13 +- packages/dom/src/dom.js | 8 +- .../__snapshots__/rich-text.test.js.snap | 6 + .../specs/__snapshots__/rtl.test.js.snap | 63 +++++++++ packages/e2e-tests/specs/rich-text.test.js | 15 +++ packages/e2e-tests/specs/rtl.test.js | 122 ++++++++++++++++++ 7 files changed, 225 insertions(+), 8 deletions(-) create mode 100644 packages/e2e-tests/specs/__snapshots__/rtl.test.js.snap create mode 100644 packages/e2e-tests/specs/rtl.test.js diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index c6cfad310c89c6..9ab16675db0e9f 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -202,7 +202,6 @@ export class RichText extends Component { range, multilineTag: this.multilineTag, multilineWrapperTags: this.multilineWrapperTags, - prepareEditableTree: this.props.prepareEditableTree, __unstableIsEditableTree: true, } ); } @@ -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. diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index d03b11d0a84f7a..c98f9bc682d51c 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -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 @@ -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 ( ( @@ -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(); } } diff --git a/packages/dom/src/dom.js b/packages/dom/src/dom.js index aba561f992d593..c2491a0c07be1c 100644 --- a/packages/dom/src/dom.js +++ b/packages/dom/src/dom.js @@ -143,12 +143,16 @@ 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 ); @@ -156,7 +160,7 @@ function isEdge( container, isReverse, onlyVertical ) { 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 ] ); diff --git a/packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap b/packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap index bf5fefd31a7e32..8aab0dcd5b734b 100644 --- a/packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap +++ b/packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap @@ -24,6 +24,12 @@ exports[`RichText should apply multiple formats when selection is collapsed 1`] " `; +exports[`RichText should handle Home and End keys 1`] = ` +" +

-12+

+" +`; + exports[`RichText should handle change in tag name gracefully 1`] = ` "

diff --git a/packages/e2e-tests/specs/__snapshots__/rtl.test.js.snap b/packages/e2e-tests/specs/__snapshots__/rtl.test.js.snap new file mode 100644 index 00000000000000..19b4e8c305acc2 --- /dev/null +++ b/packages/e2e-tests/specs/__snapshots__/rtl.test.js.snap @@ -0,0 +1,63 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`RTL should arrow navigate 1`] = ` +" +

٠١٢

+" +`; + +exports[`RTL should arrow navigate between blocks 1`] = ` +" +

٠
١

+ + + +

٠
١
٢

+" +`; + +exports[`RTL should merge backward 1`] = ` +" +

٠١

+" +`; + +exports[`RTL should merge forward 1`] = ` +" +

٠١

+" +`; + +exports[`RTL should navigate inline boundaries 1`] = ` +" +

١٠٢

+" +`; + +exports[`RTL should navigate inline boundaries 2`] = ` +" +

١٠٢

+" +`; + +exports[`RTL should navigate inline boundaries 3`] = ` +" +

٠١٢

+" +`; + +exports[`RTL should navigate inline boundaries 4`] = ` +" +

٠١٢

+" +`; + +exports[`RTL should split 1`] = ` +" +

٠

+ + + +

١

+" +`; diff --git a/packages/e2e-tests/specs/rich-text.test.js b/packages/e2e-tests/specs/rich-text.test.js index dcb1434abd89be..1bd6d79c1b10fc 100644 --- a/packages/e2e-tests/specs/rich-text.test.js +++ b/packages/e2e-tests/specs/rich-text.test.js @@ -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(); + } ); } ); diff --git a/packages/e2e-tests/specs/rtl.test.js b/packages/e2e-tests/specs/rtl.test.js new file mode 100644 index 00000000000000..76e6d15ff9aa30 --- /dev/null +++ b/packages/e2e-tests/specs/rtl.test.js @@ -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 (

٠١٢

). + // 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' ); + } + } ); +} );