Skip to content

Commit

Permalink
[RNMobile] keyboard focus issues (#15999)
Browse files Browse the repository at this point in the history
* Focus RichText on mount if block is selected and says so

Some blocks have multiple RichText or a RichText among other children.
Example: Quote blocks has 2 RichTexts and Image block has a RichText for
the caption. We want to control when and which of those RichTexts will
request focus. On the web side, the DOM is used to search for a inputbox
to focus but on RN, we don't have a DOM to search.

Instead, this commit makes the assumption that a RichText always request
focus if its parent has passed `true` in `isSelected` and only if the
parent hasn't inhibited that behavior by using the `noFocusOnMount`
prop.

* Simplify the passing of noFocusOnMount

* Focus and selection logic closer to GB web's

* RichText's API uses unstableOnFocus at the moment

* Don't force selection update if position is unset

That can happen when the RichText based block gets tapped somewhere and
the caret position is not updated yet from Aztec.
  • Loading branch information
hypest authored Jun 7, 2019
1 parent cbac39d commit a57202e
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 20 deletions.
28 changes: 16 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 @@ -531,10 +531,18 @@ export class RichText extends Component {
onFocus() {
this.isTouched = true;

if ( this.props.onFocus ) {
this.props.onFocus();
const { unstableOnFocus } = this.props;

if ( unstableOnFocus ) {
unstableOnFocus();
}

// We know for certain that on focus, the old selection is invalid. It
// will be recalculated on `selectionchange`.
const index = undefined;

this.props.onSelectionChange( index, index );

this.lastAztecEventType = 'focus';
}

Expand Down Expand Up @@ -657,7 +665,9 @@ export class RichText extends Component {
}

if ( ! this.comesFromAztec ) {
if ( nextProps.selectionStart !== this.props.selectionStart &&
if ( ( typeof nextProps.selectionStart !== 'undefined' ) &&
( typeof nextProps.selectionEnd !== 'undefined' ) &&
nextProps.selectionStart !== this.props.selectionStart &&
nextProps.selectionStart !== this.selectionStart &&
nextProps.isSelected ) {
this.needsSelectionUpdate = true;
Expand All @@ -681,7 +691,7 @@ export class RichText extends Component {

componentWillUnmount() {
if ( this._editor.isFocused() ) {
this._editor.blur();
// this._editor.blur();
}
}

Expand All @@ -695,7 +705,7 @@ export class RichText extends Component {
// Update selection props explicitly when component is selected as Aztec won't call onSelectionChange
// if its internal value hasn't change. When created, default value is 0, 0
this.onSelectionChange( this.props.selectionStart || 0, this.props.selectionEnd || 0 );
} else if ( ! this.props.isSelected && prevProps.isSelected && this.isIOS ) {
} else if ( ! this.props.isSelected && prevProps.isSelected ) {
this._editor.blur();
}
}
Expand Down Expand Up @@ -810,7 +820,6 @@ export class RichText extends Component {
onContentSizeChange={ this.onContentSizeChange }
onCaretVerticalPositionChange={ this.props.onCaretVerticalPositionChange }
onSelectionChange={ this.onSelectionChangeFromAztec }
isSelected={ isSelected }
blockType={ { tag: tagName } }
color={ 'black' }
maxImagesWidth={ 200 }
Expand All @@ -836,15 +845,10 @@ RichText.defaultProps = {

const RichTextContainer = compose( [
withInstanceId,
withBlockEditContext( ( { clientId, onFocus, onCaretVerticalPositionChange, isSelected }, ownProps ) => {
// ownProps.onFocus needs precedence over the block edit context
if ( ownProps.onFocus !== undefined ) {
onFocus = ownProps.onFocus;
}
withBlockEditContext( ( { clientId, onCaretVerticalPositionChange, isSelected }, ownProps ) => {
return {
clientId,
blockIsSelected: ownProps.isSelected !== undefined ? ownProps.isSelected : isSelected,
onFocus,
onCaretVerticalPositionChange,
};
} ),
Expand Down
3 changes: 0 additions & 3 deletions packages/block-library/src/heading/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,10 @@ const HeadingEdit = ( {
identifier="content"
tagName={ 'h' + attributes.level }
value={ attributes.content }
isSelected={ isSelected }
style={ {
...style,
minHeight: styles[ 'wp-block-heading' ].minHeight,
} }
onFocus={ onFocus } // always assign onFocus as a props
onBlur={ onBlur } // always assign onBlur as a props
onChange={ ( value ) => setAttributes( { content: value } ) }
onMerge={ mergeBlocks }
onSplit={ ( value ) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/image/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ class ImageEdit extends React.Component {
placeholder={ __( 'Write caption…' ) }
value={ caption }
onChange={ ( newCaption ) => setAttributes( { caption: newCaption } ) }
onFocus={ this.onFocusCaption }
unstableOnFocus={ this.onFocusCaption }
onBlur={ this.props.onBlur } // always assign onBlur as props
isSelected={ this.state.isCaptionSelected }
__unstableMobileNoFocusOnMount
Expand Down
3 changes: 0 additions & 3 deletions packages/block-library/src/paragraph/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ class ParagraphEdit extends Component {
identifier="content"
tagName="p"
value={ content }
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 ) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/components/post-title/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class PostTitle extends Component {
<RichText
tagName={ 'p' }
rootTagsToEliminate={ [ 'strong' ] }
onFocus={ this.onSelect }
unstableOnFocus={ this.onSelect }
onBlur={ this.props.onBlur } // always assign onBlur as a props
multiline={ false }
style={ style }
Expand Down

0 comments on commit a57202e

Please sign in to comment.