Skip to content

Commit

Permalink
Migrate TextInputState to take a ref instead of a reactTag
Browse files Browse the repository at this point in the history
Summary:
Changelog:
[General][Breaking] Multiple deprecations and breaking changes to TextInputState. Use native component refs instead of react tags

Reviewed By: JoshuaGross

Differential Revision: D19458214

fbshipit-source-id: f67649657fa44b6c707a0ac91f07d2ceb433bc70
  • Loading branch information
TheSavior authored and facebook-github-bot committed Feb 19, 2020
1 parent 862c719 commit 6286270
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 45 deletions.
37 changes: 28 additions & 9 deletions Libraries/Components/ScrollResponder.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,12 @@ const ScrollResponderMixin = {
return false;
}

const currentlyFocusedTextInput = TextInputState.currentlyFocusedField();
const currentlyFocusedInput = TextInputState.currentlyFocusedInput();

if (
this.props.keyboardShouldPersistTaps === 'handled' &&
currentlyFocusedTextInput != null &&
ReactNative.findNodeHandle(e.target) !== currentlyFocusedTextInput
currentlyFocusedInput != null &&
e.target !== currentlyFocusedInput
) {
return true;
}
Expand Down Expand Up @@ -224,17 +224,26 @@ const ScrollResponderMixin = {
// and a new touch starts with a non-textinput target (in which case the
// first tap should be sent to the scroll view and dismiss the keyboard,
// then the second tap goes to the actual interior view)
const currentlyFocusedTextInput = TextInputState.currentlyFocusedField();
const currentlyFocusedTextInput = TextInputState.currentlyFocusedInput();
const {keyboardShouldPersistTaps} = this.props;
const keyboardNeverPersistTaps =
!keyboardShouldPersistTaps || keyboardShouldPersistTaps === 'never';

const reactTag = ReactNative.findNodeHandle(e.target);
if (typeof e.target === 'number') {
if (__DEV__) {
console.error(

This comment has been minimized.

Copy link
@janicduplessis

janicduplessis Feb 22, 2020

Contributor

@TheSavior I'm seeing these errors since updating to latest master.

This comment has been minimized.

Copy link
@janicduplessis

janicduplessis Feb 22, 2020

Contributor

Looks like ReactNativeRenderer-dev.js does not use feature flags in shims/ReactFeatureFlags.js like ReactNativeRenderer-dev.fb.js does so enableNativeTargetAsInstance is false for OSS build.

This comment has been minimized.

Copy link
@TheSavior

TheSavior Feb 22, 2020

Author Member

Ah good catch. I need to flip the flags on all the other builds. If you send a build to the react repo I’ll merge. Otherwise I’ll get to it next week.

I really need to delete the flag from the react repo and leave only the true case but that it is more involved than just flipping the flags

This comment has been minimized.

Copy link
@janicduplessis

janicduplessis Feb 22, 2020

Contributor

Not sure how feature flags work for react builds so I'll leave it to you for next week.

This comment has been minimized.

Copy link
@TheSavior

TheSavior Mar 2, 2020

Author Member

This landed in React in facebook/react@26aa198 and will be a part of the next React sync to RN

'Did not expect event target to be a number. Should have been a native component',
);
}

return false;
}

if (
keyboardNeverPersistTaps &&
currentlyFocusedTextInput != null &&
reactTag &&
!TextInputState.isTextInput(reactTag)
e.target != null &&
!TextInputState.isTextInput(e.target)
) {
return true;
}
Expand Down Expand Up @@ -300,14 +309,24 @@ const ScrollResponderMixin = {
scrollResponderHandleResponderRelease: function(e: PressEvent) {
this.props.onResponderRelease && this.props.onResponderRelease(e);

if (typeof e.target === 'number') {
if (__DEV__) {
console.error(
'Did not expect event target to be a number. Should have been a native component',
);
}

return;
}

// By default scroll views will unfocus a textField
// if another touch occurs outside of it
const currentlyFocusedTextInput = TextInputState.currentlyFocusedField();
const currentlyFocusedTextInput = TextInputState.currentlyFocusedInput();
if (
this.props.keyboardShouldPersistTaps !== true &&
this.props.keyboardShouldPersistTaps !== 'always' &&
currentlyFocusedTextInput != null &&
ReactNative.findNodeHandle(e.target) !== currentlyFocusedTextInput &&
e.target !== currentlyFocusedTextInput &&
!this.state.observedScrollSinceBecomingResponder &&
!this.state.becameResponderWhileAnimating
) {
Expand Down
21 changes: 11 additions & 10 deletions Libraries/Components/TextInput/TextInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -888,12 +888,13 @@ function InternalTextInput(props: Props): React.Node {
useFocusOnMount(props.autoFocus, inputRef);

useEffect(() => {
const tag = ReactNative.findNodeHandle(inputRef.current);
if (tag != null) {
TextInputState.registerInput(tag);
const inputRefValue = inputRef.current;

if (inputRefValue != null) {
TextInputState.registerInput(inputRefValue);

return () => {
TextInputState.unregisterInput(tag);
TextInputState.unregisterInput(inputRefValue);
};
}
}, [inputRef]);
Expand All @@ -915,10 +916,7 @@ function InternalTextInput(props: Props): React.Node {

// TODO: Fix this returning true on null === null, when no input is focused
function isFocused(): boolean {
return (
TextInputState.currentlyFocusedField() ===
ReactNative.findNodeHandle(inputRef.current)
);
return TextInputState.currentlyFocusedInput() === inputRef.current;
}

function getNativeRef(): ?React.ElementRef<HostComponent<mixed>> {
Expand Down Expand Up @@ -1009,14 +1007,14 @@ function InternalTextInput(props: Props): React.Node {
};

const _onFocus = (event: FocusEvent) => {
TextInputState.focusField(ReactNative.findNodeHandle(inputRef.current));
TextInputState.focusInput(inputRef.current);
if (props.onFocus) {
props.onFocus(event);
}
};

const _onBlur = (event: BlurEvent) => {
TextInputState.blurField(ReactNative.findNodeHandle(inputRef.current));
TextInputState.blurInput(inputRef.current);
if (props.onBlur) {
props.onBlur(event);
}
Expand Down Expand Up @@ -1143,13 +1141,16 @@ ExportedForwardRef.propTypes = DeprecatedTextInputPropTypes;

// $FlowFixMe
ExportedForwardRef.State = {
currentlyFocusedInput: TextInputState.currentlyFocusedInput,

currentlyFocusedField: TextInputState.currentlyFocusedField,
focusTextInput: TextInputState.focusTextInput,
blurTextInput: TextInputState.blurTextInput,
};

type TextInputComponentStatics = $ReadOnly<{|
State: $ReadOnly<{|
currentlyFocusedInput: typeof TextInputState.currentlyFocusedInput,
currentlyFocusedField: typeof TextInputState.currentlyFocusedField,
focusTextInput: typeof TextInputState.focusTextInput,
blurTextInput: typeof TextInputState.blurTextInput,
Expand Down
122 changes: 104 additions & 18 deletions Libraries/Components/TextInput/TextInputState.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,82 @@

'use strict';

const React = require('react');
const Platform = require('../../Utilities/Platform');
const UIManager = require('../../ReactNative/UIManager');
const {findNodeHandle} = require('../../Renderer/shims/ReactNative');

let currentlyFocusedID: ?number = null;
import type {HostComponent} from '../../Renderer/shims/ReactNativeTypes';
type ComponentRef = React.ElementRef<HostComponent<mixed>>;

let currentlyFocusedInputRef: ?ComponentRef = null;
const inputs = new Set();

function currentlyFocusedInput(): ?ComponentRef {
return currentlyFocusedInputRef;
}

/**
* Returns the ID of the currently focused text field, if one exists
* If no text field is focused it returns null
*/
function currentlyFocusedField(): ?number {
return currentlyFocusedID;
if (__DEV__) {
console.error(
'currentlyFocusedField is deprecated and will be removed in a future release. Use currentlyFocusedInput',
);
}

return findNodeHandle(currentlyFocusedInputRef);
}

function focusInput(textField: ?ComponentRef): void {
if (currentlyFocusedInputRef !== textField && textField != null) {
currentlyFocusedInputRef = textField;
}
}

function blurInput(textField: ?ComponentRef): void {
if (currentlyFocusedInputRef === textField && textField != null) {
currentlyFocusedInputRef = null;
}
}

function focusField(textFieldID: ?number): void {
if (currentlyFocusedID !== textFieldID && textFieldID != null) {
currentlyFocusedID = textFieldID;
if (__DEV__) {
console.error('focusField no longer works. Use focusInput');
}

return;
}

function blurField(textFieldID: ?number) {
if (currentlyFocusedID === textFieldID && textFieldID != null) {
currentlyFocusedID = null;
if (__DEV__) {
console.error('blurField no longer works. Use blurInput');
}

return;
}

/**
* @param {number} TextInputID id of the text field to focus
* Focuses the specified text field
* noop if the text field was already focused
*/
function focusTextInput(textFieldID: ?number) {
if (currentlyFocusedID !== textFieldID && textFieldID != null) {
focusField(textFieldID);
function focusTextInput(textField: ?ComponentRef) {
if (typeof textField === 'number') {
if (__DEV__) {
console.error(
'focusTextInput must be called with a host component. Passing a react tag is deprecated.',
);
}

return;
}

if (currentlyFocusedInputRef !== textField && textField != null) {
const textFieldID = findNodeHandle(textField);
focusInput(textField);
if (Platform.OS === 'ios') {
UIManager.focus(textFieldID);
} else if (Platform.OS === 'android') {
Expand All @@ -66,9 +108,20 @@ function focusTextInput(textFieldID: ?number) {
* Unfocuses the specified text field
* noop if it wasn't focused
*/
function blurTextInput(textFieldID: ?number) {
if (currentlyFocusedID === textFieldID && textFieldID != null) {
blurField(textFieldID);
function blurTextInput(textField: ?ComponentRef) {
if (typeof textField === 'number') {
if (__DEV__) {
console.error(
'focusTextInput must be called with a host component. Passing a react tag is deprecated.',
);
}

return;
}

if (currentlyFocusedInputRef === textField && textField != null) {
const textFieldID = findNodeHandle(textField);
blurInput(textField);
if (Platform.OS === 'ios') {
UIManager.blur(textFieldID);
} else if (Platform.OS === 'android') {
Expand All @@ -82,19 +135,52 @@ function blurTextInput(textFieldID: ?number) {
}
}

function registerInput(textFieldID: number) {
inputs.add(textFieldID);
function registerInput(textField: ComponentRef) {
if (typeof textField === 'number') {
if (__DEV__) {
console.error(
'registerInput must be called with a host component. Passing a react tag is deprecated.',
);
}

return;
}

inputs.add(textField);
}

function unregisterInput(textFieldID: number) {
inputs.delete(textFieldID);
function unregisterInput(textField: ComponentRef) {
if (typeof textField === 'number') {
if (__DEV__) {
console.error(
'unregisterInput must be called with a host component. Passing a react tag is deprecated.',
);
}

return;
}
inputs.delete(textField);
}

function isTextInput(textFieldID: number): boolean {
return inputs.has(textFieldID);
function isTextInput(textField: ComponentRef): boolean {
if (typeof textField === 'number') {
if (__DEV__) {
console.error(
'isTextInput must be called with a host component. Passing a react tag is deprecated.',
);
}

return false;
}

return inputs.has(textField);
}

module.exports = {
currentlyFocusedInput,
focusInput,
blurInput,

currentlyFocusedField,
focusField,
blurField,
Expand Down
26 changes: 19 additions & 7 deletions Libraries/Components/TextInput/__tests__/TextInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ describe('TextInput tests', () => {

expect(textInputRef.current.isFocused()).toBe(false);
ReactNative.findNodeHandle = jest.fn().mockImplementation(ref => {
if (ref == null) {
return null;
}

if (
ref === textInputRef.current ||
ref === textInputRef.current.getNativeRef()
Expand All @@ -97,13 +101,17 @@ describe('TextInput tests', () => {
return 2;
});

const inputTag = ReactNative.findNodeHandle(textInputRef.current);

TextInput.State.focusTextInput(inputTag);
TextInput.State.focusTextInput(textInputRef.current);
expect(textInputRef.current.isFocused()).toBe(true);
expect(TextInput.State.currentlyFocusedField()).toBe(inputTag);
TextInput.State.blurTextInput(inputTag);
expect(TextInput.State.currentlyFocusedInput()).toBe(textInputRef.current);
// This function is currently deprecated and will be removed in the future
expect(TextInput.State.currentlyFocusedField()).toBe(
ReactNative.findNodeHandle(textInputRef.current),
);
TextInput.State.blurTextInput(textInputRef.current);
expect(textInputRef.current.isFocused()).toBe(false);
expect(TextInput.State.currentlyFocusedInput()).toBe(null);
// This function is currently deprecated and will be removed in the future
expect(TextInput.State.currentlyFocusedField()).toBe(null);
});

Expand Down Expand Up @@ -141,16 +149,20 @@ describe('TextInput tests', () => {
const inputTag1 = ReactNative.findNodeHandle(textInputRe1.current);
const inputTag2 = ReactNative.findNodeHandle(textInputRe2.current);

TextInput.State.focusTextInput(inputTag1);
TextInput.State.focusTextInput(textInputRe1.current);

expect(textInputRe1.current.isFocused()).toBe(true);
expect(textInputRe2.current.isFocused()).toBe(false);
expect(TextInput.State.currentlyFocusedInput()).toBe(textInputRe1.current);
// This function is currently deprecated and will be removed in the future
expect(TextInput.State.currentlyFocusedField()).toBe(inputTag1);

TextInput.State.focusTextInput(inputTag2);
TextInput.State.focusTextInput(textInputRe2.current);

expect(textInputRe1.current.isFocused()).toBe(false);
expect(textInputRe2.current.isFocused()).toBe(true);
expect(TextInput.State.currentlyFocusedInput()).toBe(textInputRe2.current);
// This function is currently deprecated and will be removed in the future
expect(TextInput.State.currentlyFocusedField()).toBe(inputTag2);
});

Expand Down
2 changes: 1 addition & 1 deletion Libraries/Utilities/dismissKeyboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
const TextInputState = require('../Components/TextInput/TextInputState');

function dismissKeyboard() {
TextInputState.blurTextInput(TextInputState.currentlyFocusedField());
TextInputState.blurTextInput(TextInputState.currentlyFocusedInput());
}

module.exports = dismissKeyboard;

0 comments on commit 6286270

Please sign in to comment.