Skip to content

Commit

Permalink
Mobile - Fix iOS Focus loop for RichText components (#53217)
Browse files Browse the repository at this point in the history 
* Mobile - Update changelog

* fix: Avoid iOS block appender focus loop

The focus callback triggered by Aztec-based programmatic focus events
can result in focus loops between rich text elements.

Android: This intentional no-op function prevents focus loops
originating when the native Aztec module programmatically focuses the
instance. The no-op is explicitly passed as an `onFocus` prop to avoid
future prop spreading from inadvertently introducing focus loops. The
user-facing focus of the element is handled by `onPress` instead.

See: wordpress-mobile/gutenberg-mobile#302

iOS: Programmatic focus from the native Aztec module is required to
ensure the React-based `TextStateInput` ref is properly set when focus
is *returned* to an instance, e.g. dismissing a bottom sheet. If the ref
is not updated, attempts to dismiss the keyboard via the `ToolbarButton`
will fail.

See: wordpress-mobile/gutenberg-mobile#702

The Android keyboard is, likely erroneously, already dismissed in the
contexts where programmatic focus may be required on iOS.

- #28748
- #29048
- wordpress-mobile/WordPress-Android#16167

Programmatic swapping focus from element to another often leads to focus
loops, only delegate the programmatic focus if there are no elements
focused.

See: wordpress-mobile/WordPress-iOS#18783

* fix: Programmatic Aztec input focus only updates internal ref

Programmatically swapping input focus creates an infinite loop if the
user taps a different input in between the programmatic focus and
the resulting update to the React Native TextInputState focused element
ref. To mitigate this, the Aztec now updates the focused element ref,
but does not call the native focus methods.

See: wordpress-mobile/WordPress-iOS#18783

* Mobile - AztecView - Check for isFocused before forcing the focus

* Mobile - DefaultBlockAppender and BlockList Footer placeholders - Removes inline functions and other minor code style changes

* Mobile - AztecView - Trigger _onFocus within _onAztecFocus to prevent having a RichText component focused while another block is selected

---------

Co-authored-by: David Calhoun <438664+dcalhoun@users.noreply.github.com>
  • Loading branch information
@dcalhoun
Gerardo Pacheco and dcalhoun authored Aug 3, 2023
1 parent b2e5f0d commit 9020d4b
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 34 deletions.
28 changes: 14 additions & 14 deletions packages/block-editor/src/components/block-list/index.native.js
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/**
* External dependencies
*/
import { View, Platform, TouchableWithoutFeedback } from 'react-native';
import { View, Platform, Pressable } from 'react-native';

/**
* WordPress dependencies
*/
import { useRef, useState } from '@wordpress/element';
import { useRef, useState, useCallback } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';
import { createBlock } from '@wordpress/blocks';
import {
Expand Down Expand Up @@ -372,20 +372,20 @@ function Footer( {
renderFooterAppender,
withFooter,
} ) {
const onAddParagraphBlock = useCallback( () => {
const paragraphBlock = createBlock( 'core/paragraph' );
addBlockToEndOfPost( paragraphBlock );
}, [ addBlockToEndOfPost ] );

if ( ! isReadOnly && withFooter ) {
return (
<>
<TouchableWithoutFeedback
accessibilityLabel={ __( 'Add paragraph block' ) }
testID={ __( 'Add paragraph block' ) }
onPress={ () => {
const paragraphBlock = createBlock( 'core/paragraph' );
addBlockToEndOfPost( paragraphBlock );
} }
>
<View style={ styles.blockListFooter } />
</TouchableWithoutFeedback>
</>
<Pressable
accessibilityLabel={ __( 'Add paragraph block' ) }
testID={ __( 'Add paragraph block' ) }
onPress={ onAddParagraphBlock }
>
<View style={ styles.blockListFooter } />
</Pressable>
);
} else if ( renderFooterAppender ) {
return <View>{ renderFooterAppender() }</View>;
Expand Down
24 changes: 13 additions & 11 deletions packages/block-editor/src/components/default-block-appender/index.native.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { TouchableWithoutFeedback, View } from 'react-native';
import { Pressable, View } from 'react-native';

/**
* WordPress dependencies
Expand All @@ -20,6 +20,9 @@ import BlockInsertionPoint from '../block-list/insertion-point';
import styles from './style.scss';
import { store as blockEditorStore } from '../../store';

const hitSlop = { top: 22, bottom: 22, left: 22, right: 22 };
const noop = () => {};

export function DefaultBlockAppender( {
isLocked,
isVisible,
Expand All @@ -37,22 +40,21 @@ export function DefaultBlockAppender( {
? decodeEntities( placeholder )
: __( 'Start writing…' );

const appenderStyles = [
styles.blockHolder,
showSeparator && containerStyle,
];

return (
<TouchableWithoutFeedback onPress={ onAppend }>
<View
style={ [
styles.blockHolder,
showSeparator && containerStyle,
] }
pointerEvents="box-only"
>
<Pressable onPress={ onAppend } hitSlop={ hitSlop }>
<View style={ appenderStyles } pointerEvents="box-only">
{ showSeparator ? (
<BlockInsertionPoint />
) : (
<RichText placeholder={ value } onChange={ () => {} } />
<RichText placeholder={ value } onChange={ noop } />
) }
</View>
</TouchableWithoutFeedback>
</Pressable>
);
}

Expand Down
9 changes: 9 additions & 0 deletions packages/react-native-aztec/src/AztecInputState.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@ export const notifyInputChange = () => {
}
};

/**
* Sets the current focused element ref held within TextInputState.
*
* @param {RefObject} element Element to be set as the focused element.
*/
export const focusInput = ( element ) => {
TextInputState.focusInput( element );
};

/**
* Focuses the specified element.
*
Expand Down
51 changes: 42 additions & 9 deletions packages/react-native-aztec/src/AztecView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,49 @@ class AztecView extends Component {
}

_onAztecFocus( event ) {
// IMPORTANT: the onFocus events from Aztec are thrown away on Android as these are handled by onPress() in the upper level.
// It's necessary to do this otherwise onFocus may be set by `{...otherProps}` and thus the onPress + onFocus
// combination generate an infinite loop as described in https://github.com/wordpress-mobile/gutenberg-mobile/issues/302
// For iOS, this is necessary to let the system know when Aztec was focused programatically.
if ( Platform.OS === 'ios' ) {
// IMPORTANT: This function serves two purposes:
//
// Android: This intentional no-op function prevents focus loops originating
// when the native Aztec module programmatically focuses the instance. The
// no-op is explicitly passed as an `onFocus` prop to avoid future prop
// spreading from inadvertently introducing focus loops. The user-facing
// focus of the element is handled by `onPress` instead.
//
// See: https://github.com/wordpress-mobile/gutenberg-mobile/issues/302
//
// iOS: Programmatic focus from the native Aztec module is required to
// ensure the React-based `TextStateInput` ref is properly set when focus
// is *returned* to an instance, e.g. dismissing a bottom sheet. If the ref
// is not updated, attempts to dismiss the keyboard via the `ToolbarButton`
// will fail.
//
// See: https://github.com/wordpress-mobile/gutenberg-mobile/issues/702
if (
// The Android keyboard is, likely erroneously, already dismissed in the
// contexts where programmatic focus may be required on iOS.
//
// - https://github.com/WordPress/gutenberg/issues/28748
// - https://github.com/WordPress/gutenberg/issues/29048
// - https://github.com/wordpress-mobile/WordPress-Android/issues/16167
Platform.OS === 'ios'
) {
this.updateCaretData( event );

this._onPress( event );
if ( ! this.isFocused() ) {
// Programmatically swapping input focus creates an infinite loop if the
// user taps a different input in between the programmatic focus and
// the resulting update to the React Native TextInputState focused element
// ref. To mitigate this, the below updates the focused element ref, but
// does not call the native focus methods.
//
// See: https://github.com/wordpress-mobile/WordPress-iOS/issues/18783
AztecInputState.focusInput( this.aztecViewRef.current );

// Calling _onFocus is needed to trigger provided onFocus callbacks
// which are needed to prevent undesired results like having a focused
// TextInput when another element has the focus.
this._onFocus( event );
}
}
}

Expand Down Expand Up @@ -285,9 +320,7 @@ class AztecView extends Component {
onBackspace={ this.props.onKeyDown && this._onBackspace }
onKeyDown={ this.props.onKeyDown && this._onKeyDown }
deleteEnter={ this.props.deleteEnter }
// IMPORTANT: the onFocus events are thrown away as these are handled by onPress() in the upper level.
// It's necessary to do this otherwise onFocus may be set by `{...otherProps}` and thus the onPress + onFocus
// combination generate an infinite loop as described in https://github.com/wordpress-mobile/gutenberg-mobile/issues/302
// IMPORTANT: Do not remove the `onFocus` prop, see `_onAztecFocus`
onFocus={ this._onAztecFocus }
onBlur={ this._onBlur }
ref={ this.aztecViewRef }
Expand Down
1 change: 1 addition & 0 deletions packages/react-native-editor/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ For each user feature we should also add a importance categorization label to i
- [**] Upgrade React Native to 0.71.11 [#51303]
- [*] Upgrade Gradle to 8.2.1 & AGP to 8.1.0 [#52872]
- [*] Fix Gallery block selection when adding media [#53127]
- [**] Fix iOS Focus loop for RichText components [#53217]

## 1.100.1
- [**] Add WP hook for registering non-core blocks [#52791]
Expand Down

0 comments on commit 9020d4b

Please sign in to comment.