Conditionally required fields are not enforced at save

[allsite]

This issue dates back many years in d7 and it would be a fantastic improvement for Backdrop if form validation worked properly in conjunction with conditional fields.

Steps to reproduce:

1. add a boolean field
2. add a short text field
3. add a condition to set the short text field as required when the boolean field is set to true
4. Save the entity

This should throw a form error instead of saving, yet the entity is saved.

[laryn]

Thanks @allsite! Questions for you:

1. Is this the respective issue in the D7 queue? https://www.drupal.org/project/conditional_fields/issues/1561272
2. That issue has a patch that is RTBC. Would you be able to port that patch over here as a PR?

I'd be happy to review and get this merged! (And if you aren't comfortable making a PR I will get to it eventually).

[allsite]

@laryn I appreciate your attention on this!

1. Yes it is. I tested patch #209 from the link above and it resolves the issue in Drupal 7.82.
2. I'm new here and haven't yet made a PR. My current focus is outlining bugs/limitations and making sure they're at least in a queue.

[EDIT: THIS TURNED OUT TO BE A NON-ISSUE AFTER INSTALLING JQUERY_UPDATE]
An issue the patch fails to address, is while using Bootstrap theme, the "required" asterisk is not displayed after the field label, when required by a condition. The following snippet should be added inside the tags, which works in Seven, Skeletontheme but not Bootstrap. I can confirm no changes to boostrap's markup happen when a field is set to be required by a condition, making a css fix impossible.

<span class="form-required" title="This field is required.">*</span>

This is a major UX issue with 155k sites reporting using Bootstrap compared to 5k using Skeletontheme. If we have to develop custom code to add the asterisk that renders the patch almost useless in terms of developer time and how much is wasted reinventing all the conditions in js or php.

In summary, I believe resolving these 9-year-running issues would bring the module to 100%. Use cases include anything from Call for Proposals to POS systems. Great polish for Backdrop!

[allsite]

[UPDATE]
The above bootstrap issue was resolved by installing jquery_update. Silly me! So it's a task of simply porting over the patched module. Thank you!

[laryn]

I'm new here and haven't yet made a PR

@allsite No worries -- and if you're comfortable testing patches it wouldn't take long to learn how to make a PR. (But I agree, getting it in an issue queue is a great first step).

[laryn]

Closing per the comment above:

[EDIT: THIS TURNED OUT TO BE A NON-ISSUE...

Feel free to reopen if it actually remains an issue.

[yorkshire-pudding]

@laryn I don't think this is a non-issue. I was having the same issue. I have applied the patch and it fixes if you use the value comparison. It didn't fix if I used the status of checked, but this is the latest patch in that ticket. I'll attach it in a PR shortly

[laryn]

Thanks @yorkshire-pudding -- I took a quick peek and left a comment on the PR.

@allsite when this is ready are you interested in testing the code?

[laryn]

@yorkshire-pudding I made a commit that removed the extra bits that somehow snuck in again, and changed one instance of drupal_* to backdrop_*. I also made one other tweak to see if we could get the "checked/unchecked" condition working in addition to the "value" condition. I did an initial test and it seems to work as advertised!

@allsite Let me know if you think you'll have a chance to test the PR or not. @yorkshire-pudding wouldn't hurt if you'd test the latest version of your PR code as well since I made a few changes.

[yorkshire-pudding]

Thanks @laryn - I never checked for Drupal as the port was already done.
I've tested again with both values and checked and they both work as advertised. I've no idea why git keeps taking that bit out. I double checked it was there before re-committing.

[laryn]

@yorkshire-pudding No worries -- but for clarity the drupal_* piece I meant was in the patch that you were porting over here. One of the lines called drupal_array_get_nested_value which simply had to be changed to backdrop_array_get_nested_value.

Thanks for bringing this patch over. As far as git, it can be a little confusing when you are first starting out. I noticed you submitted the PR from the main branch of your fork. I would recommend playing with branches so you don't have to do everything in the main/same branch and can make specific changes that are only applied in one branch until merged into the main branch.