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

Fully controlled form elements #188

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

CyberAP
Copy link
Contributor

@CyberAP CyberAP commented Jul 9, 2020

Change v-model behaviour to be in full control over the displayed value.

Rendered RFC

@jods4
Copy link

jods4 commented Jul 9, 2020

I am a bit confused by this RFC.

The basic example says:

Currently Vue will render anything that user enters in the even though state value is different from what the user actually sees.
After the change user only sees the first character, which perfectly corresponds to the actual state.

I find that untrue, in the following repro only the first char is displayed:
https://codesandbox.io/s/v-model-decomposed-camel-case-84732

The RFC talks about $forceUpdate but never mentions the fact that whenever the v-model expression triggers (in the reactivity sense) the input will be updated.

In particular, this includes when v-model updates the source and this is why the sandbox above only displays a single character:

  • User types
  • Event input is handled by v-model
  • Value is written back into source expression
  • Ref triggers
  • input is updated, since state value is different from current value

@CyberAP
Copy link
Contributor Author

CyberAP commented Jul 9, 2020

I find that untrue, in the following repro only the first char is displayed:
https://codesandbox.io/s/v-model-decomposed-camel-case-84732

Thanks for pointing that out, this shows that current behaviour is inconsistent. For setup it works, for options it does not. But the behaviour should be the same both for setup and options.

@jods4
Copy link

jods4 commented Jul 9, 2020

I see, I have never used the options API.
I had the impression that in Vue 3 the Options API was re-implemented on top of reactivity, which intuitively would mean that they behave the same. If they don't I agree the behaviour should be consistent and predictable.

Maybe the RFC should be more clear about how v-model works differently in each API and how it's gonna unify them. Currently there's no mention of either. All examples are based on Options API.

From my first read I got the impression that it wants to change the way v-model works -- or add alternative ways/mode (e.g. .controlled).
I don't quite understand how modifying v-model that already works for Composition will fix Options: shouldn't the underlying layer be unified first?

Not trying to challenge the RFC but as a Composition API user I don't quite get it.

@CyberAP
Copy link
Contributor Author

CyberAP commented Jul 9, 2020

Yes, this is worth mentioning in RFC, I'll update it later accordingly. I didn't mention it beforehand because I was unaware that setup does not have this problem.

@ByScripts
Copy link

I find that untrue, in the following repro only the first char is displayed:
https://codesandbox.io/s/v-model-decomposed-camel-case-84732

Thanks for pointing that out, this shows that current behaviour is inconsistent. For setup it works, for options it does not. But the behaviour should be the same both for setup and options.

Is it expected than even for setup(), the behavior is different for get/set and computed ?

https://codesandbox.io/s/v-model-decomposed-camel-case-5ftt7

@CyberAP
Copy link
Contributor Author

CyberAP commented Jul 20, 2020

Is it expected than even for setup(), the behavior is different for get/set and computed ?

https://codesandbox.io/s/v-model-decomposed-camel-case-5ftt7

It is not, thanks for adding that! I'll update RFC later to reflect this inconsistency as well.

@CyberAP
Copy link
Contributor Author

CyberAP commented Sep 10, 2020

Added a few more examples:

Also expanded Composition API section with current inconsistencies.

@jods4
Copy link

jods4 commented Nov 3, 2020

I don't think the "fix readonly on checkboxes" motivation is good.

Is the code example even working?
It doesn't propagate the update event, sure, but the UI would be left in an inconsistent state.
In this simple fiddle, the model doesn't change but you can see the checkbox state still changes when you click it:
https://jsfiddle.net/tkz305s8/

In this case I think there are cleaner way to "fix HTML".
If you're writing a v-checkbox component, you could fix the problem at the HTML level by cancelling the event:

<template>
  <input type=checkbox @click="e => { if (readonly) e.preventDefault() }">
</template>

What this does is clear and works at lowest level.
(Fun fact: despite its name click event is called for every checkbox change, including if the change is trigger by keyboard).

(If you can, a better option is to use disabled but it has minor differences to readonly)

@CarterLi
Copy link

CarterLi commented Jun 9, 2022

A form element should always be controlled because that's how binding works.

<input type="text" value="TEXT" />

A developer binds property value of element input to string TEXT. In this case, property value of element input should always be TEXT, which makes the input box behave like readonly.

<input type="text" :value.attr="'TEXT'" />

A developer binds attribute value of element input to string TEXT. In this case, attribute value of element input should always be TEXT. However, attribute value is the initial value of an input element. In this case, users can freely change text in the input box.

That's how binding works, and so called controlled. This behavior should be constant, and should not change depending on what elements it applies.

@tve
Copy link

tve commented Nov 29, 2022

I'm trying to understand the status of this RFC... In the top-level readme it states

Open a new thread in Discussions and make sure to set category to "RFC Discussions"

Is there such a discussion thread?
Sorry for the dumb question...

@leopiccionia
Copy link

I'm trying to understand the status of this RFC...

This PR was created before the implementation of GitHub discussions. As far as I know, there's no related Discussion thread, but one could be opened.

@mahnunchik
Copy link

This would be a useful improvement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants