Skip to content

Commit

Permalink
[Rnmobile] Fix the list handling on Android (#15168)
Browse files Browse the repository at this point in the history
* If text already changed, don't modify it

* Able to not lose content

* Use a flag to signal Aztec-originated changes

And assume that when that flag is false, component changes need to get
sent/reflected down to Aztec.

* Differentiate Android and iOS since assumptions diverged

The iOS side still expects to just check against `this.lastContent` to
force the change into Aztec.

* Force Aztec update if "Enter" fired before text change

* Need to specify firedAfterTextChanged on all Aztec events

* Fix lint issues

* chore: Fix: Lint error that makes unit tests (and CI tests) fail. (#15073)

* Trivial change to trigger Travis

* Revert "Trivial change to trigger Travis"

This reverts commit e22ffde.

* Just use onFormatChange which now defaults to "force"

* Have Aztec delete the detected Enter key for paragraphs

Aztec-Android doesn't swallow the Enter key (like the list handling does) so,
instruct Aztec to delete it for the paragraph block.

* Don't force Aztec update on format button toggles

Doing this by reverting onFormatChange's behavior back to assuming
doUpdateChild is false by default.
  • Loading branch information
hypest authored May 2, 2019
1 parent d88977b commit 9a99071
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 12 deletions.
48 changes: 36 additions & 12 deletions packages/block-editor/src/components/rich-text/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ export class RichText extends Component {
// value. This also provides an opportunity for the parent component to
// determine whether the before/after value has changed using a trivial
// strict equality operation.
if ( isEmpty( after ) ) {
if ( isEmpty( after ) && before.text.length === currentRecord.text.length ) {
before = currentRecord;
} else if ( isEmpty( before ) ) {
} else if ( isEmpty( before ) && after.text.length === currentRecord.text.length ) {
after = currentRecord;
}

Expand Down Expand Up @@ -230,7 +230,7 @@ export class RichText extends Component {
} );
if ( newContent && newContent !== this.props.value ) {
this.props.onChange( newContent );
if ( record.needsSelectionUpdate && record.start && record.end ) {
if ( record.needsSelectionUpdate && record.start && record.end && doUpdateChild ) {
this.forceSelectionUpdate( record.start, record.end );
}
} else {
Expand Down Expand Up @@ -274,6 +274,8 @@ export class RichText extends Component {
this.lastEventCount = event.nativeEvent.eventCount;
const contentWithoutRootTag = this.removeRootTagsProduceByAztec( unescapeSpaces( event.nativeEvent.text ) );
this.lastContent = contentWithoutRootTag;
this.comesFromAztec = true;
this.firedAfterTextChanged = true; // the onChange event always fires after the fact
this.props.onChange( this.lastContent );
}

Expand All @@ -289,6 +291,8 @@ export class RichText extends Component {
// eslint-disable-next-line no-unused-vars
onEnter( event ) {
this.lastEventCount = event.nativeEvent.eventCount;
this.comesFromAztec = true;
this.firedAfterTextChanged = event.nativeEvent.firedAfterTextChanged;

const currentRecord = this.createRecord( {
...event.nativeEvent,
Expand All @@ -303,10 +307,10 @@ export class RichText extends Component {
this.setState( {
needsSelectionUpdate: false,
} );
this.onSplit( ...split( currentRecord ).map( this.valueToFormat ) );
this.splitContent( currentRecord );
} else {
const insertedLineSeparator = { needsSelectionUpdate: true, ...insertLineSeparator( currentRecord ) };
this.onFormatChangeForceChild( insertedLineSeparator );
this.onFormatChange( insertedLineSeparator, ! this.firedAfterTextChanged );
}
} else if ( event.shiftKey || ! this.onSplit ) {
const insertedLineBreak = { needsSelectionUpdate: true, ...insert( currentRecord, '\n' ) };
Expand Down Expand Up @@ -393,7 +397,6 @@ export class RichText extends Component {
},
} );
this.lastContent = this.valueToFormat( linkedRecord );
this.lastEventCount = undefined;
this.props.onChange( this.lastContent );

// Allows us to ask for this information when we get a report.
Expand Down Expand Up @@ -425,7 +428,6 @@ export class RichText extends Component {
const recordToInsert = create( { html: pastedContent } );
const insertedContent = insert( currentRecord, recordToInsert );
const newContent = this.valueToFormat( insertedContent );
this.lastEventCount = undefined;
this.lastContent = newContent;

// explicitly set selection after inline paste
Expand Down Expand Up @@ -488,6 +490,8 @@ export class RichText extends Component {
// we don't want to refresh aztec native as no content can have changed from this event
// let's update lastContent to prevent that in shouldComponentUpdate
this.lastContent = newContent;
this.comesFromAztec = true;
this.firedAfterTextChanged = true; // Selection change event always fires after the fact
this.props.onChange( this.lastContent );
}
}
Expand Down Expand Up @@ -573,7 +577,7 @@ export class RichText extends Component {

// This logic will handle the selection when two blocks are merged or when block is split
// into two blocks
if ( nextTextContent.startsWith( this.savedContent ) ) {
if ( nextTextContent.startsWith( this.savedContent ) && this.savedContent && this.savedContent.length > 0 ) {
let length = this.savedContent.length;
if ( length === 0 && nextTextContent !== this.props.value ) {
length = this.props.value.length;
Expand All @@ -592,7 +596,7 @@ export class RichText extends Component {
}

shouldComponentUpdate( nextProps ) {
if ( nextProps.tagName !== this.props.tagName || nextProps.isSelected !== this.props.isSelected ) {
if ( nextProps.tagName !== this.props.tagName ) {
this.lastEventCount = undefined;
this.lastContent = undefined;
return true;
Expand All @@ -602,13 +606,27 @@ export class RichText extends Component {
// It was removed in https://github.com/WordPress/gutenberg/pull/12417 to fix undo/redo problem.

// If the component is changed React side (undo/redo/merging/splitting/custom text actions)
// we need to make sure the native is updated as well
// we need to make sure the native is updated as well.

const previousValueToCheck = Platform.OS === 'android' ? this.props.value : this.lastContent;

// Also, don't trust the "this.lastContent" as on Android, incomplete text events arrive
// with only some of the text, while the virtual keyboard's suggestion system does its magic.
// ** compare with this.lastContent for optimizing performance by not forcing Aztec with text it already has
// , but compare with props.value to not lose "half word" text because of Android virtual keyb autosuggestion behavior
if ( ( typeof nextProps.value !== 'undefined' ) &&
( typeof this.lastContent !== 'undefined' ) &&
nextProps.value !== this.lastContent ) {
( typeof this.props.value !== 'undefined' ) &&
( Platform.OS === 'ios' || ( Platform.OS === 'android' && ( ! this.comesFromAztec || ! this.firedAfterTextChanged ) ) ) &&
nextProps.value !== previousValueToCheck ) {
this.lastEventCount = undefined; // force a refresh on the native side
}

if ( Platform.OS === 'android' && this.comesFromAztec === false ) {
if ( this.needsSelectionUpdate ) {
this.lastEventCount = undefined; // force a refresh on the native side
}
}

return true;
}

Expand Down Expand Up @@ -683,6 +701,11 @@ export class RichText extends Component {
}
}

if ( this.comesFromAztec ) {
this.comesFromAztec = false;
this.firedAfterTextChanged = false;
}

return (
<View>
{ isSelected && this.multilineTag === 'li' && (
Expand Down Expand Up @@ -713,6 +736,7 @@ export class RichText extends Component {
text={ { text: html, eventCount: this.lastEventCount, selection } }
placeholder={ this.props.placeholder }
placeholderTextColor={ this.props.placeholderTextColor || styles[ 'block-editor-rich-text' ].textDecorationColor }
deleteEnter={ this.props.deleteEnter }
onChange={ this.onChange }
onFocus={ this.onFocus }
onBlur={ this.onBlur }
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/paragraph/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ class ParagraphEdit extends Component {
isSelected={ this.props.isSelected }
onFocus={ this.props.onFocus } // always assign onFocus as a props
onBlur={ this.props.onBlur } // always assign onBlur as a props
deleteEnter={ true }
style={ style }
onChange={ ( nextContent ) => {
setAttributes( {
Expand Down

0 comments on commit 9a99071

Please sign in to comment.