Skip to content

Commit

Permalink
Input, SmartUrlInput: Use modern React.createRef API.
Browse files Browse the repository at this point in the history
Now that the `Input` component clearly doesn't use the ref on its
child `TextInput` -- only `Input`'s consumers use it; see the
previous commit -- we can straightforwardly use the new
`React.createRef` API.

We can also do so in `SmartUrlInput`.

This will smoothen some necessary changes during the upcoming RN
v0.61 -> v0.62 upgrade. In particular, it seems that the way
`TextInput` is defined has changed in an interesting way (it
certainly has changed), or else something happened in the Flow
upgrade, to cause this error in several places where we use
`TextInput` as a type:

```
Cannot use `TextInput` as a type. A name can be used as a type only
if it refers to a type definition, an interface definition, or a
class definition. To get the type of a non-class value, use
`typeof`.
```

I discovered, in this commit, that `React$Ref`, not
`React$ElementRef`, seems to be the correct type for the `ref`
attribute and the return value of `React.createRef`. Weeks ago, we
used `React$RefElement` for `this.mentionWarnings` in `ComposeBox`
and overlooked [1] the fact that it ended up being more or less
completely un-typechecked, in 20bd98a. I'll add a TODO in the next
commit, saying we should fix that.

[1] zulip#4101 (comment)
  • Loading branch information
chrisbobbe committed Sep 2, 2020
1 parent 90c5994 commit ca25891
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 37 deletions.
8 changes: 2 additions & 6 deletions src/common/Input.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export type Props = $ReadOnly<{|
...$PropertyType<TextInput, 'props'>,
placeholder: LocalizableText,
onChangeText?: (text: string) => void,
textInputRef?: (component: ?TextInput) => void,
textInputRef?: React$Ref<typeof TextInput>,
|}>;

type State = {|
Expand Down Expand Up @@ -87,11 +87,7 @@ export default class Input extends PureComponent<Props, State> {
underlineColorAndroid={isFocused ? BORDER_COLOR : HALF_COLOR}
onFocus={this.handleFocus}
onBlur={this.handleBlur}
ref={(component: ?TextInput) => {
if (textInputRef) {
textInputRef(component);
}
}}
ref={textInputRef}
{...restProps}
/>
)}
Expand Down
10 changes: 4 additions & 6 deletions src/common/InputWithClearButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default class InputWithClearButton extends PureComponent<Props, State> {
canBeCleared: false,
text: '',
};
textInput: ?TextInput;
textInputRef = React.createRef<TextInput>();

handleChangeText = (text: string) => {
this.setState({
Expand All @@ -46,8 +46,8 @@ export default class InputWithClearButton extends PureComponent<Props, State> {

handleClear = () => {
this.handleChangeText('');
if (this.textInput) {
this.textInput.clear();
if (this.textInputRef.current) {
this.textInputRef.current.clear();
}
};

Expand All @@ -58,9 +58,7 @@ export default class InputWithClearButton extends PureComponent<Props, State> {
<View style={styles.row}>
<Input
{...this.props}
textInputRef={textInput => {
this.textInput = textInput;
}}
textInputRef={this.textInputRef}
onChangeText={this.handleChangeText}
value={text}
/>
Expand Down
20 changes: 11 additions & 9 deletions src/common/SmartUrlInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ export default class SmartUrlInput extends PureComponent<Props, State> {
state = {
value: '',
};
textInputRef: ?TextInput;
textInputRef = React.createRef<TextInput>();
focusListener: void | NavigationEventSubscription;

componentDidMount() {
this.focusListener = this.props.navigation.addListener('didFocus', () => {
if (this.textInputRef) {
this.textInputRef.focus();
if (this.textInputRef.current) {
this.textInputRef.current.focus();
}
});
}
Expand All @@ -92,9 +92,13 @@ export default class SmartUrlInput extends PureComponent<Props, State> {

urlPress = () => {
const { textInputRef } = this;
if (textInputRef) {
textInputRef.blur();
setTimeout(() => textInputRef.focus(), 100);
if (textInputRef.current) {
textInputRef.current.blur();
setTimeout(() => {
if (textInputRef.current) {
textInputRef.current.focus();
}
}, 100);
}
};

Expand Down Expand Up @@ -144,9 +148,7 @@ export default class SmartUrlInput extends PureComponent<Props, State> {
underlineColorAndroid="transparent"
onSubmitEditing={onSubmitEditing}
enablesReturnKeyAutomatically={enablesReturnKeyAutomatically}
ref={(component: TextInput | null) => {
this.textInputRef = component;
}}
ref={this.textInputRef}
/>
{!value && this.renderPlaceholderPart(defaultOrganization)}
{suffix !== null && this.renderPlaceholderPart(suffix)}
Expand Down
28 changes: 12 additions & 16 deletions src/compose/ComposeBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ function randomInt(min, max) {
return Math.floor(Math.random() * (max - min + 1)) + min;
}

export const updateTextInput = (textInput: ?TextInput, text: string): void => {
if (!textInput) {
export const updateTextInput = (textInput: TextInput | null, text: string): void => {
if (textInput === null) {
// Depending on the lifecycle events this function is called from,
// this might not be set yet.
return;
Expand All @@ -125,8 +125,8 @@ class ComposeBox extends PureComponent<Props, State> {
static contextType = ThemeContext;
context: ThemeData;

messageInput: ?TextInput = null;
topicInput: ?TextInput = null;
messageInputRef = React.createRef<TextInput>();
topicInputRef = React.createRef<TextInput>();
mentionWarnings: React$ElementRef<MentionWarnings> = React.createRef();
inputBlurTimeoutId: ?TimeoutID = null;

Expand Down Expand Up @@ -165,12 +165,12 @@ class ComposeBox extends PureComponent<Props, State> {
};

setMessageInputValue = (message: string) => {
updateTextInput(this.messageInput, message);
updateTextInput(this.messageInputRef.current, message);
this.handleMessageChange(message);
};

setTopicInputValue = (topic: string) => {
updateTextInput(this.topicInput, topic);
updateTextInput(this.topicInputRef.current, topic);
this.handleTopicChange(topic);
};

Expand Down Expand Up @@ -327,8 +327,8 @@ class ComposeBox extends PureComponent<Props, State> {
});
}
completeEditMessage();
if (this.messageInput) {
this.messageInput.blur();
if (this.messageInputRef.current !== null) {
this.messageInputRef.current.blur();
}
};

Expand All @@ -341,8 +341,8 @@ class ComposeBox extends PureComponent<Props, State> {
const message = nextProps.editMessage ? nextProps.editMessage.content : '';
this.setMessageInputValue(message);
this.setTopicInputValue(topic);
if (this.messageInput) {
this.messageInput.focus();
if (this.messageInputRef.current !== null) {
this.messageInputRef.current.focus();
}
}
}
Expand Down Expand Up @@ -454,9 +454,7 @@ class ComposeBox extends PureComponent<Props, State> {
placeholder="Topic"
defaultValue={topic}
selectTextOnFocus
textInputRef={component => {
this.topicInput = component;
}}
textInputRef={this.topicInputRef}
onChangeText={this.handleTopicChange}
onFocus={this.handleTopicFocus}
onBlur={this.handleTopicBlur}
Expand All @@ -472,9 +470,7 @@ class ComposeBox extends PureComponent<Props, State> {
underlineColorAndroid="transparent"
placeholder={placeholder}
defaultValue={message}
textInputRef={component => {
this.messageInput = component;
}}
textInputRef={this.messageInputRef}
onBlur={this.handleMessageBlur}
onChangeText={this.handleMessageChange}
onFocus={this.handleMessageFocus}
Expand Down

0 comments on commit ca25891

Please sign in to comment.