Posted by eaton on July 22, 2007 at 10:00pm
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | node system |
| Category: | feature request |
| Priority: | minor |
| Assigned: | eaton |
| Status: | closed (fixed) |
Issue Summary
When hook_form_altering the node editing form, there's crazy goofy things that need to be done -- pulling a form element out of the form, checking for a special key, appending it to the form_id, and so on...
This just sets $form['#node_edit_form'] = TRUE. Check that, and you know you're on the node edit form. ;) It's not a killer patch or anything, but it would definitely make things simpler.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| nodetype_form.patch | 3.77 KB | Ignored: Check issue status. | None | None |
Comments
#1
oh yes, this is far too messy right now. subscribe.
#2
no longer applies. Probably small enough it could squeeze into this version though.
#3
anyone available to reroll and rtbc this?
#4
Moving feature requests to D7 queue.
#5
Back from the dead, and ready for review. The whole test suite is passing for me.
#6
Now, with a period at end of a comment, as suggested by Catch.
#7
+100 for something nicer than
- if (isset($form['type']) && isset($form['#node']) && $form['type']['#value'] . '_node_form' == $form_id) {This patch is a really nice DX improvement.
However, why:
+ $form['#node_type'] = $node->type;Since $form['#node'] is set on any form where $form['#node_edit_form'], you can just use $form['#node']->type same as you use $form['#node']->nid or any other node property. Then that makes one less FAPI property for people to know about.
#8
Yeah, I concur that the #node_type property is overkill. checking for #node to ensure that it's a node edit form is risky, but once we know it's a node edit form, we can very easily check its type. Thanks to PHP eval order, the following works nicely:
if (!empty($form['#node_edit_form']) && $form['#node']->type == 'blog') {...}#9
Cool. Let's remove that (doesn't seem to be used anywhere in the patch) and get correct the "// Set the id of the top-level form tag" comment so it correctly reflects what the code below is doing now.
#10
My mistake, I was looking at an earlier patch "+ // Set the id and other useful elements." capitalize ID please.
Also, I don't see a hunk for book.module? Are there other ones we're missing too?
#11
Removed that extra line which set the type. I did *not* change id to ID because I refer to the HTML id attribute which is almost always lowercase.
I added the book.module hunk. I searched for more instances of this node form check but found none more in core.
#12
Re-rolled with appropriate menu.module changes.
#13
Looks like a small but valuable improvement and all tests continue to pass. Committed to CVS HEAD.
#14
Docs, please! :D
* Need an update to the upgrading docs
* Need an update to the FAPI reference for HEAD.
#15
Added to upgrade guide http://drupal.org/node/224333#node_form ... I'm not touching that FAPI reference doc though. I proposed an alternate solution for that doc but it has not been embraced yet - #100680: Automatically generate Forms API & other big array documentation
#16
.
#17
.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.