Skip to content

Commit

Permalink
Removing <TextInput>'s onTextInput event
Browse files Browse the repository at this point in the history
Summary:
This is the first diff in the series. It removes Flow types for this feature to verify that we actually do not have any usages. After it lands, we will remove actual support on the native side.

There are several reasons why removing it is a good idea:
* There is no any evidence that this feature is actually useful. That was discussed several times (e.g. see T7936714) during RN lifetime and the overall consensus is: We need something else, something like sync `onChange` event instead of it.
* Supporting the previous point, it's not used (at least inside Facebook). I searched hard and I could find only one place where it's used: in the TextInput Example.
* To deliver more functionality we should lean towards W3C specs, this one is not W3C compliant.
* Supporting this Feature in Fabric is quite challenging, so I want to do it sooner than later.
* This feature was never documented.

Changelog: [Breaking] `<TextInput>::onTextInput` event was removed

Reviewed By: TheSavior

Differential Revision: D18456175

fbshipit-source-id: c7a8ed7a86b33ecc01d45497645fe249556fdf96
  • Loading branch information
shergin authored and facebook-github-bot committed Nov 19, 2019
1 parent 9f0b80b commit 3f7e0a2
Showing 1 changed file with 0 additions and 8 deletions.
8 changes: 0 additions & 8 deletions Libraries/Components/TextInput/TextInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,6 @@ export type Props = $ReadOnly<{|
*/
onContentSizeChange?: ?(e: ContentSizeChangeEvent) => mixed,

onTextInput?: ?(e: TextInputEvent) => mixed,

/**
* Callback that is called when text input ends.
*/
Expand Down Expand Up @@ -948,7 +946,6 @@ class InternalTextInput extends React.Component<Props> {
onScroll={this._onScroll}
onSelectionChange={this._onSelectionChange}
onSelectionChangeShouldSetResponder={emptyFunctionThatReturnsTrue}
onTextInput={this._onTextInput}
selection={selection}
style={style}
text={this._getText()}
Expand Down Expand Up @@ -983,7 +980,6 @@ class InternalTextInput extends React.Component<Props> {
onFocus={this._onFocus}
onScroll={this._onScroll}
onSelectionChange={this._onSelectionChange}
onTextInput={this._onTextInput}
selection={selection}
style={style}
text={this._getText()}
Expand Down Expand Up @@ -1111,10 +1107,6 @@ class InternalTextInput extends React.Component<Props> {
}
};

_onTextInput = (event: TextInputEvent) => {
this.props.onTextInput && this.props.onTextInput(event);
};

_onScroll = (event: ScrollEvent) => {
this.props.onScroll && this.props.onScroll(event);
};
Expand Down

1 comment on commit 3f7e0a2

@stigi
Copy link
Contributor

@stigi stigi commented on 3f7e0a2 Aug 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shergin With this change we have to rely on onChange or onChangeText neither of which carries the range attribute that TextInputEvent had. As far as I see it there's also no easy way to get it otherwise (without guessing). This breaks our rich text editor we've built internally, that needs the range to i.e. handle autocorrect and other things.

Please sign in to comment.