Fields re-ordered by CCK do not have the right weight in Vertical Tabs

jshuster@drupal.org - April 24, 2009 - 21:48
Project:Vertical Tabs
Version:6.x-1.x-dev
Component:Miscellaneous
Category:task
Priority:normal
Assigned:Dave Reid
Status:needs work
Description

When configuring some fieldsets as vertical tabs and leaving others as regular fieldsets, I'd like to position all the vertical tabs above the regular fields.

Rearranging the group order in CCK is the obvious fix, but the menu and book fieldsets (and a few others) keep floating to the top.

So I set up an after-build function and reset weights on all the top-level groups, then resorted $form to the order I'd like ... but the menu and book fieldsets keep floating up to the top.

Apparently, CCK has some issues setting weights (http://drupal.org/node/290411), and it makes sense for this module to follow CCK group weights. But absent the ability to control group order in CCK, is there some other way that I've overlooked to force all the vertical tabbed fieldsets to the top?

Thanks ... and nice module!

#1

snorkers - May 7, 2009 - 19:16

I agree - this would be a nice facility to be able to re-order the sequence of vertical tabs, as a few 'randoms' seem to float to the top of the list.

#2

quicksketch - May 18, 2009 - 23:02

The "official" way to order tabs is simply to re-order the fields using CCK (admin/content/node-type/[type]/fields). Any modules that don't expose themselves to CCK should have the functionality added so that they appear on this list of fields as well.

#3

quicksketch - May 18, 2009 - 23:19
Status:active» fixed

Also related #357708: Adding weight to the fieldset?. Marking as fixed, since it's not the responsibility of Vertical Tabs to make fieldsets reorderable.

#4

System Message - June 1, 2009 - 23:20
Status:fixed» closed

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

#5

KarenS - June 28, 2009 - 14:20
Status:closed» active

Reopening this so people will see this comment. Reordering fields using CCK will not work because CCK applies the weights in pre_render, too late for Vertical Tabs to react to them. I've posted a patch for CCK at #409144: content_extra_weights gets corrupted and breaks the ordering of form elements on node edit form which I think will fix this, but I want to get input from the other CCK maintainers before committing it.

#6

KarenS - June 29, 2009 - 09:41
Title:Relative positioning of vertical tabs with ordinary fieldsets?» Fields re-ordered by CCK do not have the right weight in Vertical Tabs
Category:support request» task
Status:active» needs review

OK, as you will see in that issue, the weights are not set by CCK during hook_form_alter, they are set later, so Vertical Tabs will not have the right weights with the current code. A simple patch will fix this problem by retrieving the adjusted weights using a CCK helper function.

AttachmentSize
vertical_tabs_cck.patch 1.1 KB

#7

quicksketch - June 30, 2009 - 03:44
Status:needs review» needs work

Ugh, seems ugly to me. Vertical Tabs already has a weight of 300 I think, which should ensure it's last-run order. However, if CCK uses #pre_render to adjust weights, then we've got a bit of a problem. I think the preferable course of action would be to move Vertical Tabs weighting to #pre_render also, so it will fire after CCK. It'll also make the D6 module more like the D7 core version, which also uses pre_render.

#8

quicksketch - June 30, 2009 - 03:58
Status:needs work» needs review

This patch switches Vertical Tabs to a pre_render function instead. I think it should fix the problem if I'm understanding it correctly.

AttachmentSize
vertical_tabs_pre_render.patch 7.23 KB

#9

bigmack83 - July 11, 2009 - 21:34
Version:6.x-1.0-beta2» 6.x-1.0-beta3
Status:needs review» reviewed & tested by the community

Having the same issue, i tried the patch in #8 against the beta3 release and works great.

#10

dboulet - August 6, 2009 - 17:44

Thanks for the patch!

#11

bomarmonk - August 22, 2009 - 20:53

I'll review this patch and come back with my results. Thank you!

#12

Dave Reid - August 22, 2009 - 22:39
Status:reviewed & tested by the community» needs work

This needs to be re-worked in order to allow other forms besides the node form to be able to use this. See the latest CVS change in #500044: Extend the support to different modules / form pages.

#13

dww - November 1, 2009 - 20:18
Version:6.x-1.0-beta3» 6.x-1.x-dev
Status:needs work» needs review

Marked #613784: Vertical tabs weights incorrect (cck interaction issue) duplicate with this. Nothing new there, just FYI.

Ran into this bug myself, so I decided to take a shot. Rerolled for latest DRUPAL-6--1 branch as per Dave Reid. Tested on the site I was seeing the trouble and it seems to be working like a charm.

AttachmentSize
444378-13.vertical_tabs_cck_weights.patch 2.2 KB

#14

dww - November 1, 2009 - 20:20

Better yet, named the function vertical_tabs_node_form_pre_render() for better namespace.

AttachmentSize
444378-14.vertical_tabs_cck_weights.patch 2.22 KB

#15

hefox - November 6, 2009 - 19:23

Patch worked for me :)

#16

dww - November 6, 2009 - 20:21
Status:needs review» reviewed & tested by the community

;)

#17

Dave Reid - November 6, 2009 - 20:24

Will test out tonight and if it's indeed good will commit. Thanks everyone for testing so far.

#18

Dave Reid - November 6, 2009 - 20:25
Assigned to:Anonymous» Dave Reid

#19

Dave Reid - November 6, 2009 - 23:45
Status:reviewed & tested by the community» fixed

Tested personally and works great. Fieldsets were re-ordered as expected and no side-effects! Committed to CVS (http://drupal.org/cvs?commit=285314). Thanks everyone!

#20

jcmarco - November 8, 2009 - 01:55
Status:fixed» needs work

I found a collateral damage with this patch.

When you have cck_fieldgroup_tabs, before this patch, you were able to show the vertical tabs into the first tab with all the main content not in a defined fieldgroup tab ,
but now, the new #pre_render in vertical tabs (module weight:300) is executed after the #pre_render in cck_fieldgroup_tabs (module weigth:19) and it is not rendered before the cck_fieldgroup_tabs render the tabs.

Setting the vertical tabs module weight to 0 and this last #pre_render patch, the fields reordering in cck fields orders works fine even with the vertical tabs and the cck_fieldgroup_tabs render the vertical tabs correctly when it renders a tab.

#21

kiamlaluno - November 9, 2009 - 11:49

Setting the vertical tabs module weight to 0 and this last #pre_render patch, the fields reordering in cck fields orders works fine even with the vertical tabs and the cck_fieldgroup_tabs render the vertical tabs correctly when it renders a tab.

I think the Vertical tabs weight has been raised to 300 because the module needs to be the last module to execute hook_form_alter().

Would not to increase the weight of cck_fieldgroup_tabs.module be possible without to have any collateral effects?

#22

jcmarco - November 9, 2009 - 19:00

It sounds good! The result is the same.
Probably more modules that are using vertical tabs could be affected
if they are using #pre_render to render their content and vertical tabs.

#23

realityloop - November 10, 2009 - 03:17

yes it affects nodeformcols as well I can't get vertical tabs to display in the footer region of nodeformcols anymore

#24

Dave Reid - November 10, 2009 - 00:35

What is node_form_cols?

#25

hefox - November 10, 2009 - 01:01

#26

kiamlaluno - November 10, 2009 - 08:41

The issue realityloop is referring to is #628192: Can't move vertical tabs to footer area.

It's not clear why Vertical tabs output should be moved in the footer area. Considering that in the node edit form it contains fields for some node features, I don't see the purpose of moving them in an area that would have less visibility (if the footer area is the theme footer).

#27

hefox - November 10, 2009 - 14:26

Based on the description I would guess it's:

add ***
[ left column][right column]
[footer]

Not the actual drupal footer (that's theme dependent and doesn't need to exist, so would be problamic to use, specially for form submission).

I haven't used it before though.

I'm not the maintier or anything of .. anything, but here's my thoughts on the issue:

Perhaps one of the issues is that vertical columns does not create an order-able entry in the Manage forum and cck settings (at least I haven't seen an entry there for it)? However, other items do make those entries in that (which become under vertical tabs) so it gets complicated. Also, that won't solve the issue since/if nodeformcols does it's actions in form_alter since the vertical tab column doesn't exist yet now ( http://drupalcode.org/viewvc/drupal/contributions/modules/nodeformcols/n... ). My guess is that nodeformcol completely takes over from cck (for ordering, etc.) considering it creates the three 'regions' so perhaps the solution would reside in nodeformcol? There's already an issue created there ( #628192: Can't move vertical tabs to footer area by realityloop ).

#28

kiamlaluno - November 10, 2009 - 17:27

Should not Node form columns use hook_theme_registry_alter() to modify the theme data?

I have misunderstood which footer area the issue was referring to. It's correct to say that it was the footer area that the modules adds to the page.

5 /**
6 * Implementation of hook_theme().
7 */
8 function nodeformcols_theme($aExisting) {
9   return array(
10     // This needs to run after node.module's hook_theme(), which we ensure
11     // by setting this module's weight to 1 during install.
12     'node_form' => array(
13       'template' => 'node-form',
14     ),
15     'nodeformcols_configuration' => array(
16       'template' => 'nodeformcols-configuration',
17       'arguments' => array('element' => array()),
18     ),
19   );
20 }

#29

realityloop - November 10, 2009 - 21:12

hefox has hit the nail on the head for what I was trying to explain:
vertical columns does not create an order-able entry in the Manage form and cck settings

 
 

Drupal is a registered trademark of Dries Buytaert.