Posted by bowersox on November 12, 2009 at 5:11am
9 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | node system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | vertical tabs |
Issue Summary
I believe the summaries are missing from vertical tabs on the edit content type admin page (e.g. /admin/structure/types/manage/article).
It appears that we are never attaching content_types.js at all. I have included a patch that simply adds 3 lines of code to content_types.inc:
+ '#attached' => array(
+ 'js' => array(drupal_get_path('module', 'node') . '/content_types.js'),
+ ),I've attached screenshots showing the current page in HEAD and the patched version that has the summaries.
This is in Drupal 7 HEAD as of 11/11/2009. I couldn't find any other relevant issue addressing this. I don't know this code terribly well so please advise if I'm way off.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| content-types-vertical-tab-summaries-missing.patch | 712 bytes | Idle | Passed on all environments. | View details |
| manage-content-types-verttabs-head.gif | 55.63 KB | Ignored: Check issue status. | None | None |
| manage-content-types-verttabs-patched.gif | 57.54 KB | Ignored: Check issue status. | None | None |
Comments
#1
Looks like this issue added the functionality originally: #439798: Vertical Tabs for the Node Type Form.
Back then it included this code...
$form['#attached_js'] = array(drupal_get_path('module', 'node') .'/content_types.js');...at the very top of node_type_form() which is probably a more appropriate place.
#2
Haha, not sure where content_types.js went to.
#3
yea I noticed that the content types didn't have a summary, but I didn't notice there was a js file just sitting there :x
#4
oops, didn't mean to crosspost
#5
Should the
#attachedvalue actually appear in each relevant section of the form?For example, in modules/node/node.pages.inc we attach node.js in each section of the form:
$form['revision_information'] = array('#attached' => ... 'node.js'),
...
$form['author'] = ...
'#attached' => ... 'node.js'),
...
$form['options'] = ...
'#attached' => ... 'node.js'),
#6
hmm that would make sense, because if someone form_alters out that piece the js shouldn't still get added
#7
Sounds like this still needs work?
#8
Check this one out. This patch attaches content_types.js in each relevant section of the form: submission, workflow, and display. Those 3 sections of the form get vertical tab summaries from content_types.js.
#9
On IRC @RobLoach was saying maybe we don't need this in 3 places and we should go back to 1 attach. He also suggested we could get rid of the redundant attach statements for node.js in modules/node/node.pages.inc.
I was away from IRC for a bit and missed the chance to follow up to ask what about the situation @seutje described of someone form_alter'ing one section out but still needing the JS.
Either way, we should fix this and restore the functionality.
#10
It's more than reasonable that somehow we could alter one of the fieldsets to #access = FALSE. So it should be attached to any fieldset that it could be. Or just attached to the form itself.
#11
Adding tag
#12
This is the best solution.
#13
This is a bug fix that's been sitting at RTBC for quite a while. Is there anything I can do to help this get through the process?
#14
Hmm, seems like an appropriate place for it would be on the vertical tab element itself. The JavaScript provides the summaries for the additional settings vertical tabs, so that's probably the most appropriate place it could be, without having to duplicate code in three different places.
#15
The patch works correctly and restores the vertical tab summaries when editing content types (eg /admin/structure/types/manage/article). Looks ready for RTBC to me.
#16
Very much indeed
#17
#14: 630486.patch queued for re-testing.
#18
Agreed.
#19
Committed to CVS HEAD.
#20
Automatically closed -- issue fixed for 2 weeks with no activity.