Place all top level form elements into a default vertical tab group

This looks great! I'll review when I get the chance.

Hi
just tried the patch and some problems appeared; after enabling the main vertical tab the buttons like save and review disappeared !!
additionally all CCK filed and groups also disappear.

I had the same issue as #357300-2: Place all top level form elements into a default vertical tab group and was able to fix it by adding this.

After (in your patch):
($key !== 'vertical_tab_default') &&

Added:
($form[$key][submit]['#type'] != 'submit') &&

Thanks for the patch, I love it!

Not really sure if the change I made is the "correct" way to fix the issue with the patch, but it does work for me.

Investigating further, it still looks like it is not allowing one to submit, even with the change I posted above.

The edit page just gets reloaded on hitting save.

I'm guessing that the patch will need to be more selective about exactly which parts of the $form object it moves into the default fieldset. I'll take a closer look at it when I have the time.

Thanks for the response. I agree. I tried to make the script a bit more selective, but perhaps I didn't make it selective enough either.

I just tried to filter out the submit buttons. There may be some other inputs or fieldsets that are getting pulled in as well, and that would cause the form not to post properly.

I look forward to any further development. It really cleaned up my posting pages when I had it enabled (*just couldn't submit, which is a bit important* ;)

Thanks again.

Any update on this patch lesmana? Hoping to add this to my site before launching ;)

I was able to get it working on my site by doing things like:

around line 52 of the patched file. This is most certainly not the best way to do it, but it does get it working for me.

subscribe

This works, for the most part.

One complication though. Where Organic Groups are concerned. The OG Settings (such as Membership type, Private group etc) are top level elements. When I place this into the default vertical tab, and have administrator permissions (ie: Admin can see all fields) then the tab displays the fields as expected. However, for a normal authenticated user that can create groups, and set their type but not edit other permissions then nothing displays in the vertical tab not even Title & Membership type.

I have a feeling this has to do with the access check in the patch where if a user doesnt have access to the fields, they arent displayed... this renders the entire tab unable to display, because the user has access only to title, but nothing else.

subscribing

About #8 - you shouldn't check for #title in generic module - that is very ugly workaround cause #titles are always different
Below is better check than in #8 :

Seems to work ( captures title and body node fields successfully). Please test it

I tried the patch along with the change in #12 but found it too selective and doesn't allow you to have other fields in the default fieldset (e.g. cck).

I think a better approach is to work out which elements must be excluded from the fieldset and then allow the others.

I am currently using a list like this for my site:

Here it is in context:

subscribe

subscribe

Would anyone care to reroll the patch with some of the suggested modifications, such as those in #13? This sounds like it could be a great idea, but without an updated patch, it's too hard to review.

EvanDonovan
I did it, please review

Attachment	Size
vertical_tabs.patch	2.82 KB

I think rather than having a list of "$restricted_keys", it'd make sense instead to have a list of "$restricted_types", so it would exclude hidden, value, or unknown field types from the list. Other than that, it looks like this patch is missing the relevant JavaScript changes, could they be included also?

Oh, it doesn't look like JS changes aren't necessary because the fields are shoved into the fieldset during hook_form_alter(), silly me. Moving fields around physically in the form is dangerous business. Modules will frequently assume the location of certain fields, such as $form['title'], and moving it from this position could cause conflicts with other modules.

A better approach to take here would be to add a #pre_render property on the form during the hook_form_alter(), then move the items during the pre_render stage, effectively changing where the elements are rendered but not affecting the original $form variable. This is nearly the approach used by Drupal 7 and should significantly reduce the chance of conflicts with other modules.

You are right, this patch was broken anyway: it makes all non-core fieldsets as children of default tab. How could I miss it, huh... Probably because I had fieldgroups set up as tabs already...
I'll try to fix this as you suggested

Attached new patch, updated with prerender callback function. Also I unified both per-key and per-type control: it seems using only one is not flexible enough. For example, 'buttons' element has valid 'markup' type but we need it outside of VT. There can be more examples.
So $valid_types and $exclude_keys remain subject to discussion.

We probably need a better way to control it: maybe add hook to let modules exclude some elements from inclusion to default tab ?

Attachment	Size
vertical_tabs_defaulttab_prerender.patch	5.14 KB

Added support for hook_vertical_tabs_default_tab_alter(). Now even if there exists some conflict between VT and some other module, it can be easily resolved either by module maintainer or site admin by implementing this hook.

Attachment	Size
vertical_tabs_defaulttab_prerender.patch	5.41 KB

The newest patch seems to be working flawlessly for me! Could others please test so it might get committed to the module?

subscribe

-------
Shuodui - Chinese Article Submission
www.shuodui.com.cn

Tested and working really well.

A new feature request, when it is used with tabs, cck fieldgroup tabs or multistep modules, where the node content is broken in different "screens",
then if there is no main content to show in the actual tab, then the vertical tab is there but empty and this probably could be avoided, (when is in the tab where the title or content edition is, it works fine)

#22 Patch seems to work great for me.

Great work guys.

Hmm.. actually, it seems to break the content_profile module. I get this error on trying to register a new user, which disappears after disabled vertical tabs

Fatal error: Unsupported operand types in
/XXX/sites/all/modules/content_profile/modules/content_profile_registration.module
on line 185

subscribe

There is one problem with the patch:
It requires that all content is placed within the vertical tabs, it is not possible to keep some fieldgroups outside.
(this is a problem, when you are using content types that are not yet compatible with vertical tabs)

Ok, replying to my comment in #29, the solution is fairly simple: I would suggest to remove "fieldset" from the valid types in vertical_tabs_node_form_prerender.

My intuition: If the user decided against explicitly enabling tab functionality for this fieldgroup, then don't force it.

@drupal24: patch contains hook that can be used to control what is placed inside and outside of default vertical tab. You are also missing the point that fieldset doesn't always == fieldgroup. Fieldgroup can be rendered as fieldset, but fieldset alone is just an element containing some data. So it's perfectly valid to put it inside by default.

If disabled fieldgroups still get inside, it's whole different question.

EDIT: I get what you mean. You want disabled fieldgroups outside of vertical tabs. But I made it intended default behaviour: if something is rendered before or after VT than there is no much point in this patch. I made this patch so Vertical Tabs could basically override whole form besides stuff that needs to be outside such as buttons. I understand that there can be use cases as described by you, but it's one way or another, just a matter of taste. As patch author I implemented what I like better ;) Hook for altering is included so any module can change default behavior anyway.
So yes, with this patch enabling or disabling fieldgroup changes if fieldgroup is rendered as separate v.tab or inside of default v.tab, which I think was original intention.
Ofcourse this may be expanded to provide better user interface which will give user more granular control of what happens with each fieldgroup.
But I won't implement it myself, for 2 reasons:
1) I'm happy with current behavior
2) Since I made this patch so much time passed and there was no feedback from module maintainers. Noone wants to waste time and efforts.

2) Since I made this patch so much time passed and there was no feedback from module maintainers. Noone wants to waste time and efforts.

Ow, sorry, I did not really look at the dates. It is a pity that there are so many improvements lost by not being commited to the projects.

However, I spend some time with vertical tabs and your patch and I am happy too. Turned out to be a simpler aproach than hacking cck editing into panels. And it's faster.

I have made some changes to the code and the js, and now I have fieldgroups above and below the tabs etc. I find it quite useful to put some unneeded/expert user stuff in collapsed fieldgroups underneath.

Actually there is error in the patch: drupal_alter() by default allows alteration only for first argument passed. So in this patch hook_vertical_tabs_default_tab_alter() doesn't work like I wanted.
I will happyly reroll this patch and maybe advance it with some user settings for default tab behavior, AFTER I receive feedback from module maintainer that this will be eventually committed. Until that I will keep any changes in my local version.

Obviously this patch "needs work" but keeping it in "needs review" so it will get more attention.

I rerolled last #22 patch to work with last dev version (Aug-23).

It works fine for me, it has the same behavior than cck_fieldgroup_tabs where if you set a name for a default tab, then everything not included in a tab is set into a default tab with the defined name.

Attachment	Size
defaulttab_prerender.patch	4.76 KB

I am trying to get this latest patch to work, but I get

I tried changing the permissions of the patched module (chmod og=rw filename), in case the problem was in the patching messing those permissions, but no luck. Can you post a patched version of the module and I'll test again? Thank you.

subscribing

I'm not sure that this should be committed based on the fact that this is now how the D7 functionality is. Could this be implemented as a separate sub-module?

+

http://drupal.org/project/vt_default

I do not expect that I need another extra module. It would be much better if it's integrated in the main vertical tabs module.

Again, I state that I'm not sure what the point is including this when it's not supported in Drupal 7 and this is intended to be a backport. If the other maintainers think it's a good idea, we can go from there.

There was no feedback for several months. Its too late to complain when there is ALREADY separate module. Effort already spent. Don't be silly.

Why is the project name if the other module not vertical_tabs_default? It's a general development rule of Drupal - not to shorten names, variables, translatable strings, etc.

Is this the issue queue for the Vertical tabs default module? Nope, ok just checking.

BTW It's db_log.module and not database_logging.module in core, nonetheless, so that argument is bunk. :)

Sounds like you'd like to rename the Vertical Tabs module to "vt"? :-)