Skip to content

netteForms: alert() replaced with <span class="nette-form-error">#1126

Closed
dg wants to merge 2 commits into
masterfrom
nette-forms
Closed

netteForms: alert() replaced with <span class="nette-form-error">#1126
dg wants to merge 2 commits into
masterfrom
nette-forms

Conversation

@dg

@dg dg commented May 31, 2013

Copy link
Copy Markdown
Member

No description provided.

@Majkl578

Copy link
Copy Markdown
Contributor

👎
Even though I'm not fan of alerts, I don't like this even more. I think current behavior is pragmatic and should stay as is - not affecting DOM in any way. If one ones to change it, there is no problem with that (we already have few live validators etc.).

@fprochazka

Copy link
Copy Markdown
Contributor

I like it. It should be as an alternative, that you can turn on by something like

Nette.addError = Nette.showFormErrors;

This is the intended behaviour, right?


Oh, where went addError ?

@enumag

enumag commented May 31, 2013

Copy link
Copy Markdown
Contributor

IMO the JS validation should be triggered after change / blur of any field in the form (not just when the user tries to submit the form) and display the errors in the same places as server-side validation - that is next to the inputs. I've already implemented it (using @hosiplan's twitter bootstrap renderer) and it is very convenient for the user.
EDIT: You can find my implementation here: https://gist.github.com/enumag/5687497.
I'm not sure if such behaviour should be default but I like it.

@smasty

smasty commented May 31, 2013

Copy link
Copy Markdown
Contributor

How about using setCustomValidity on the elements if supported by the browser?

@dg

dg commented May 31, 2013

Copy link
Copy Markdown
Member Author

@enumag now you can use default Nette.validateForm.

@dg

dg commented May 4, 2014

Copy link
Copy Markdown
Member Author

Moved to nette/forms#9

@dg dg closed this May 4, 2014
@dg dg deleted the nette-forms branch May 4, 2014 18:25
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.

5 participants