Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
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
23 changes: 13 additions & 10 deletions Examples/UIExplorer/TextInputExample.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,21 @@ var TextEventsExample = React.createClass({
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.

Should FlowType State type be added for this?

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,
});
onContentSizeChange={(contentSize) => {
this.setState({height: contentSize.height});
}}
onChangeText={(text) => {

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.

Maybe leave this until we've implemented onTextChange?

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.

onChangeText already works, I just mentioned that we should probably rename it for consistency. Unless you mean something else?

this.setState({text});
}}
style={[styles.default, {height: Math.max(35, this.state.height)}]}
value={this.state.text}
Expand Down Expand Up @@ -412,19 +415,19 @@ exports.examples = [
render: function() {
return (
<View>
<TextInput
<TextInput
style={[styles.singleLine, {fontFamily: 'sans-serif'}]}
placeholder="Custom fonts like Sans-Serif are supported"
/>
<TextInput
<TextInput
style={[styles.singleLine, {fontFamily: 'sans-serif', fontWeight: 'bold'}]}
placeholder="Sans-Serif bold"
/>
<TextInput
<TextInput
style={[styles.singleLine, {fontFamily: 'sans-serif', fontStyle: 'italic'}]}
placeholder="Sans-Serif italic"
/>
<TextInput
<TextInput
style={[styles.singleLine, {fontFamily: 'serif'}]}
placeholder="Serif"
/>
Expand Down
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
23 changes: 23 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 that is called when the text input's content size changes.
* 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 @@ -523,6 +528,11 @@ const TextInput = React.createClass({
};
}

var onContentSizeChange;
if (this.props.onContentSizeChange) {
onContentSizeChange = this._onContentSizeChange;
}

var props = Object.assign({}, this.props);
props.style = [styles.input, this.props.style];
if (!props.multiline) {
Expand Down Expand Up @@ -566,6 +576,7 @@ const TextInput = React.createClass({
onFocus={this._onFocus}
onBlur={this._onBlur}
onChange={this._onChange}
onContentSizeChange={onContentSizeChange}
onSelectionChange={onSelectionChange}
onTextInput={this._onTextInput}
onSelectionChangeShouldSetResponder={emptyFunction.thatReturnsTrue}
Expand Down Expand Up @@ -599,6 +610,11 @@ const TextInput = React.createClass({
};
}

var onContentSizeChange;
if (this.props.onContentSizeChange) {
onContentSizeChange = this._onContentSizeChange;
}

var autoCapitalize =
UIManager.AndroidTextInput.Constants.AutoCapitalizationType[this.props.autoCapitalize];
var children = this.props.children;
Expand Down Expand Up @@ -626,6 +642,7 @@ const TextInput = React.createClass({
onFocus={this._onFocus}
onBlur={this._onBlur}
onChange={this._onChange}
onContentSizeChange={onContentSizeChange}
onSelectionChange={onSelectionChange}
onTextInput={this._onTextInput}
onEndEditing={this.props.onEndEditing}
Expand Down Expand Up @@ -673,6 +690,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
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,12 @@

/* package */ static Map getDirectEventTypeConstants() {
return MapBuilder.builder()
.put("topSelectionChange", MapBuilder.of("registrationName", "onSelectionChange"))
.put("topLoadingStart", MapBuilder.of("registrationName", "onLoadingStart"))
.put("topLoadingFinish", MapBuilder.of("registrationName", "onLoadingFinish"))
.put("topLoadingError", MapBuilder.of("registrationName", "onLoadingError"))
.put("topContentSizeChange", MapBuilder.of("registrationName", "onContentSizeChange"))
.put("topLayout", MapBuilder.of("registrationName", "onLayout"))
.put("topLoadingError", MapBuilder.of("registrationName", "onLoadingError"))
.put("topLoadingFinish", MapBuilder.of("registrationName", "onLoadingFinish"))
.put("topLoadingStart", MapBuilder.of("registrationName", "onLoadingStart"))
.put("topSelectionChange", MapBuilder.of("registrationName", "onSelectionChange"))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ public void scrollTo(
Map getExportedCustomDirectEventTypeConstants() {
return MapBuilder.builder()
.put(ScrollEventType.SCROLL.getJSEventName(), MapBuilder.of("registrationName", "onScroll"))
.put(
ContentSizeChangeEvent.EVENT_NAME,
MapBuilder.of("registrationName", "onContentSizeChange"))
.build();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can't get rid of this. We use this internally in apps.

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.

The event was moved here https://github.com/facebook/react-native/pull/8457/files#diff-0404c7801b7b68d8a58a11f338c5d666R74 since onContentSizeChange is now used by scollview and textinput. Is there a difference between doing it in the manager vs in uimanager that I'm not aware of?

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

package com.facebook.react.views.textinput;

public interface ContentSizeWatcher {

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.

The coding convention is Listener instead of Watcher I think

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.

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.

I guess we could rename both to ...Listener as this is what is used usually on android.

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.

Actually the name is probably based on android.text.TextWatcher so we can keep it this way.

public void onLayout();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

package com.facebook.react.views.textinput;

import com.facebook.react.bridge.Arguments;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.uimanager.events.Event;
import com.facebook.react.uimanager.events.RCTEventEmitter;

/**
* Event emitted by EditText native view when content size changes.
*/
public class ReactContentSizeChangedEvent extends Event<ReactTextChangedEvent> {

public static final String EVENT_NAME = "topContentSizeChange";

private int mContentWidth;
private int mContentHeight;

public ReactContentSizeChangedEvent(
int viewId,
long timestampMs,
int contentSizeWidth,
int contentSizeHeight) {
super(viewId, timestampMs);
mContentWidth = contentSizeWidth;
mContentHeight = contentSizeHeight;
}

@Override
public String getEventName() {
return EVENT_NAME;
}

@Override
public void dispatch(RCTEventEmitter rctEventEmitter) {
rctEventEmitter.receiveEvent(getViewTag(), getEventName(), serializeEventData());
}

private WritableMap serializeEventData() {
WritableMap eventData = Arguments.createMap();

WritableMap contentSize = Arguments.createMap();
contentSize.putDouble("width", mContentWidth);
contentSize.putDouble("height", mContentHeight);
eventData.putMap("contentSize", contentSize);

eventData.putInt("target", getViewTag());
return eventData;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public class ReactEditText extends EditText {
private boolean mContainsImages;
private boolean mBlurOnSubmit;
private @Nullable SelectionWatcher mSelectionWatcher;
private @Nullable ContentSizeWatcher mContentSizeWatcher;
private final InternalKeyListener mKeyListener;

private static final KeyListener sKeyListener = QwertyKeyListener.getInstanceForFullKeyboard();
Expand Down Expand Up @@ -102,7 +103,23 @@ public ReactEditText(Context context) {
// TODO: t6408636 verify if we should schedule a layout after a View does a requestLayout()
@Override
public boolean isLayoutRequested() {
return false;
// If we are watching and updating container height based on content size

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.

If you return true here, in the case of single-line text, the cursor will go out of view after you've typed after the end of the visible line. That means that onContentSizeChange should be limited to only work with multiline texts.

// then we don't want to scroll right away. This isn't perfect -- you might
// want to limit the height the text input can grow to. Possible solution
// is to add another prop that determines whether we should scroll to end
// of text.
if (mContentSizeWatcher != null) {
return true;
} else {
return false;
}
}

@Override
protected void onLayout(boolean changed, int left, int top, int right, int bottom) {
if (mContentSizeWatcher != null) {
mContentSizeWatcher.onLayout();
}
}

// Consume 'Enter' key events: TextView tries to give focus to the next TextInput, but it can't
Expand Down Expand Up @@ -162,6 +179,10 @@ public void removeTextChangedListener(TextWatcher watcher) {
}
}

public void setContentSizeWatcher(ContentSizeWatcher contentSizeWatcher) {
mContentSizeWatcher = contentSizeWatcher;
}

@Override
protected void onSelectionChanged(int selStart, int selEnd) {
super.onSelectionChanged(selStart, selEnd);
Expand Down Expand Up @@ -447,7 +468,7 @@ public void onTextChanged(CharSequence s, int start, int before, int count) {
@Override
public void afterTextChanged(Editable s) {
if (!mIsSettingTextFromJS && mListeners != null) {
for (android.text.TextWatcher listener : mListeners) {
for (TextWatcher listener : mListeners) {
listener.afterTextChanged(s);
}
}
Expand Down
Loading