Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I have a boolean variable set up in Rules, and I set up a "Data Comparison" action in the Rules UI, selecting this variable.
When I got to the step where I was supposed to enter the value to compare, it gave me a checkbox saying "Data Value":
I found that very confusing... A help/description message saying "Check for TRUE, and uncheck for FALSE" or something like that would have helped, or better yet instead of a checkbox, make it a select or radios with values TRUE and FALSE. The single on-off checkbox with label "Data value" just didn't make sense. Keep in mind that in a "data comparison" action I have to choose a variable/value to compare with another variable/value, so checking a box doesn't seem like "entering a value" to me.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 2014065-boolean-value.png | 8.3 KB | heddn |
| #15 | 2014065-boolean-force-redirect.png | 11.35 KB | heddn |
| #13 | d7_rules.patch | 792 bytes | fago |
| #11 | rules-ui-very-confusing-2014065-11.patch | 1.24 KB | heddn |
| #5 | rules-UI-BooleanValues-2014065.png | 7.54 KB | heddn |
Comments
Comment #1
jhodgdonThe UI is similarly confusing for Set a Data Value for Booleans, where it also has a checkbox with label "Value".
Comment #2
heddnI agree that this is very confusing. It was only after finding this issue that I was able to figure out how to use this type of data comparison.
Comment #3
fagoConverting to radios sounds reasonable. Anyone cares to roll a patch?
Comment #4
heddnHere you go.
Comment #5
heddnHere's a screenshot.
Comment #6
jhodgdonThat looks like probably the right fix... guess it probably needs manual testing though.
Does this patch fix Booleans wherever they appear, or does there need to be a separate fix depending on if it's being used in a Data Comparison or a Set a Data Value?
Comment #7
heddnIn response to: #6, it fixes both conditional and set data value.
Tagging appropriately.
Comment #8
ditcheva CreditAttribution: ditcheva commented+1 for this - I agree the UI is very confusing. Just tested the patch in #4 and it works well for me! Maybe with another user test, we can switch to 'reviewed and tested'??
Comment #9
jhodgdonLooks like we have two manual tests and I reviewed the code too, so let's at least provisionally set to RTBC.
Comment #10
fagoI see that this is confusing in that situation, but the same UI will be used to generate other checkboxes as well, e.g. check the page redirect action with the patch applied. I doubt it makes a good UX there?
So, we
a) need to find a solution that works for both cases or
b) we leave the current way as default and implement an alternate way that's used to input booleans for the generic actions/conditions only. I doubt this would be very simple though, as we'd have to some checking to add the UI customizations only for booleans params... :-/
Maybe, you've got some good ideas for a) though?
Comment #11
heddnAfter looking at the page redirect action I see what you mean. I've revised the wording a little better now. This wording makes sense to me. Anyone else?
Comment #12
fagohm. I tried to improve it a bit - see attached, but I think it's still a too big pita for the page-redirect case.
What about going with radios and labels "Enabled" / "Disabled" ?
Comment #13
fagoComment #14
fagothat's not so simple, actually ;-)
Comment #15
heddnComment #16
heddnCross-post on the screenshots. They are related to #11
Comment #17
bserem CreditAttribution: bserem commentedThis really needs to be pushed to Rules... I was fiddling with the data comparison for quite some time and it really doesn't make sense the way it is being displayed by default.
Comment #18
ditcheva CreditAttribution: ditcheva commentedI like the approach in #11. That makes the most sense to me!
Comment #19
SocialNicheGuru CreditAttribution: SocialNicheGuru commented#13 works for me.
Comment #20
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedis it ok to show rtbc if you are not a maintainer. Just changing status so that it will trigger in the maintainer queue.
Comment #21
fagook, I've committed #13.
For the regular configuration checkbox imo it would make sense to provide a separate "checkbox" ui class, which one could specify for those parameters. I decided to commit this now anyway as it avoids further confusion with the data comparison now. The checkbox ui class can be provided in a follow-up, please feel free to open an issue + ping me for guidance if you are interested on working on that.