Skip to content

Commit

Permalink
fix: Avoid iOS block appender focus loop
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dcalhoun authored and Gerardo committed Aug 2, 2023
1 parent 6abc32d commit 5be930a
Showing 1 changed file with 34 additions and 10 deletions.
44 changes: 34 additions & 10 deletions packages/react-native-aztec/src/AztecView.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,40 @@ 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' &&
// Programmatic swaping focus from element to another often leads to focus
// loops, only delegate the programmatic focus if there are no elements
// focused.
//
// See: https://github.com/wordpress-mobile/WordPress-iOS/issues/18783
! AztecInputState.getCurrentFocusedElement()
) {
this.updateCaretData( event );

this._onPress( event );
this._onPress(); // Call to move the focus in RN way (TextInputState)
}
}

Expand Down Expand Up @@ -285,9 +311,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

0 comments on commit 5be930a

Please sign in to comment.