diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 90b10613f86229..541ac81ec7a1fb 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -8,7 +8,6 @@ import { omit, pickBy, } from 'lodash'; -import memize from 'memize'; /** * WordPress dependencies @@ -88,9 +87,9 @@ const globalStyle = document.createElement( 'style' ); document.head.appendChild( globalStyle ); -function createPrepareEditableTree( props ) { +function createPrepareEditableTree( props, prefix ) { const fns = Object.keys( props ).reduce( ( accumulator, key ) => { - if ( key.startsWith( 'format_prepare_functions' ) ) { + if ( key.startsWith( prefix ) ) { accumulator.push( props[ key ] ); } @@ -145,11 +144,6 @@ export class RichText extends Component { this.handleHorizontalNavigation = this.handleHorizontalNavigation.bind( this ); this.onPointerDown = this.onPointerDown.bind( this ); - this.formatToValue = memize( - this.formatToValue.bind( this ), - { maxSize: 1 } - ); - this.patterns = getPatterns( { onReplace, valueToFormat: this.valueToFormat, @@ -162,10 +156,11 @@ export class RichText extends Component { this.usedDeprecatedChildrenSource = Array.isArray( value ); this.lastHistoryValue = value; - // Internal values that are update synchronously, unlike props. + // Internal values are updated synchronously, unlike props and state. this.value = value; - this.selectionStart = selectionStart; - this.selectionEnd = selectionEnd; + this.record = this.formatToValue( value ); + this.record.start = selectionStart; + this.record.end = selectionEnd; } componentWillUnmount() { @@ -195,11 +190,7 @@ export class RichText extends Component { * @return {Object} The current record (value and selection). */ getRecord() { - const { value, selectionStart: start, selectionEnd: end } = this.props; - const { formats, replacements, text } = this.formatToValue( value ); - const { activeFormats } = this.state; - - return { formats, replacements, text, start, end, activeFormats }; + return this.record; } createRecord() { @@ -211,7 +202,6 @@ export class RichText extends Component { range, multilineTag: this.multilineTag, multilineWrapperTags: this.multilineWrapperTags, - prepareEditableTree: createPrepareEditableTree( this.props ), __unstableIsEditableTree: true, } ); } @@ -222,13 +212,13 @@ export class RichText extends Component { current: this.editableRef, multilineTag: this.multilineTag, multilineWrapperTags: this.multilineWrapperTags, - prepareEditableTree: createPrepareEditableTree( this.props ), + prepareEditableTree: createPrepareEditableTree( this.props, 'format_prepare_functions' ), __unstableDomOnly: domOnly, } ); } isEmpty() { - return isEmpty( this.formatToValue( this.props.value ) ); + return isEmpty( this.record ); } /** @@ -378,7 +368,20 @@ export class RichText extends Component { } this.recalculateBoundaryStyle(); - this.onSelectionChange(); + + // We know for certain that on focus, the old selection is invalid. It + // will be recalculated on `selectionchange`. + const index = undefined; + const activeFormats = undefined; + + this.record = { + ...this.record, + start: index, + end: index, + activeFormats, + }; + this.props.onSelectionChange( index, index ); + this.setState( { activeFormats } ); document.addEventListener( 'selectionchange', this.onSelectionChange ); } @@ -419,8 +422,7 @@ export class RichText extends Component { } const value = this.createRecord(); - const { activeFormats = [] } = this.state; - const start = this.selectionStart; + const { start, activeFormats = [] } = this.record; // Update the formats between the last and new caret position. const change = updateFormats( { @@ -459,12 +461,23 @@ export class RichText extends Component { * Handles the `selectionchange` event: sync the selection to local state. */ onSelectionChange() { - const value = this.createRecord(); - const { start, end } = value; + const { start, end } = this.createRecord(); + const value = this.getRecord(); - if ( start !== this.selectionStart || end !== this.selectionEnd ) { + if ( start !== value.start || end !== value.end ) { const { isCaretWithinFormattedText } = this.props; - const activeFormats = getActiveFormats( value ); + const newValue = { + ...value, + start, + end, + // Allow `getActiveFormats` to get new `activeFormats`. + activeFormats: undefined, + }; + + const activeFormats = getActiveFormats( newValue ); + + // Update the value with the new active formats. + newValue.activeFormats = activeFormats; if ( ! isCaretWithinFormattedText && activeFormats.length ) { this.props.onEnterFormattedText(); @@ -472,11 +485,12 @@ export class RichText extends Component { this.props.onExitFormattedText(); } - this.setState( { activeFormats } ); - this.applyRecord( { ...value, activeFormats }, { domOnly: true } ); + // It is important that the internal value is updated first, + // otherwise the value will be wrong on render! + this.record = newValue; + this.applyRecord( newValue, { domOnly: true } ); this.props.onSelectionChange( start, end ); - this.selectionStart = start; - this.selectionEnd = end; + this.setState( { activeFormats } ); if ( activeFormats.length > 0 ) { this.recalculateBoundaryStyle(); @@ -521,11 +535,10 @@ export class RichText extends Component { } ); this.value = this.valueToFormat( record ); + this.record = record; this.props.onChange( this.value ); - this.setState( { activeFormats } ); this.props.onSelectionChange( start, end ); - this.selectionStart = start; - this.selectionEnd = end; + this.setState( { activeFormats } ); if ( ! withoutHistory ) { this.onCreateUndoLevel(); @@ -739,11 +752,13 @@ export class RichText extends Component { * @param {SyntheticEvent} event A synthetic keyboard event. */ handleHorizontalNavigation( event ) { - const value = this.createRecord(); - const { formats, text, start, end } = value; - const { activeFormats = [] } = this.state; + const value = this.getRecord(); + const { text, formats, start, end, activeFormats = [] } = value; 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. @@ -804,24 +819,26 @@ export class RichText extends Component { if ( newActiveFormatsLength !== activeFormats.length ) { const newActiveFormats = source.slice( 0, newActiveFormatsLength ); - this.applyRecord( { ...value, activeFormats: newActiveFormats } ); + const newValue = { ...value, activeFormats: newActiveFormats }; + this.record = newValue; + this.applyRecord( newValue ); this.setState( { activeFormats: newActiveFormats } ); return; } const newPos = value.start + ( isReverse ? -1 : 1 ); - const newActiveFormats = isReverse ? formatsBefore.length : formatsAfter.length; - - this.setState( { selectedFormat: newActiveFormats } ); - this.props.onSelectionChange( newPos, newPos ); - this.selectionStart = newPos; - this.selectionEnd = newPos; - this.applyRecord( { + const newActiveFormats = isReverse ? formatsBefore : formatsAfter; + const newValue = { ...value, start: newPos, end: newPos, activeFormats: newActiveFormats, - } ); + }; + + this.record = newValue; + this.applyRecord( newValue ); + this.props.onSelectionChange( newPos, newPos ); + this.setState( { activeFormats: newActiveFormats } ); } /** @@ -898,8 +915,7 @@ export class RichText extends Component { } componentDidUpdate( prevProps ) { - const { tagName, value, isSelected } = this.props; - const record = this.getRecord(); + const { tagName, value, selectionStart, selectionEnd, isSelected } = this.props; // Check if the content changed. let shouldReapply = ( @@ -911,8 +927,8 @@ export class RichText extends Component { // Check if the selection changed. shouldReapply = shouldReapply || ( isSelected && ! prevProps.isSelected && ( - this.selectionStart !== record.start || - this.selectionEnd !== record.end + this.record.start !== selectionStart || + this.record.end !== selectionEnd ) ); @@ -925,18 +941,32 @@ export class RichText extends Component { shouldReapply = shouldReapply || ! isShallowEqual( prepareProps, prevPrepareProps ); + const { activeFormats = [] } = this.record; + if ( shouldReapply ) { - if ( ! isSelected ) { - delete record.start; - delete record.end; - } + this.value = value; + this.record = this.formatToValue( value ); + this.record.start = selectionStart; + this.record.end = selectionEnd; + + updateFormats( { + value: this.record, + start: this.record.start, + end: this.record.end, + formats: activeFormats, + } ); - this.applyRecord( record ); + this.applyRecord( this.record ); + } else if ( + this.record.start !== selectionStart || + this.record.end !== selectionEnd + ) { + this.record = { + ...this.record, + start: selectionStart, + end: selectionEnd, + }; } - - this.value = value; - this.selectionStart = record.start; - this.selectionEnd = record.end; } /** @@ -948,19 +978,20 @@ export class RichText extends Component { formatToValue( value ) { // Handle deprecated `children` and `node` sources. if ( Array.isArray( value ) ) { - return create( { - html: children.toHTML( value ), - multilineTag: this.multilineTag, - multilineWrapperTags: this.multilineWrapperTags, - } ); + value = children.toHTML( value ); } if ( this.props.format === 'string' ) { - return create( { + const prepare = createPrepareEditableTree( this.props, 'format_value_functions' ); + + value = create( { html: value, multilineTag: this.multilineTag, multilineWrapperTags: this.multilineWrapperTags, } ); + value.formats = prepare( value ); + + return value; } // Guard for blocks passing `null` in onSplit callbacks. May be removed @@ -976,7 +1007,7 @@ export class RichText extends Component { return toDom( { value, multilineTag: this.multilineTag, - prepareEditableTree: createPrepareEditableTree( this.props ), + prepareEditableTree: createPrepareEditableTree( this.props, 'format_prepare_functions' ), } ).body.innerHTML; } diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index 1acb85b1695584..656ac5b4ed49e7 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(); } + // 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 b676448ec4424f..64fb97534f60a2 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' ); + } + } ); +} ); diff --git a/packages/rich-text/src/register-format-type.js b/packages/rich-text/src/register-format-type.js index eb6d48c9e56368..e6efc99f5fec97 100644 --- a/packages/rich-text/src/register-format-type.js +++ b/packages/rich-text/src/register-format-type.js @@ -146,18 +146,23 @@ export function registerFormatType( name, settings ) { blockClientId: props.clientId, }; - newProps[ `format_prepare_functions_(${ name })` ] = - settings.__experimentalCreatePrepareEditableTree( - propsByPrefix, - args - ); - if ( settings.__experimentalCreateOnChangeEditableValue ) { + newProps[ `format_value_functions_(${ name })` ] = + settings.__experimentalCreatePrepareEditableTree( + propsByPrefix, + args + ); newProps[ `format_on_change_functions_(${ name })` ] = settings.__experimentalCreateOnChangeEditableValue( propsByPrefix, args ); + } else { + newProps[ `format_prepare_functions_(${ name })` ] = + settings.__experimentalCreatePrepareEditableTree( + propsByPrefix, + args + ); } return ;