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

fixes textarea not changing when value changes to null (issue #2533) #3041

Closed
wants to merge 1 commit into from

Conversation

martinstein
Copy link

This is a fix for issue #2533.

I've just signed the CLA, too.

@syranide
Copy link
Contributor

syranide commented Feb 5, 2015

That doesn't actually work for valueLink I think?

@syranide
Copy link
Contributor

syranide commented Feb 5, 2015

It should also reset to defaultValue.

@martinstein
Copy link
Author

Can't test right now. I'll have to check that later...

Regarding your 2nd point with the defaultValue: I don't think it should reset to that. How would you clear the textarea then - in the presence of a defaultValue? Then it would be impossible to clear the textarea with a value of null, because each time that you want to clear it, it would reset to defaultValue instead.

@syranide
Copy link
Contributor

syranide commented Feb 5, 2015

@martinstein The only way to clear uncontrolled inputs is to physically set .value, that's just how they work and this wouldn't change it. When going from uncontrolled to controlled it should either keep or reset the value, anything else is arbitrary if you ask me... and keeping the value is out of picture. cc @zpao

@martinstein
Copy link
Author

I didn't quite understand what you mean there, sorry. I think we are talking about 2 different use cases. My case, where this issue trips me up is this:

  • I have a controlled <textarea> which starts out with a user's description. I.e. the value is set to that description.
  • Now the user changes to a different one, whose description is null. So, of course, the textarea value should be cleared / set to empty. Because of this issue, the old user's description stays, which is a bug.

Of course, after the user has changed, the textarea is still/should stay controlled even though the value is now null. After all, it's still the same form, only the value has changed from "something" to null.

Maybe that helps to clarify. Could you explain your case?

@syranide
Copy link
Contributor

syranide commented Feb 5, 2015

•Now the user changes to a different one, whose description is null . So, of course, the textarea value should be cleared / set to empty. Because of this issue, the old user's description stays, which is a bug.

Yes, so defaultValue should be an empty string.

Of course, after the user has changed, the textarea is still/should stay controlled even though the value is now null . After all, it's still the same form, only the value has changed from "something" to null .

Huh? If value == null then it is by definition uncontrolled?

PS. It seems like you're thinking about this from a use-case perspective, that's something you should implement as a custom component, wrapping an input. This has to make sense from a technical perspective in React, where all you know is that you're going from one state to another, but more than that, the previous state is irrelevant. Rendering with the same props, at any time, should present the same output except for uncontrolled inputs, but rendering an uncontrolled input should either set it to defaultValue (because there was no previous sate) or do nothing at all.

@martinstein
Copy link
Author

Okay, I get your first point. With an empty defaultValue that works.

Yeah, by definition you are right. I find React's definition in this regard somewhat unfortunate, similar to the thoughs of @rymohr here:
#2533 (comment)

In my case, the way the form is used is still the same, even after the null update: As soon as the end-user types in 1 character into the textarea, it becomes controlled again because the change listener updates the value (as I understand, that's the normal way it's done for controlled components).

That's why I find React`s definition a little bit weird. It is still the same textarea, with identical behavior in all cases (a controlled update of the value after some changes), but if that value is null, we now call it "uncontrolled".

But anyway, your suggestion with defaultValue is fine with me and I can update the pull request later.

@martinstein
Copy link
Author

@syranide You were right. My (old) pull request didn't support valueLink.

I've updated the pull request. Now it works for valueLink, too and resets to defaultValue if that is not null. So, it should be good now.

stub.replaceProps({value: null, onChange: emptyFunction});
expect(node.value).toEqual('');
});

Copy link
Member

Choose a reason for hiding this comment

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

We need some more tests here. What's the expected behavior when there is a defaultValue? What if defaultValue also changes? Should null and undefined behave differently?

I commented in the issue for a bit more discussion before we make a final decision.

@iam4x
Copy link

iam4x commented Jun 8, 2015

Is this will be merged? Because <input> will changes when the value null, this will apply the same behaviour on the inputs.

@syranide
Copy link
Contributor

syranide commented Jun 9, 2015

@iam4x Just because it resets doesn't mean it's a good idea to rely on this behavior. null switches controls into becoming uncontrolled, which is not what you really want.

@jimfb
Copy link
Contributor

jimfb commented Nov 10, 2015

Superseded by #5013

@jimfb jimfb closed this Nov 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants