Skip to content

Commit

Permalink
Allow focusing TextInput when already focused
Browse files Browse the repository at this point in the history
Summary:
Right now, `requestFocus()` is a no-op if the EditText view thinks it's already focused. In certain cases, though, we still want to focus the view even if it's already focused - for example, if TalkBack is enabled and you dismiss the keyboard, you want to be able to tap on the TextInput again to bring back the keyboard, even though the View never thinks it lost focus.

What I'm doing instead is basically disregarding the View's current focus state if we *would* focus the TextInput, which is in 3 circumstances:

- When the view is attached to a window, if autofocus is true
- When the focus is being requested by JS
- When the focus is being requested by an accessibility action from the OS

Changelog: [Android][Fixed] Change how TextInput responds to requestFocus to fix a11y focus issue

Reviewed By: mdvacca

Differential Revision: D19750312

fbshipit-source-id: 30b9fab40af4a083fa98f57aba7e586540238bea
  • Loading branch information
Emily Janzer authored and facebook-github-bot committed Feb 18, 2020
1 parent d8ff5a5 commit d4a498a
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import android.util.TypedValue;
import android.view.View;
import android.view.ViewGroup;
import android.view.accessibility.AccessibilityNodeInfo;
import android.view.inputmethod.EditorInfo;
import android.widget.EditText;
import com.facebook.react.bridge.JavaScriptModule;
Expand Down Expand Up @@ -107,6 +108,97 @@ public void testOnSubmitEditing() throws Throwable {
fireEditorActionAndCheckRecording(reactEditText, EditorInfo.IME_ACTION_NONE);
}

public void testRequestFocusDoesNothing() throws Throwable {
String testId = "textInput1";

final ReactEditText reactEditText = getViewByTestId(testId);
runTestOnUiThread(
new Runnable() {
@Override
public void run() {
reactEditText.clearFocus();
}
});
waitForBridgeAndUIIdle();
assertFalse(reactEditText.isFocused());

runTestOnUiThread(
new Runnable() {
@Override
public void run() {
reactEditText.requestFocus();
}
});
waitForBridgeAndUIIdle();

// Calling requestFocus() directly should no-op
assertFalse(reactEditText.isFocused());
}

public void testRequestFocusFromJS() throws Throwable {
String testId = "textInput1";

final ReactEditText reactEditText = getViewByTestId(testId);

runTestOnUiThread(
new Runnable() {
@Override
public void run() {
reactEditText.clearFocus();
}
});
waitForBridgeAndUIIdle();
assertFalse(reactEditText.isFocused());

runTestOnUiThread(
new Runnable() {
@Override
public void run() {
reactEditText.requestFocusFromJS();
}
});
waitForBridgeAndUIIdle();
assertTrue(reactEditText.isFocused());
}

public void testAccessibilityFocus() throws Throwable {
String testId = "textInput1";

final ReactEditText reactEditText = getViewByTestId(testId);
runTestOnUiThread(
new Runnable() {
@Override
public void run() {
reactEditText.clearFocus();
}
});
waitForBridgeAndUIIdle();
assertFalse(reactEditText.isFocused());

runTestOnUiThread(
new Runnable() {
@Override
public void run() {
reactEditText.performAccessibilityAction(
AccessibilityNodeInfo.ACTION_ACCESSIBILITY_FOCUS, null);
reactEditText.performAccessibilityAction(AccessibilityNodeInfo.ACTION_CLICK, null);
}
});
waitForBridgeAndUIIdle();
assertTrue(reactEditText.isFocused());

runTestOnUiThread(
new Runnable() {
@Override
public void run() {
reactEditText.performAccessibilityAction(
AccessibilityNodeInfo.ACTION_CLEAR_FOCUS, null);
}
});
waitForBridgeAndUIIdle();
assertFalse(reactEditText.isFocused());
}

private void fireEditorActionAndCheckRecording(
final ReactEditText reactEditText, final int actionId) throws Throwable {
fireEditorActionAndCheckRecording(reactEditText, actionId, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ public class ReactEditText extends AppCompatEditText {
// *TextChanged events should be triggered. This is less expensive than removing the text
// listeners and adding them back again after the text change is completed.
protected boolean mIsSettingTextFromJS;
// This component is controlled, so we want it to get focused only when JS ask it to do so.
// Whenever android requests focus, except for accessibility click, it will be ignored.
private boolean mShouldAllowFocus;
private int mDefaultGravityHorizontal;
private int mDefaultGravityVertical;

Expand Down Expand Up @@ -127,7 +124,6 @@ public ReactEditText(Context context) {
mNativeEventCount = 0;
mMostRecentEventCount = 0;
mIsSettingTextFromJS = false;
mShouldAllowFocus = false;
mBlurOnSubmit = null;
mDisableFullscreen = false;
mListeners = null;
Expand All @@ -152,10 +148,7 @@ public ReactEditText(Context context) {
@Override
public boolean performAccessibilityAction(View host, int action, Bundle args) {
if (action == AccessibilityNodeInfo.ACTION_CLICK) {
mShouldAllowFocus = true;
requestFocus();
mShouldAllowFocus = false;
return true;
return requestFocusInternal();
}
return super.performAccessibilityAction(host, action, args);
}
Expand Down Expand Up @@ -248,18 +241,18 @@ public void clearFocus() {

@Override
public boolean requestFocus(int direction, Rect previouslyFocusedRect) {
// Always return true if we are already focused. This is used by android in certain places,
// such as text selection.
if (isFocused()) {
return true;
}

if (!mShouldAllowFocus) {
return false;
}
// This is a no-op so that when the OS calls requestFocus(), nothing will happen. ReactEditText
// 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.

This comment has been minimized.

Copy link
@msgharpu

msgharpu Jul 6, 2021

There's another case wherein, user presses tab or shift+tab from a hardware keyboard. Due to this no-op there's an active bug. Is it by design?

return isFocused();
}

private boolean requestFocusInternal() {
setFocusableInTouchMode(true);
boolean focused = super.requestFocus(direction, previouslyFocusedRect);
// We must explicitly call this method on the super class; if we call requestFocus() without
// any arguments, it will call into the overridden requestFocus(int, Rect) above, which no-ops.
boolean focused = super.requestFocus(View.FOCUS_DOWN, null);
if (getShowSoftInputOnFocus()) {
showSoftKeyboard();
}
Expand Down Expand Up @@ -461,9 +454,7 @@ public void maybeUpdateTypeface() {

// VisibleForTesting from {@link TextInputEventsTestCase}.
public void requestFocusFromJS() {
mShouldAllowFocus = true;
requestFocus();
mShouldAllowFocus = false;
requestFocusInternal();
}

/* package */ void clearFocusFromJS() {
Expand Down Expand Up @@ -754,9 +745,7 @@ public void onAttachedToWindow() {
}

if (mAutoFocus && !mDidAttachToWindow) {
mShouldAllowFocus = true;
requestFocus();
mShouldAllowFocus = false;
requestFocusInternal();
}

mDidAttachToWindow = true;
Expand Down

0 comments on commit d4a498a

Please sign in to comment.