- Issue created by @mdolnik
- 🇨🇦Canada mdolnik
Added a patch to resolve this issue.
In summary this adds two parts to add validation:
1. The signature of
ElementBase::prepareState()
has been updated to accept an array of$states
instead of a single string:- This ensures that callers of this method can now provide not just the
visible
state, butvisible,required
states if the field is required.
- This causes the Other text element to be required via JS validation whenever its visible AND required.2. A new
#element_validate
callback has been addedElementBase::validateOther()
- This ensures that if JS Validation is disabled (or somehow bypassed) that the validation will be performed server-side.
I have manually tested this on:
- Single-value select lists
- Single-value buttons (radios)
- Multi-value buttons (checkboxes)With all of the above testing with JS Validation enabled/disabled.
One caveat is if JS Validation is disabled, the logic in
ElementBase::validateOther()
does not work for Radios. This is related to and caused by the issue that #3171843 🐛 Multiple-value checkboxes do not render existing values Active is trying to fix and its #6 patch will resolve this. (Radios will still successfully work with the JS Validation with/without #3171843 being resolved) - 🇨🇦Canada mdolnik
This is the same patch file as above, but can be applied to version 4.0.0
- Status changed to Needs review
over 1 year ago 7:07pm 24 April 2023 - last update
over 1 year ago 12 pass, 4 fail The last submitted patch, 2: add_support_to_require-3356113-2.patch, failed testing. View results →
- last update
over 1 year ago 21 pass - 🇨🇦Canada mdolnik
Added a new patch which removes the changes to
ElementBase::prepareState()
.While this helped resolve the issues with the logic in
ElementBase::validateOther()
for Radios, it ended up complicating things when the fields as a whole are required via states by outside logic.Removing the modified states code also prevents the need to update relevant tests.
Since the issue with
ElementBase::validateOther()
for Radios is caused by the issue that #3171843 is trying to fix, it should be considered to be a requirement for this task to be completed.