bootstrap
bootstrap copied to clipboard
Make a form validation handler | handle form messages
A Javascript handler for form|fields messages.
As a developer, I have been several times, in the awkward position, needing to implement custom code, that will co-operate with bs-form fields. So, many times I had the same thought. "What if bootstrap provided the base 'JS adapter' for its fields"
This PR tries to provide a Bootstrap component, that automatically handle browser validation messages, and in addition will give the opportunity to use custom, given feedback messages.
In advance, it contains JavaScript code, providing
- on form component:
- method to
clear
these messages, - get FormData` (for ajax usage)
- execute
validation
- access each formField instance
- JS hook, through component configuration, where developers can use for Ajax submission (not provided by default) and/or attach any custom validation messages
- method to
- per field methods:
-
clear
appended feedbacks - append
success | error | info
feedback messages
-
Needs:
- [ ] Documentation
- [ ] Tests
- Feedback
Cons: Adds a significant size as it introduces two new classes :thinking:
Related #28414 close #28995
Preview : https://deploy-preview-34043--twbs-bootstrap.netlify.app/docs/5.2/forms/validation/#custom-styles
almost perfect. one annoying issue though: if the form field already has an aria-describedby
to begin with (see for instance the "Username" control, which initially already references the element containing the "@" sign), then this is overwritten by the success or error message. instead, it should be concatenated (so that the aria-describedby
shows what was there to begin with, a space, and the id
of the error message) - see the server side static example later on. this may require storing the value of aria-describedby
(if present) before the validation happened, using this to generate the aria-describedby
when the validation message is shown, etc.
also, not sure if this PR also touches on the tooltip case, but here the generated markup seems incorrect ... the actual valid/invalid tooltip sits outside of the <div>
that is referenced by the aria-describedby
- so this references just an empty container, with the actual tooltip being a sibling of this.
After submitting with empty values, the username results in this structure
<div class="col-md-4">
<label for="validationCustomUsername" class="form-label">Username</label>
<div class="input-group has-validation">
<span class="input-group-text" id="inputGroupPrepend">@</span>
<input type="text" class="form-control" id="validationCustomUsername" aria-describedby="inputGroupPrepend" required="" data-bs-invalid="Please choose a username.">
<div class="field-feedback invalid-feedback" id="inputGroupPrepend">Please choose a username.</div>
</div>
</div>
note that this now gives both the <span>
with the "@" and the invalid feedback <div>
the same id
, which is invalid. and the end result is that the AT still only announces the latter.
what i meant above, which may not have been clear: the end result should be that the aria-describedby
contains a reference to both the previous/existing value it had (pointing to the <span>
) and to the validation <div>
, as a space-separated value. so the end result should be something like
<div class="col-md-4">
<label for="validationCustomUsername" class="form-label">Username</label>
<div class="input-group has-validation">
<span class="input-group-text" id="inputGroupPrepend">@</span>
<input type="text" class="form-control" id="validationCustomUsername" aria-describedby="inputGroupPrepend validationCustomUsername-formTip" required="" data-bs-invalid="Please choose a username.">
<div class="field-feedback invalid-feedback" id="validationCustomUsername-formTip">Please choose a username.</div>
</div>
</div>
also, the latest changes seem to have broken the tooltips case https://deploy-preview-34043--twbs-bootstrap.netlify.app/docs/5.0/forms/validation/#tooltips which now just show the browser default error handling rather than the custom tooltips
btw probably worth saying: it might look like i just come in here to point out what's wrong... but this is already a great way forward! if we can tweak these last few bits, it'll be wonderful to have validation properly addressed in BS.
btw probably worth saying: it might look like i just come in here to point out what's wrong... but this is already a great way forward! if we can tweak these last few bits, it'll be wonderful to have validation properly addressed in BS.
You are right on this. give me some minutes
note that this now gives both the with the "@" and the invalid feedback
the same id, which is invalid. and the end result is that the AT still only announces the latter.got it. Thanks
it's almost perfect now, thanks for indulging me there @GeoSot. the last aspect that would be good to address is potentially tricky.
if you try to submit an invalid/incomplete form, and you get the error messages...if you then go back and correct the errors, visually the error message disappears and the green success styling is shown. however, behind the scenes, the aria-describedby
still points to the now hidden error message. and even if an error message is completely hidden using display:none
, screen readers will still read/announce it when it's referenced by aria-describedby
. this can then lead to confusing situation where the user corrects the form entry, it visually shows it's correct, but regardless it still announces the error.
i think the only way around this would be to watch for a change
event on the form (or individual form controls themselves?), and then dynamically resetting aria-describedby
to its initial value (e.g. in the case of "Username" only pointing to the @
container) / changing it to the success message id
if present (and in the case of "Username", this would then need to point to BOTH the @
and the success message if it had one ... for this reason, will be good to keep a reference right at the start to whatever the aria-describedby
value was before validation as well.)
i.e. after validation once the user does go back and edit, currently it visually then shows this
and the markup for that username input is
<input type="text" class="form-control" id="validationCustomUsername" aria-describedby="inputGroupPrepend validationCustomUsername-formTip" required="" data-bs-invalid="Please choose a username.">
i.e. note that aria-describedby
now still points to inputGroupPrepend validationCustomUsername-formTip
when it should just point to inputGroupPrepend
last outstanding aspect, if possible, is removing the reference to the error message in aria-describedby
once a previously invalid field becomes valid (either right away when the user fulfills the validation requirement, or at least when they try to resubmit and the validation run happens again).
i.e. currently, even after filling in a previously empty field, and the field now not being valid, the error message is still associated/announced ... and with aria-describedby
, even if the target referenced is display:none
, it's still announced. see this animation (with NVDA speech viewer), and note that after filling in the "City" and then moving away and returning, it still announces as "City, edit, required, please provide a valid city".
getting there :)
i see the aria-describedby
is correctly updated if at first something was invalid, and you then go in to make it valid. great stuff.
however, doing multiple erroneous submissions results in error messages being multiplied...
in the case of the tooltip variant, visually there's only one tooltip, but the aria-describedby
balloons out with multiple references...as a result, AT will announce the referenced tooltip multiple times too
lastly, in the "supported elements" case, the initial errors aren't associated with the form fields. and editing the form to then be valid, some of the errors remain visible (but still not announced) https://deploy-preview-34043--twbs-bootstrap.netlify.app/docs/5.0/forms/validation/#supported-elements
thank you @patrickhlauke :)
I was aware of duplicated feedback elements, but I wasn't sure how to solve it. Although, I think I have found a valid solution.
lastly, in the "supported elements" case
I didn't change something there (at least yet), as it is a fixed example that shows the classes usage. (it works the same way on our published docs)
In order to do our checks, the only changed sections are
- Forms:
- Custom styles
- Tooltips
- Examples:
- Checkout
- Checkout RTL
When we finish the basic tests, we can see more on documentation area, and js-code needs to be discussed|optimized with the help of the js team
I was aware of duplicated feedback elements, but I wasn't sure how to solve it. Although, I think I have found a valid solution.
conceptually, it likely needs to do a check of "if the relevant error is already shown/the aria-describedby
already contains the reference, do nothing, otherwise show the error/add the reference to the aria-describedby
".
I didn't change something there (at least yet), as it is a fixed example that shows the classes usage. (it works the same way on our published docs)
we probably need to make those dynamic as well (starting off with an already present error, but then following the same JS logic that removes any reference/hides the error message/etc once user interacts with it)
added a commit that normalises all id
s used in the various form examples, and adds the relevant aria-describedby
etc to the static examples, as a first step
i see the issue of multiple errors appearing when hitting submit multiple times has now been fixed. great stuff.
out of interest, how easy/hard would it be to hook up the validation script to https://deploy-preview-34043--twbs-bootstrap.netlify.app/docs/5.0/forms/validation/#supported-elements as well? as currently the CSS changes kick in (partially) and it leaves this in weird semi-states...
one other oddity i spotted (but likely no easy way around it) is that for the checkbox example, the generated error comes straight after the input, so ends up before the label
conceptually, it likely needs to do a check of "if the relevant error is already shown/the aria-describedby already contains the reference, do nothing, otherwise show the error/add the reference to the aria-describedby".
done
we probably need to make those dynamic as well (starting off with an already present error, but then following the same JS logic that removes any reference/hides the error message/etc once user interacts with it)
I think it will be a bit more interventional, than it should
out of interest, how easy/hard would it be to hook up the validation script to https://deploy-preview-34043--twbs-bootstrap.netlify.app/docs/5.0/forms/validation/#supported-elements as well? as currently the CSS changes kick in (partially) and it leaves this in weird semi-states...
It can be done, but it is considerable better to make .is-valid
& is-invalid
to have more weight than pseudo-classes, as they are used as overrides
one other oddity i spotted (but likely no easy way around it) is that for the checkbox example, the generated error comes straight after the input, so ends up before the label
done
It can be done, but it is considerable better to make
.is-valid
&is-invalid
to have more weight than pseudo-classes, as they are used as overrides
but without JS to remove the aria-describedby
the error messages will still be announced even when display:none
'd ... which is what led us down this path of using JS in the first place
Just for my own sanity (as I had lost track of where we were at with this), made a quick recording using Chrome/NVDA of how this all currently shakes out
https://user-images.githubusercontent.com/895831/169644477-cf5fc958-e5d9-4b62-8755-8b17e37b0efd.mp4
- 0:00 - 0:42 Custom styles: work great/as I'd expect/want them
- 0:42 - 1:27 Browser styles: work as expected by browser (with Chrome popping up the validation bubbles one at a time as you submit)
- 1:27 - 1:59 Server side: notice that only the errors are tied to the form fields. for consistency, probably makes sense to also tie the "Looks good" text to the form control, the same way that error messages are
- 1:59 - 2:18 Supported elements: no association between validation messages and their form fields - wonder if we can either automate this/run the script over it, or if we just manually add the
aria-describedby
here? - 2:18 - 3:00 Tooltips: work as expected
@patrickhlauke for start, I want to really thank you for your patience and your help.
I saw the review above and :
1:27 - 1:59 Server side: notice that only the errors are tied to the form fields. for consistency, probably makes sense to also tie the "Looks good" text to the form control, the same way that error messages are
seems to be an issue of docs, because we are just indicating the proper markup in case of server validation, fixed in: 07240fedb72eadf739f94f6df59ee40081a3329d
1:59 - 2:18 Supported elements: no association between validation messages and their form fields - wonder if we can either automate this/run the script over it, or if we just manually add the aria-describedby here?
seems to be an issue of docs, which I handled, adding the aria-describedby
attribute manually (we cannot use the js script as the submit btn is disabled ), fixed in: https://github.com/twbs/bootstrap/pull/34043/commits/f9041ccd583576f38714da830d47a188f7e251a1
snuck in a tweak to the documentation text itself...
@GeoSot you'll probably also want to add a bullet point to the "how it works" section, to mention the extra scripting that ties the form control to its feedback/error message when it appears. just draft it up, happy to give it a read/tweak once there https://deploy-preview-34043--twbs-bootstrap.netlify.app/docs/5.2/forms/validation/#how-it-works
Thank you @patrickhlauke 🙂
I have it in my mind too, but I am holding myself, till I get some feedback from other @twbs/team members (or contributors), concerning the usefulness of this idea, and opinions over the JavaScript code. (Just to minimize the process steps and avoid going back and forth on file changing)