Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions Examples/UIExplorer/TextInputExample.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,21 @@ class AutoExpandingTextInput extends React.Component {

constructor(props) {
super(props);
this.state = {text: '', height: 0};
this.state = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here about FlowType: Should FlowType State type be added for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is not needed as flow can infer the types from the object literal.

text: 'React Native enables you to build world-class application experiences on native platforms using a consistent developer experience based on JavaScript and React. The focus of React Native is on developer efficiency across all the platforms you care about — learn once, write anywhere. Facebook uses React Native in multiple production apps and will continue investing in React Native.',
height: 0,
};
}
render() {
return (
<TextInput
{...this.props}
multiline={true}
onChange={(event) => {
this.setState({
text: event.nativeEvent.text,
height: event.nativeEvent.contentSize.height,
});
onChangeText={(text) => {
this.setState({text});
}}
onContentSizeChange={(contentSize) => {
this.setState({height: contentSize.height});
}}
style={[styles.default, {height: Math.max(35, this.state.height)}]}
value={this.state.text}
Expand Down
12 changes: 12 additions & 0 deletions Libraries/Components/TextInput/TextInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,11 @@ const TextInput = React.createClass({
* Changed text is passed as an argument to the callback handler.
*/
onChangeText: PropTypes.func,
/**
* Callback this is called when the text input's content size changes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Callback this => Callback that

* The new content size is passed as an argument to the callback.
*/
onContentSizeChange: PropTypes.func,
/**
* Callback that is called when text input ends.
*/
Expand Down Expand Up @@ -566,6 +571,7 @@ const TextInput = React.createClass({
onFocus={this._onFocus}
onBlur={this._onBlur}
onChange={this._onChange}
onContentSizeChange={this._onContentSizeChange}
onSelectionChange={onSelectionChange}
onTextInput={this._onTextInput}
onSelectionChangeShouldSetResponder={emptyFunction.thatReturnsTrue}
Expand Down Expand Up @@ -673,6 +679,12 @@ const TextInput = React.createClass({
}
},

_onContentSizeChange(event: Event) {
if (this.props.onContentSizeChange) {
this.props.onContentSizeChange(event.nativeEvent.contentSize);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems unlike any of the other event handlers we provide. If we add a new property to this event, it's really hard to make it backwards compatible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea I wasn't sure about this, I did like onChangeText where we pass only the text. Personally I prefer having only the relevant value passed the the callback instead of the whole event in case where the event is pretty specific.

With that said I don't mind changing this to passing the event either as I can see it has some advantages and we may use this pattern more often.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With destructuring assignment the syntax overhead of unpacking an object vs. getting just the relevant value is only { } so I think we should go with the more future-proof approach and use the object.

}
},

_onChange: function(event: Event) {
// Make sure to fire the mostRecentEventCount first so it is already set on
// native when the text value is set.
Expand Down
1 change: 1 addition & 0 deletions Libraries/Text/RCTTextView.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
@property (nonatomic, strong) NSNumber *maxLength;

@property (nonatomic, copy) RCTDirectEventBlock onChange;
@property (nonatomic, copy) RCTDirectEventBlock onContentSizeChange;
@property (nonatomic, copy) RCTDirectEventBlock onSelectionChange;
@property (nonatomic, copy) RCTDirectEventBlock onTextInput;

Expand Down
19 changes: 19 additions & 0 deletions Libraries/Text/RCTTextView.m
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ @implementation RCTTextView
BOOL _blockTextShouldChange;
BOOL _nativeUpdatesInFlight;
NSInteger _nativeEventCount;

CGSize _previousContentSize;
BOOL _sendContentSizeUpdates;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rename this to _viewDidCompleteInitialLayout?

}

- (instancetype)initWithEventDispatcher:(RCTEventDispatcher *)eventDispatcher
Expand Down Expand Up @@ -261,6 +264,17 @@ - (void)updateContentSize
size.height = [_textView sizeThatFits:size].height;
_scrollView.contentSize = size;
_textView.frame = (CGRect){CGPointZero, size};

if (_sendContentSizeUpdates && _onContentSizeChange && !CGSizeEqualToSize(_previousContentSize, size)) {
_previousContentSize = size;
_onContentSizeChange(@{
@"contentSize": @{
@"height": @(size.height),
@"width": @(size.width),
},
@"target": self.reactTag,
});
}
}

- (void)updatePlaceholder
Expand Down Expand Up @@ -633,6 +647,11 @@ - (BOOL)resignFirstResponder
- (void)layoutSubviews
{
[super layoutSubviews];

// Start sending content size updates only after the view has been layed out

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

laid out

// otherwise we send multiple events with bad dimentions on initial render.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dimensions

_sendContentSizeUpdates = YES;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@javache / @sahrens - can either of you confirm if this is the right thing to do here?


[self updateFrames];
}

Expand Down
1 change: 1 addition & 0 deletions Libraries/Text/RCTTextViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ - (UIView *)view
RCT_REMAP_VIEW_PROPERTY(keyboardAppearance, textView.keyboardAppearance, UIKeyboardAppearance)
RCT_EXPORT_VIEW_PROPERTY(maxLength, NSNumber)
RCT_EXPORT_VIEW_PROPERTY(onChange, RCTBubblingEventBlock)
RCT_EXPORT_VIEW_PROPERTY(onContentSizeChange, RCTBubblingEventBlock)
RCT_EXPORT_VIEW_PROPERTY(onSelectionChange, RCTDirectEventBlock)
RCT_EXPORT_VIEW_PROPERTY(onTextInput, RCTDirectEventBlock)
RCT_EXPORT_VIEW_PROPERTY(placeholder, NSString)
Expand Down