This patch changes the captcha persistence widget from a check box to radio buttons, which should make it easier to understand that setting.
I'll add a screenshot in follow up.
Concerning source code, this change will also make it easier to add additional types of captcha persistence in the future, if needed.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | persistence_4.patch | 5.37 KB | soxofaan |
| #10 | persistence_3.patch | 4.23 KB | soxofaan |
| #8 | persistence_2.patch | 3.04 KB | soxofaan |
| #1 | persistence.png | 34.43 KB | soxofaan |
| persistence.patch | 2.67 KB | soxofaan |
Comments
Comment #1
soxofaan commentedscreenshot of what patch does
Comment #2
robloachI think I like the checkbox design more. Radio buttons are usually used when there are more than two options available. Although I could be wrong here as I'm no designer.
Comment #3
soxofaan commentedFrom the viewpoint of the programmer a checkbox is indeed the natural choice. But with radio buttons you can better present the options to the administrator.
In the checkbox design the whole explanation is in the description string, which has to be read carefully. Without that description, it is not obvious what the option does.
In the radio buttons design the options are directly related to the widget elements, which is easier to parse. There is no description needed anymore.
Another advantage is that it is easier to add an option like "Always add a captcha, except after preview" in the future.
Comment #4
soxofaan commentedSee http://drupal.org/node/169758 for a usage scenario where the current checkbox design fails to communicate its meaning clearly.
Comment #5
robloachI really don't mind what design is chosen. It's completely up to you guys and wundo. The thing I do mind is all those ugly defines in the code ;-) .
Comment #6
dman commentedOK, seeing that it's not just a case of my own confusion/preference , I'll say "why not". I've found that radios are appropriate for slightly complex cases, so I'l vote for this.
+1
... But yes, the DEFINEs are maybe overkill ;-)
.dan
Comment #7
soxofaan commentedBut what is the alternative? Hardcoded values are error prone and bad coding style.
Moreover, I don't think the defines are ugly, they make the code more readable. "CAPTCHA_PERSISTENCE_SKIP_ONCE_SUCCESSFUL" is has a more obvious meaning than "1".
Comment #8
soxofaan commentedupdated patch:
against captcha.module version 1.42.2.27
use of "CAPTCHA" instead of "captcha"
Comment #9
dman commentedOK, yeah.
I had nothing serious about the DEFINEs - just a big fix for a small problem. I agree they are sorta-best-practice.
Just do it.
Comment #10
soxofaan commentedNew version of patch, which adds an option to make persistence global (not per form) as suggested by http://drupal.org/node/175912
the persistence widget now looks like:
Comment #11
robloachI like the look of this one now.
Comment #12
soxofaan commentedNew version of patch with addition of solution for the cache+persistence bug (as suggested in http://drupal.org/node/175909#comment-306165)
Comment #13
robloachExcellent.
Comment #14
(not verified) commented