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
ellatrix authored May 14, 2019
1 parent 5963776 commit c64faf4
Show file tree
Hide file tree
Showing 8 changed files with 329 additions and 78 deletions.
163 changes: 97 additions & 66 deletions packages/block-editor/src/components/rich-text/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
omit,
pickBy,
} from 'lodash';
import memize from 'memize';

/**
* WordPress dependencies
Expand Down Expand Up @@ -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 ] );
}

Expand Down Expand Up @@ -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,
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand All @@ -211,7 +202,6 @@ export class RichText extends Component {
range,
multilineTag: this.multilineTag,
multilineWrapperTags: this.multilineWrapperTags,
prepareEditableTree: createPrepareEditableTree( this.props ),
__unstableIsEditableTree: true,
} );
}
Expand All @@ -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 );
}

/**
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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( {
Expand Down Expand Up @@ -459,24 +461,36 @@ 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();
} else if ( isCaretWithinFormattedText && ! activeFormats.length ) {
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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 } );
}

/**
Expand Down Expand Up @@ -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 = (
Expand All @@ -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
)
);

Expand All @@ -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;
}

/**
Expand All @@ -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
Expand All @@ -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;
}

Expand Down
13 changes: 9 additions & 4 deletions packages/block-editor/src/components/writing-flow/index.js
View file
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();
}

// 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
View file
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
Loading

0 comments on commit c64faf4

Please sign in to comment.