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.

CommentFileSizeAuthor
#14 630486.patch748 bytesrobloach
#8 content-types-vertical-tab-summaries-missing-v2.patch2.17 KBbowersox
manage-content-types-verttabs-patched.gif57.54 KBbowersox
manage-content-types-verttabs-head.gif55.63 KBbowersox
content-types-vertical-tab-summaries-missing.patch712 bytesbowersox

Comments

bowersox’s picture

bowersox commented

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.

robloach’s picture

robloach
he/him
commented
Status: Active » Reviewed & tested by the community

Haha, not sure where content_types.js went to.

seutje’s picture

seutje commented
Status: Reviewed & tested by the community » Active

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

seutje’s picture

seutje commented
Status: Active » Reviewed & tested by the community

oops, didn't mean to crosspost

bowersox’s picture

bowersox commented

Should the #attached value 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'),
seutje’s picture

seutje commented

hmm that would make sense, because if someone form_alters out that piece the js shouldn't still get added

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work

Sounds like this still needs work?

bowersox’s picture

bowersox commented
Status: Needs work » Needs review
StatusFileSize
new content-types-vertical-tab-summaries-missing-v2.patch2.17 KB

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.

bowersox’s picture

bowersox commented

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.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

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.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Issue tags: +vertical tabs

Adding tag

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs review » Reviewed & tested by the community

This is the best solution.

bowersox’s picture

bowersox commented

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?

robloach’s picture

robloach
he/him
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 630486.patch748 bytes

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.

bowersox’s picture

bowersox commented

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.

casey’s picture

casey commented
Status: Needs review » Reviewed & tested by the community

Very much indeed

casey’s picture

casey commented

#14: 630486.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Agreed.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

Status: Fixed » Closed (fixed)
Issue tags: -vertical tabs

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +vertical tabs

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.