-
-
Notifications
You must be signed in to change notification settings - Fork 234
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
Forms: Default values behaviour with disabled fields #961
Comments
Can't you use setValue() on disabled fields instead? |
Sure but I can't use just one setDefaults call, I need to use setValue(s) separately. Appareantly nobody cares. Closing. |
Don't close it, I believe it's valid request. |
Not sure what it'd be good for, but at least it makes sense. |
@Majkl578 the use case is in description. And I agree. |
@enumag Oh, and by the way, 14 days of nobody responding means nobody has time to think about it/implement it. If we wouldn't like it, you would hears us wery clearly. |
OK, reopening. :-D |
Wouldn't be better solution implementing read-only functionality rather than changing behaviour of forms? Also, at this point, when you use setDefault() you know that user can change the value, when you use setValue() he can't. If you implement the disabled functionality it wouldn't be so straightforward. |
Well, both sides have some pros and cons. enumag
Elijen
If I should choose, I would change it as @enumag suggest. |
In what cases are you using disabled controls? |
I use it in form for editation of a user. User himself can't change his username but admin can. I could remove the field entirely but I like disabled field more in this case. |
Has been reverted (70cd9d4) it causes some problems http://forum.nette.org/cs/14337-neocekavane-chovani-setdefaults#p103106 |
In my opinion BaseControl::setDefaultValue() and Container::setDefaults should set value to disabled field even if the form was submitted. The disabled field is empty after error with current behaviour.
I'd like to hear your opinions first. I'll prepare PR if this idea gets some support.
The text was updated successfully, but these errors were encountered: