Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onContentSizeChange in textinput not getting height when text height changes. #11692

Closed
yasicmd opened this issue Jan 2, 2017 · 17 comments
Closed
Assignees
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@yasicmd
Copy link

yasicmd commented Jan 2, 2017

<TextInput
multiline={true}
onContentSizeChange={(event) => {
this.setState({height: event.nativeEvent.contentSize.height});
}}
onChangeText={(value) => {
this.setState({value});
}}
style={[{height: 40,borderWidth: 0.5,borderColor: '#0f0f0f',fontSize: 13,padding: 4,width:width-40}, {height: Math.max(40, this.state.height)}]}
defaultValue={this.state.value}
/>

@cgilson
Copy link

cgilson commented Jan 16, 2017

Getting same issue, but only on Android. Event is fired on mount, but isn't fired on text height change. Identical code works fine for iOS:

<TextInput
editable = {true}
multiline = {true}
[...]
onContentSizeChange={(event) => {
this.setState({height: event.nativeEvent.contentSize.height});
}}
style={{height: Math.max(35, this.state.height)}}
/>

@yasicmd
Copy link
Author

yasicmd commented Jan 23, 2017

onChange={(event) => {
this.setState({
height: event.nativeEvent.contentSize.height,
});
this.props.onModalHeightChange((Math.max(40, this.state.height)>140)? 140+205: Math.max(40, this.state.height)+205)
}}

this worked for me

@yasicmd
Copy link
Author

yasicmd commented Jan 23, 2017

instead of onContentSizeChange

@hramos
Copy link
Contributor

hramos commented May 25, 2017

Closing this issue because it has been inactive for a while. If you think it should still be opened let us know why.

@hramos hramos closed this as completed May 25, 2017
@shergin shergin self-assigned this May 25, 2017
@shergin
Copy link
Contributor

shergin commented May 25, 2017

Actually, I am working on this right now. I hope is will fix very soon.

@hramos hramos reopened this May 25, 2017
facebook-github-bot pushed a commit that referenced this issue May 26, 2017
Summary:
Previously <TextInput>'s onContentSizeChange event fires very rearly, usually just once after initial layout. This diff fixed that.
I also considered to a bunch of another things to get the native notification, but I found that overriding `onTextChanged` is the most reliable, easy and effitient way to implement this.

I tried/considered:
 * onLayout (does not fire)
 * OnPreDrawListener (fires to often)
 * OnGlobalLayoutListener (does not fire)
 * OnLayoutChangeListener (does not fire)
 * isLayoutRequested (too hacky)

(I also fixed the <AutoExpandingTextInput> demo to illustrate the fix.)

And just heads up, we will remove `contentSize` info from `onChange` event very soon.

GH issue: #11692

Reviewed By: achen1

Differential Revision: D5132589

fbshipit-source-id: e7edbd8dc5ae891a6f4a87b51d9450b8c6ce4a1e
@shergin
Copy link
Contributor

shergin commented May 26, 2017

Fixed! 3539352

@amanthegreatone
Copy link

amanthegreatone commented May 29, 2017

@shergin is this patched into a release version?

@kesha-antonov
Copy link
Contributor

kesha-antonov commented Jul 13, 2017

@shergin @hramos Tried 0.46-stable (0.46.2).
It works! Thanks!

sayar pushed a commit to microsoft/react-native-windows that referenced this issue Jul 26, 2017
Summary:
Previously <TextInput>'s onContentSizeChange event fires very rearly, usually just once after initial layout. This diff fixed that.
I also considered to a bunch of another things to get the native notification, but I found that overriding `onTextChanged` is the most reliable, easy and effitient way to implement this.

I tried/considered:
 * onLayout (does not fire)
 * OnPreDrawListener (fires to often)
 * OnGlobalLayoutListener (does not fire)
 * OnLayoutChangeListener (does not fire)
 * isLayoutRequested (too hacky)

(I also fixed the <AutoExpandingTextInput> demo to illustrate the fix.)

And just heads up, we will remove `contentSize` info from `onChange` event very soon.

GH issue: facebook/react-native#11692

Reviewed By: achen1

Differential Revision: D5132589

fbshipit-source-id: e7edbd8dc5ae891a6f4a87b51d9450b8c6ce4a1e
@ramsestom
Copy link

using react native 0.46.4 on android, I can't get contentSize from the event fired by onChange from TextInput...
the event.nativeEvent fired only has properties target, eventCount and text

@shergin
Copy link
Contributor

shergin commented Aug 11, 2017

@ramsestom That's expected behaviour. You have to use onContentSizeChange instead.

@ramsestom
Copy link

ramsestom commented Aug 11, 2017

onContentSizeChange is simply never fired (I change my input in my textinput field but the event is never fired even when the number of lines increase)
As for onChange not reporting contentSize in the fired event, I don't think it is the expected behaviour as it used to be the case in previous react-native release (and many libs and source codes depends on it). So looks like a regression

EDIT: actually onContentSizeChange fire correctly
EDIT2: when the text of the textinput is right aligned, onContentSizeChange actually works until 4-6 lines and then stops working after that. Everything is fine is the text is left aligned (the default).

@shergin
Copy link
Contributor

shergin commented Aug 11, 2017

@ramsestom Yes, it was breaking change. (But this feature was never documented.) And I made it... for goods. We had really good reasons for that. Sorry for the inconvenience!

Could you please create separate issue discussing inconsistent behaviour of onContentSizeChange?

@eli-isbell-h360
Copy link

Not sure if this has been moved to another issue yet, didn't see one anywhere...

@shergin
Testing on RN 0.47.1 (IOS), same use case as OP, I find onContentSizeChange is grabbing the initial height of the TextInput and continues to fire as text is entered, but never gets changes in height.

@shergin
Copy link
Contributor

shergin commented Aug 19, 2017

@eli-isbell-h360 Interesting, could you please create a snack?

@eli-isbell-h360
Copy link

@shergin Yessir, here it is: https://snack.expo.io/SJB1E3uOb

It looks like onContentSizeChange fails to read the correct height if the placeholder is changed by onFocus and/or onEndEditing. I was using these to clear the placeholder immediately onFocus instead of waiting for the value to change. It also seems like the height changes, but not correctly when a larger fontSize is used.

@shergin
Copy link
Contributor

shergin commented Aug 22, 2017

@eli-isbell-h360
Thank you!
Why do you and reapply contentSize as height? It is unnecessary. If you need autogrowing , simply do not set height explicitly; you also can limit height specifying maxHeight.
This is true for iOS for now, but right now I am working on same feature for Android.

Besides this I will investigate this issue.

@eli-isbell-h360
Copy link

@shergin
True! I believe I was attempting to control the height more directly because of unexpected behavior I was seeing with the custom fonts and fontSizes, but maxHeight with no explicit height is what I am using now.

Thanks for working on this!

@facebook facebook locked as resolved and limited conversation to collaborators May 29, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

9 participants