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.The #states 'required' action doesn't actually make the form element required. It *does* change the appearance of the form element, and make it *look* like it's required. But you can still submit with the element empty.
So for example with the code below, if you click the checkbox to make the field required, you can still submit the form with the texfield empty. A complete example module is attached. It demonstrates two other bugs, so use the 'Bug Demo: #states making element required' menu entry.
$form['make_required'] = array(
'#type' => 'checkbox',
'#title' => t('Make textfield required'),
);
$form['a_textfield'] = array(
'#type' => 'textfield',
'#title' => t('A textfield'),
'#description' => t('The "make_required" checkbox should be able to make this a required field'),
'#states' => array(
'required' => array(
':input[name=make_required]' => array('checked' => TRUE),
),
),
);
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | drupal.states-docs.21.patch | 5.4 KB | sun |
| #16 | drupal.states-docs.16.patch | 4.83 KB | rfay |
| #15 | states_bug_demo.tgz | 2.65 KB | rfay |
| #13 | drupal.states-docs.13.patch | 4.84 KB | rfay |
| #11 | drupal.states-docs.11.patch | 10.6 KB | rfay |
Comments
Comment #1
kkaefer CreditAttribution: kkaefer commentedThis isn't working by design. #states is so far *only* about visual representation. If you want to have validation, what you have to do currently is to check manually in the validate form hook.
However, the design of #states doesn't prohibit in any way that this gets actually implemented on a server side level as well; there's just no code for that yet.
Comment #2
rfayI would suggest it shouldn't be in there if it doesn't do anything, certainly if it doesn't do what it implies it does. I do understand why it can't...
Looking forward to your presentation.
Comment #3
rfayAny suggestions from anybody on what we should do with this? Just document it? Not mention it? In the documentation say "not really"?
Comment #4
sunJust document the fact, for core. And let contrib leverage it to do insanity.
Comment #5
rfayAgreed. I'll update the Form API reference. the core docs associated with the patch (that go in the process function) probably need an update too. I'll try to spiff them up in this issue.
Comment #6
rfayAttached is a rewrite of the docblock for drupal_process_states(). No code or kittens were changed in this patch, just docblock.
For ease of reading it, the api module's rendering of the docblock is at http://d7.drupalexamples.info/node/2. It didn't render well at all here in the filters we have for comments :-)
Comment #7
sunNice start! I'd suggest to be more concise and shorten lengthy sentences though.
Also not sure whether the entire description follows a certain structure/flow, which it should.
Revamped. :)
Comment #8
rfayOutstanding, nice work. Some minor style and api-module rendering issues fixed up in this one.
Rendered version is updated at http://d7.drupalexamples.info/node/2
Comment #9
rfayIf #767268: #states doesn't work correctly with radio buttons goes in first I'm sure this one will need a reroll.
Comment #10
sunLet's use 'visible' here. The initial patch was slightly confusing exactly because of the need-to-get-your-head-around-what-invisible-state-means confusion.
In general, the negated states should only be used if technically required.
Here, we state that multiple conditions must be met... but only one is visible, so the reader starts to ask whether the inner or the outer array is meant here. So let's just add a second
JQUERY_SELECTOR => REMOTE_CONDITIONS
Unlike above, the reader will have understood the structure at this point - so no need to duplicate in the second example.
Duplicate newline.
I purposively shortened this in my version, because I'm totally not sure whether this is technically correct. I've only ever seen "checked" and "value" in remote conditions, and none of the other states listed here.
If it's true that all of them are both states and remote states, then we should perhaps change the wording above accordingly.
In any case, it would make sense to split the list of states into two; whereas the second contains all states that are "possible" but not really implemented by Drupal core. (i.e. WTF is 'relevant' ? ;)
Let's remove this @todo, so we can commit this patch earlier. If required, we can open follow-up issues.
I think we can and should generalize this note. All states apply to presentation only, so there is never any server-side logic involved...
Powered by Dreditor.
Comment #11
rfayOK, this addresses all of the points in #10, I think. I also brought in the information about how to use 'value' with selects and radios from #767268: #states doesn't work correctly with radio buttons.
@sun, you laid down the gauntlet to know what actually works and what doesn't, so I had to find out.
I found this out by writing a (nearly?) exhaustive test which creates a gargantuan form.
The test is included in this patch, and it is *not* to annoy you. But after finally writing something that should have been done a long time ago, it would be sad if it lived out its days on my computer when it could be used to see if the #states system is sane. If anybody disapproves, I'll be happy to pull it.
You also may not like that I called out in the docblock the many states which "work" but don't actually do anything. We can kill that part if necessary and just hide those from public view.
Comment #12
MichaelCole CreditAttribution: MichaelCole commentedGreat job on this documentation. Without any context, I read the docs and got it.
Your instinct that the sentence "And these state names are supported in both states and remote conditions, but they don't actually do anything," is confusing is correct.
What does "supported" mean? Is it in the Drupal code, but doesn't do anything? Defined in an outside standard? Part of the jQuery selector syntax?
I think if you clarify the verb supported, this patch will be ready to go.
IMHO, if these states are defined as Drupal constants, then this "unsupported" section is appropriate.
However, if these are defined outside Drupal (e.g. jquery), and Drupal only knows about the "supported" states, then this section can be replaced with "Other states are defined by JQuery (like "relevant", "irrelevant", "readonly", etc) are not currently used by Drupal". No need to exhaustively list them here.
Nice work!
Comment #13
rfayPer #12 I tried to brush up the language a bit. These are actually states defined in core code, so @MichaelCole per your comments the section would stay here.
This patch drops the humongous test form. I'll try to find another home for that.
The rendered version of this is updated at http://d7.drupalexamples.info/node/2
Comment #14
sunWhat is an "appropriate" element? ;) Let's just drop that word.
1) Duplicate "be".
2) Everything starting from "but they refer..." can be dropped. Again, this is fundamental to all states, so we should explain this at the very beginning of the entire docblock instead.
3) Text should end in a colon.
Let's remove "And" and keep the starting sentence consistent with the others, i.e.
"The following states may be applied to elements and used for remote conditions, but are not implemented by Drupal core:"
Let's shorten this.
"When referencing select lists and radio buttons in remote conditions, a 'value' condition has to be used:"
Missing blank line before @param
84 critical left. Go review some!
Comment #15
rfay[x-post on the status. I'll fix when I post patch]
Here I'm attaching a test module with the code that was in form_test.module in #11. It's useful when one wants to do comprehensive click-testing of #states. Click on the menu entry "#states: Comprehensive exercise routine". It will probably stress your machine to load it as it creates a huge form.
Comment #16
rfayThis addresses the concerns in #14 except:
I left this in. I think this is a specific case where it's important to understand the difference. In all other cases we're changing the visual representation of the form in obvious ways. With required/optional we're doing something which implies that the element is being made 'required'.
In fact, this was the very point of this issue :-) The original title of the issue was "#states 'required' action doesn't actually make form element required"
Comment #17
sunSorry, but again: This premise applies to all states and conditions. None of them will actually "ensure" that element X is actually "checked". Same applies to "required" and all others. There is no difference, and people definitely have to understand that all #states are about presentation only.
Comment #18
rfay@sun, if you apply "checked" to a checkbox, it makes it checked. When you submit, it will be checked. If you apply 'disabled' to an element, access to the element is prevented. 'required' just fiddles with the css of the element to imply it's required (a server-side attribute) when it actually is not. There is a fundamental difference.
Should we just remove the reference to the things that aren't actually useful? touched/untouched, required/optional, relevant/irrelevant, and the like?
Comment #19
sun@rfay: .......... only, and only if JavaScript is enabled.
Comment #20
rfayCertainly, agreed. But the point is that there is a fundamental difference between what the "real" states do and the others. The "real" ones do what they imply they should. Expand/collapse, check/uncheck, show/hide. Yes, they do it only when javascript is enabled. But they do something. The rest don't actually do anything.
I propose that we remove the required/optional and its friends from the documentation. I think they were experiments. kkaefer says this one got committed before it was ready and that's the reason for so many things like this.
Comment #21
sunClarified.
Comment #22
rfayI'll take it. RTBC.
Comment #23
webchickAwesome!!! Great job on this, guys!
Committed to HEAD.