Skip to content

Conversation

sandello-alkr
Copy link
Contributor

Solves #200.
TODO: error bubbling.

Solves braincrafted#200.
TODO: error bubbling.
@florianeckerstorfer
Copy link
Member

Thanks @sandello-alkr. I didn't knew that. Do you want to add error bubbling before I merge?

@sandello-alkr
Copy link
Contributor Author

Yes, I'll watch into this little later. I don't like recursion in template, so I need some time for research about nested forms behavior.

@florianeckerstorfer
Copy link
Member

@sandello-alkr Anything new on this PR? Should I merge it in its current form and we add error bubbling later?

@sandello-alkr
Copy link
Contributor Author

I'm not sure right now if error bubbling is something good at all. It makes me crazy when I can't locate field with error, cause of whole form is "red". Maybe we should totally remove error bubbling?

@sandello-alkr
Copy link
Contributor Author

Anyway, thx for this remind, I'v totally forgot about this (-_-")

@florianeckerstorfer
Copy link
Member

There are some instances when it is useful. For example, in a login form where you don't want to say "username is wrong" or "password is wrong" but rather say "username/password combination is wrong". In that case the whole form should be marked as invalid.

@sandello-alkr
Copy link
Contributor Author

I've added has-error class to form tag, so now if you need error bubbling for the whole form, you can use error_bubbling option. That would push error from specific input, to the form.
If you need to highlight virtual form, I suggest use custom validator, adding error to needed form child.
Also you need to be careful with collections, cause it have "error_bubbling" option true by default.

florianeckerstorfer pushed a commit that referenced this pull request May 29, 2014
@florianeckerstorfer florianeckerstorfer merged commit 93d8d42 into braincrafted:develop May 29, 2014
@florianeckerstorfer
Copy link
Member

Thanks.

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.

2 participants