Skip to content

Commit

Permalink
Allow text inputs to handle native requestFocus calls (#48547)
Browse files Browse the repository at this point in the history
Summary:

For some unknown reason, we have been swallowing [`requestFocus`](https://developer.android.com/reference/android/view/View#requestFocus(int) calls since `TextInput` is a controlled component - meaning you can control this components value and focus state from JS. This decision was originally made pre 2015 and I cannot find the reason why

I do not think this makes sense. We can still request focus from JS, while allowing the OS to request focus as well in certain cases and we would still be controlling this text input.

This is breaking keyboard navigation. Pressing tab or arrow keys will no-op if the next destination is a `TextInput`. This is because Android will call `requestFocus` from [here](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/view/ViewRootImpl.java;l=7868?q=performFocusNavigation) when handling key events. Notably, Explore By Touch (TalkBack) swiping gestures WOULD focus `TextInputs` since they go through `ExploreByTouchHelper` methods which we override to call the proper `requestFocusInternal()` method.

**In this diff**: I move the logic in `requestFocusInternal()` into `requestFocus`.

Reviewed By: NickGerleman

Differential Revision: D67953398
  • Loading branch information
joevilches authored and facebook-github-bot committed Jan 14, 2025
1 parent e3c63cf commit 43470e4
Show file tree
Hide file tree
Showing 22 changed files with 184 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<3e10f8d2f623da3b7b502d8fa78f82a4>>
* @generated SignedSource<<2c321a7a8e811dc238a75f76180843b9>>
*/

/**
Expand Down Expand Up @@ -244,6 +244,12 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun useAlwaysAvailableJSErrorHandling(): Boolean = accessor.useAlwaysAvailableJSErrorHandling()

/**
* If true, focusing in ReactEditText will mainly use stock Android requestFocus() behavior. If false it will use legacy custom focus behavior.
*/
@JvmStatic
public fun useEditTextStockAndroidFocusBehavior(): Boolean = accessor.useEditTextStockAndroidFocusBehavior()

/**
* Should this application enable the Fabric Interop Layer for Android? If yes, the application will behave so that it can accept non-Fabric components and render them on Fabric. This toggle is controlling extra logic such as custom event dispatching that are needed for the Fabric Interop Layer to work correctly.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<497bbb23778fe0f9763e9bfa715ea3aa>>
* @generated SignedSource<<e724939eaa9fa53c9b25c8a5a1e3196d>>
*/

/**
Expand Down Expand Up @@ -56,6 +56,7 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var loadVectorDrawablesOnImagesCache: Boolean? = null
private var traceTurboModulePromiseRejectionsOnAndroidCache: Boolean? = null
private var useAlwaysAvailableJSErrorHandlingCache: Boolean? = null
private var useEditTextStockAndroidFocusBehaviorCache: Boolean? = null
private var useFabricInteropCache: Boolean? = null
private var useImmediateExecutorInAndroidBridgelessCache: Boolean? = null
private var useNativeViewConfigsInBridgelessModeCache: Boolean? = null
Expand Down Expand Up @@ -390,6 +391,15 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun useEditTextStockAndroidFocusBehavior(): Boolean {
var cached = useEditTextStockAndroidFocusBehaviorCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.useEditTextStockAndroidFocusBehavior()
useEditTextStockAndroidFocusBehaviorCache = cached
}
return cached
}

override fun useFabricInterop(): Boolean {
var cached = useFabricInteropCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<d801f87c988fbd921e2379f236e1711f>>
* @generated SignedSource<<a7f3ca986725fa85a6926c9e7966669b>>
*/

/**
Expand Down Expand Up @@ -100,6 +100,8 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun useAlwaysAvailableJSErrorHandling(): Boolean

@DoNotStrip @JvmStatic public external fun useEditTextStockAndroidFocusBehavior(): Boolean

@DoNotStrip @JvmStatic public external fun useFabricInterop(): Boolean

@DoNotStrip @JvmStatic public external fun useImmediateExecutorInAndroidBridgeless(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<5de2cfc00f486b7d07266939ce18a397>>
* @generated SignedSource<<85d40be44d053b58a74ad76467c8e5e9>>
*/

/**
Expand Down Expand Up @@ -95,6 +95,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun useAlwaysAvailableJSErrorHandling(): Boolean = false

override fun useEditTextStockAndroidFocusBehavior(): Boolean = true

override fun useFabricInterop(): Boolean = false

override fun useImmediateExecutorInAndroidBridgeless(): Boolean = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<f1ed4107e24ced5d2673bfb065573062>>
* @generated SignedSource<<18d5c2ffa66a36e364bc358a144534ec>>
*/

/**
Expand Down Expand Up @@ -60,6 +60,7 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var loadVectorDrawablesOnImagesCache: Boolean? = null
private var traceTurboModulePromiseRejectionsOnAndroidCache: Boolean? = null
private var useAlwaysAvailableJSErrorHandlingCache: Boolean? = null
private var useEditTextStockAndroidFocusBehaviorCache: Boolean? = null
private var useFabricInteropCache: Boolean? = null
private var useImmediateExecutorInAndroidBridgelessCache: Boolean? = null
private var useNativeViewConfigsInBridgelessModeCache: Boolean? = null
Expand Down Expand Up @@ -430,6 +431,16 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun useEditTextStockAndroidFocusBehavior(): Boolean {
var cached = useEditTextStockAndroidFocusBehaviorCache
if (cached == null) {
cached = currentProvider.useEditTextStockAndroidFocusBehavior()
accessedFeatureFlags.add("useEditTextStockAndroidFocusBehavior")
useEditTextStockAndroidFocusBehaviorCache = cached
}
return cached
}

override fun useFabricInterop(): Boolean {
var cached = useFabricInteropCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<3cd802bdd1d383ea0668e43319d53b3f>>
* @generated SignedSource<<4f90c47ea6f23c2ebfae1f35790c4af5>>
*/

/**
Expand Down Expand Up @@ -95,6 +95,8 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun useAlwaysAvailableJSErrorHandling(): Boolean

@DoNotStrip public fun useEditTextStockAndroidFocusBehavior(): Boolean

@DoNotStrip public fun useFabricInterop(): Boolean

@DoNotStrip public fun useImmediateExecutorInAndroidBridgeless(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import com.facebook.react.bridge.ReactSoftExceptionLogger;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.common.build.ReactBuildConfig;
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
import com.facebook.react.uimanager.BackgroundStyleApplicator;
import com.facebook.react.uimanager.LengthPercentage;
import com.facebook.react.uimanager.LengthPercentageType;
Expand Down Expand Up @@ -145,7 +146,9 @@ public class ReactEditText extends AppCompatEditText {

public ReactEditText(Context context) {
super(context);
setFocusableInTouchMode(false);
if (!ReactNativeFeatureFlags.useEditTextStockAndroidFocusBehavior()) {
setFocusableInTouchMode(false);
}

mInputMethodManager =
(InputMethodManager)
Expand Down Expand Up @@ -188,7 +191,9 @@ public boolean performAccessibilityAction(View host, int action, Bundle args) {
// selection on accessibility click to undo that.
setSelection(length);
}
return requestFocusInternal();
return ReactNativeFeatureFlags.useEditTextStockAndroidFocusBehavior()
? requestFocusProgramatically()
: requestFocusInternal();
}
return super.performAccessibilityAction(host, action, args);
}
Expand Down Expand Up @@ -338,7 +343,10 @@ public boolean onTextContextMenuItem(int id) {

@Override
public void clearFocus() {
setFocusableInTouchMode(false);
boolean useStockFocusBehavior = ReactNativeFeatureFlags.useEditTextStockAndroidFocusBehavior();
if (!useStockFocusBehavior) {
setFocusableInTouchMode(false);
}
super.clearFocus();
hideSoftKeyboard();
}
Expand All @@ -349,7 +357,9 @@ public boolean requestFocus(int direction, Rect previouslyFocusedRect) {
// is a controlled component, which means its focus is controlled by JS, with two exceptions:
// autofocus when it's attached to the window, and responding to accessibility events. In both
// of these cases, we call requestFocusInternal() directly.
return isFocused();
return ReactNativeFeatureFlags.useEditTextStockAndroidFocusBehavior()
? super.requestFocus(direction, previouslyFocusedRect)
: isFocused();
}

private boolean requestFocusInternal() {
Expand All @@ -360,6 +370,20 @@ private boolean requestFocusInternal() {
if (getShowSoftInputOnFocus()) {
showSoftKeyboard();
}

return focused;
}

// For cases like autoFocus, or ref.focus() where we request focus programatically and not through
// interacting with the EditText directly (like clicking on it). We cannot use stock
// requestFocus() because it will not pop up the soft keyboard, only clicking the input will do
// that. This method will eventually replace requestFocusInternal()
private boolean requestFocusProgramatically() {
boolean focused = super.requestFocus(View.FOCUS_DOWN, null);
if (isInTouchMode() && getShowSoftInputOnFocus()) {
showSoftKeyboard();
}

return focused;
}

Expand Down Expand Up @@ -632,7 +656,11 @@ public void maybeUpdateTypeface() {

// VisibleForTesting from {@link TextInputEventsTestCase}.
public void requestFocusFromJS() {
requestFocusInternal();
if (ReactNativeFeatureFlags.useEditTextStockAndroidFocusBehavior()) {
requestFocusProgramatically();
} else {
requestFocusInternal();
}
}

/* package */ void clearFocusFromJS() {
Expand Down Expand Up @@ -1079,7 +1107,11 @@ public void onAttachedToWindow() {
}

if (mAutoFocus && !mDidAttachToWindow) {
requestFocusInternal();
if (ReactNativeFeatureFlags.useEditTextStockAndroidFocusBehavior()) {
requestFocusProgramatically();
} else {
requestFocusInternal();
}
}

mDidAttachToWindow = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<7c3853858da56eb5f471abccf9dcbf55>>
* @generated SignedSource<<63ebe186ccd5ee30fc654aa8d90f27f9>>
*/

/**
Expand Down Expand Up @@ -255,6 +255,12 @@ class ReactNativeFeatureFlagsProviderHolder
return method(javaProvider_);
}

bool useEditTextStockAndroidFocusBehavior() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("useEditTextStockAndroidFocusBehavior");
return method(javaProvider_);
}

bool useFabricInterop() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("useFabricInterop");
Expand Down Expand Up @@ -493,6 +499,11 @@ bool JReactNativeFeatureFlagsCxxInterop::useAlwaysAvailableJSErrorHandling(
return ReactNativeFeatureFlags::useAlwaysAvailableJSErrorHandling();
}

bool JReactNativeFeatureFlagsCxxInterop::useEditTextStockAndroidFocusBehavior(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::useEditTextStockAndroidFocusBehavior();
}

bool JReactNativeFeatureFlagsCxxInterop::useFabricInterop(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::useFabricInterop();
Expand Down Expand Up @@ -677,6 +688,9 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
makeNativeMethod(
"useAlwaysAvailableJSErrorHandling",
JReactNativeFeatureFlagsCxxInterop::useAlwaysAvailableJSErrorHandling),
makeNativeMethod(
"useEditTextStockAndroidFocusBehavior",
JReactNativeFeatureFlagsCxxInterop::useEditTextStockAndroidFocusBehavior),
makeNativeMethod(
"useFabricInterop",
JReactNativeFeatureFlagsCxxInterop::useFabricInterop),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<77b4ed5aa33290ba9da1719544e974cb>>
* @generated SignedSource<<fdb23cf034dea348c661f59718f07291>>
*/

/**
Expand Down Expand Up @@ -138,6 +138,9 @@ class JReactNativeFeatureFlagsCxxInterop
static bool useAlwaysAvailableJSErrorHandling(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool useEditTextStockAndroidFocusBehavior(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool useFabricInterop(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.facebook.react.bridge.BridgeReactContext
import com.facebook.react.bridge.CatalystInstance
import com.facebook.react.bridge.JavaOnlyMap
import com.facebook.react.bridge.ReactTestHelper.createMockCatalystInstance
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
import com.facebook.react.uimanager.DisplayMetricsHolder
import com.facebook.react.uimanager.ReactStylesDiffMap
import com.facebook.react.uimanager.ThemedReactContext
Expand Down Expand Up @@ -65,6 +66,7 @@ class ReactTextInputPropertyTest {
manager = ReactTextInputManager()
DisplayMetricsHolder.setWindowDisplayMetrics(DisplayMetrics())
view = manager.createViewInstance(themedContext)
ReactNativeFeatureFlagsForTests.setUp()
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<7d301656072183649246db8fa738fc4d>>
* @generated SignedSource<<58c69846193598ce0c3666c37b8a185f>>
*/

/**
Expand Down Expand Up @@ -170,6 +170,10 @@ bool ReactNativeFeatureFlags::useAlwaysAvailableJSErrorHandling() {
return getAccessor().useAlwaysAvailableJSErrorHandling();
}

bool ReactNativeFeatureFlags::useEditTextStockAndroidFocusBehavior() {
return getAccessor().useEditTextStockAndroidFocusBehavior();
}

bool ReactNativeFeatureFlags::useFabricInterop() {
return getAccessor().useFabricInterop();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<eee9560521020eb75fa84c6e0cef31a3>>
* @generated SignedSource<<1d578508c3cd69bbf9616a811508a03e>>
*/

/**
Expand Down Expand Up @@ -219,6 +219,11 @@ class ReactNativeFeatureFlags {
*/
RN_EXPORT static bool useAlwaysAvailableJSErrorHandling();

/**
* If true, focusing in ReactEditText will mainly use stock Android requestFocus() behavior. If false it will use legacy custom focus behavior.
*/
RN_EXPORT static bool useEditTextStockAndroidFocusBehavior();

/**
* Should this application enable the Fabric Interop Layer for Android? If yes, the application will behave so that it can accept non-Fabric components and render them on Fabric. This toggle is controlling extra logic such as custom event dispatching that are needed for the Fabric Interop Layer to work correctly.
*/
Expand Down
Loading

0 comments on commit 43470e4

Please sign in to comment.