Validation #321
Validation #321
Conversation
|
|
||
| function checkTextFields (errors) { | ||
| $(document).find('input[type="text"], textarea').each(function () { | ||
| var $fieldset = $(this).parents('fieldset') |
There was a problem hiding this comment.
I think this should be closest instead of parents. Closest stops at the closest, parents looks at all parents for any possible matches.
There was a problem hiding this comment.
good shout @joelanman there is a lot of parents() in there that could be changed
| function checkTextFields (errors) { | ||
| $(document).find('input[type="text"], textarea').each(function () { | ||
| var $fieldset = $(this).parents('fieldset') | ||
| var label = $(this).parent().find('label').clone().children().remove().end().text() |
There was a problem hiding this comment.
this might be easier to read and understand if it was split into two or three operations, rather than one long chain
|
|
||
| function checkTextFields (errors) { | ||
| $(document).find('input[type="text"], textarea').each(function () { | ||
| var $fieldset = $(this).parents('fieldset') |
There was a problem hiding this comment.
you use $(this) multiple times in this block, might be better to do var $this = $(this) once at the start
|
Just wondering whether we can remove the requirement to wrap elements in fieldset? It adds a lot of extra code and I'm not sure it's semantically correct to wrap single elements. And to confirm - this just covers required - no other validation? |
|
I think we should try and use the markup from elements: https://govuk-elements.herokuapp.com/errors/#highlight-errors happy to have a bash at that |
|
Yeah, I originally tried to do that, but then it became more confusing in the documentation telling people where to put the data-require attribute. It seemed easier for the sake of prototyping (whilst not necessarily semantically correct) to add it to a wrapper, rather than saying 'if its a text field, put it on the input, if its radio buttons put it on the fieldset, if there are more than two fields in a group then add it somewhere else, if its a text area add it somewhere else etc. It might be the best approach, but consistency seemed easier to explain. |
|
Happy to work with you to implement it that way though. Would probably make more sense to just work a bit harder on the documentation! |
|
ah of course - you need something for radios, we could just use |
Added basic validation for prototypes.
In this PR, I have set up a simple way to check for required inputs that are either blank or not selected.