Vertical tabs are not collapsed when displayed in a custom block region. The content of the several tabs appears all at once, one field after the other.
Steps to reproduce:
- enable ds, ds_extras, field_group
- enable "View mode per node" and "Region to block" on admin/structure/ds/list/extras
- on admin/structure/types/manage/article/display/teaser, pick a layout
- then, on the "Block regions" tab, create a custom region
- populate this region with vertical tab group, some vertical tabs and content
- on admin/structure/block, add the block region to the content region
- create an article node and set the view mode to "teaser"
- view the node
This is something that broke relatively recently, we noticed this with the upgrade to field_group 1.2 (it worked with field_group 1.1). The issue is still present in the latest git versions of ds and field_group.
(We post this issue against display suite as opposed to field_group because the vertical tabs work alright when displayed in the regular content area or in one of the given areas in the picked layout. Whether this is caused by or better resolved in field_group I cannot tell, of course ...)
Comment | File | Size | Author |
---|---|---|---|
#43 | ds_extras_field_group_not_rendered-2221307-43.patch | 1.67 KB | mvwensen |
#18 | ds_extras_field_group_not_rendered-2221307-18.patch | 655 bytes | stevesmename |
Comments
Comment #1
WillowDigit CreditAttribution: WillowDigit commentedI have the same problem. I created a list structure with field group HTML element and tags. It works as expected. When I move it over to a block region, only the tags (with the rewritten html) are displayed, and none of the field group wrapper items. It is the same when cloning a view move that contains field-groups, although that I suppose is expected behavior.
Comment #2
vaartio CreditAttribution: vaartio commentedSame here. Field groups are rendered correctly if you place them in content region but once you move them to custom block region the field group itself is not rendered at all, only the fields inside it. It doesn't matter about the format of a field group (fieldset, html element, div, etc.). It works with Field Group 7.x-1.1 but not with 1.2 or 1.3.
Comment #3
aspilicious CreditAttribution: aspilicious commentedISn't this a fieldgroup problem than?
Comment #4
cspitzlayWell, I'm not sure which project is in a better position to fix this but the field_group vertical tab feature still works when in a standard region. It just breaks when inside a custom region from ds_extras. So I suspected that ds_extras does something different that you may be able to change in order to make the two modules compatible again.
If you think this is wrong then feel free to reassign to field_group.
Comment #5
vaccinemedia CreditAttribution: vaccinemedia commentedI can confirm that this is also the issue when using other field groups such as the simple div element. I'm attempting to nest a couple of them with a couple of fields inside the inner one to achieve a vertically aligned layout on top of a banner image using the old table > table cell display trick in CSS....
So in the block region we have:
Banner Block
Banner Image
> Table
>>Table Cell
>>>Tagline
>>>Call to action (link field)
Comment #6
seanBI traced the issue to ds_extras_field_attach_view_alter(). This function is looking for fields, but the fieldgroup are added to the array differently. I will try to create a patch, but not sure yet how to fix this (maybe it's not possible at all?).
Possibly related: https://www.drupal.org/node/1334128
*edit
The fieldgroups are added to the $block_data array just fine, the only thing that is missing is that field_group_fields_nest() needs to run for each region to set up the render array.
Comment #7
seanBPatch is attached!
Comment #8
seanBIgnore the old patch (stupid last minute copy paste error!). Here is a new one.
Comment #9
cspitzlayThanks for the patch!
I tested it (#8) and it works.
There is just one thing I noticed.
While the old combination of ds_extras and field_group
had the first vertical tab active the new version activates the last one.
Any ideas?
Comment #10
seanBNot sure if this is related to the patch? All it really does is nest some fields in fieldgroups with the function provided by the field_groups module. I didn't test this with vertical tabs but this could be a seperate issue?
Comment #11
stevesmename CreditAttribution: stevesmename commentedTested patch (#8) on ds_extras 7.x-2.6. This patch fails, the problem is all fieldgroups are rendered even if they are not put into the specific region (block to region). Based on the patch, I don't believe that it will make any difference if this was tested on 7.x-2.x-dev
Testing failed with module versions:
Field Group (field_group): 7.x-1.4
DS Extras (ds_extras): 7.x-2.6
Comment #12
stevesmename CreditAttribution: stevesmename commentedComment #13
alit CreditAttribution: alit commentedI confirm that patch #8 works for "div" fieldgroups.
Thanks seanB!
Comment #14
bskibinskiTested patch #8 and seems to work fine (tested with the "fieldset" and "div" settings)
I don't have the problem of fieldgroups always being rendered described in #11.
If the output within the fieldgroup is empty (whether placed in a regionblock or in the normal content) the label of the fieldgroup doesn't show.
Thanks for the patch!
Display Suite Extras (ds_extras) 7.x-2.6
Fieldgroup (field_group) 7.x-1.4
Drupal 7.32
Comment #15
stevesmename CreditAttribution: stevesmename commentedPatch #8 Bug:
Finally tracked my issue (#11), I admit it was hard to reproduce. The issue is reproduced by having the following hierarchy:
- Horizontal Tab Group
-- Horizontal Tab
--- DIV Group*
---- Field
* I believe it doesn't matter if this is a DIV, Fieldset, etc. It's the fact there is a third child that exists.
I'm marking this back to Needs work:
#8 patch will serve fine for some users, although I personally don't believe the patch should be committed "as-is", 1) because known issues with nested groups, 2) because field_group_fields_nest() function that is used; this function mixes ds_extras module with field_group module, (almost) as if ds_extras module is dependent on field_group module.
Comment #16
seanB1) Must be resolved, I agree!
2) We could add a module_exists check to remove the dependency?
I hope I can find some time to take another look.
Comment #17
stevesmename CreditAttribution: stevesmename commented2) I think that comment #3 makes a very relevant point.
Comment #18
stevesmename CreditAttribution: stevesmename commentedI've confirmed that Display Suite (DS) does allow for module_exists('field_group'). It's actually used a couple times in the DS modules.
I have also fixed the issue related to nested field groups. I do think this is a issue related to ds_extras module (not field_group), I think it's related to the lateness that hook_field_attach_view_alter is called in ds_extras. I did a few tests, and they confirmed that Patch #8 was a solid attempt (very close).
If by chance, this new patch doesn't get committed, I have lastly created a module that solves the problem without the need for the Patch to be applied. I admit the patch is a better route than utilizing my sandbox module. I'm changing the status to "Patch (to be ported)", hopefully a Display Suite module maintainer can agree on the patch.
Sandbox Module: https://www.drupal.org/sandbox/stevesmename/2366703
Comment #19
stevesmename CreditAttribution: stevesmename commentedComment #20
aspilicious CreditAttribution: aspilicious commentedI'll poke swentel for this one, it would be nice if someone else could verify this works in every case.
Comment #21
swentel CreditAttribution: swentel commentedlooks good
Comment #22
stevesmename CreditAttribution: stevesmename commentedThe other option was to create a wrapper function (much like field_group_form_pre_render). I wasn't sure if this would had been easy to understand in my initial patch.
So I'd maybe suggest the following, but I am not sure what the second parameter should be ('ds_extras'?).
Here is what field_group_build_entity_groups is:
Comment #23
stevesmename CreditAttribution: stevesmename commentedComment #24
mangy.fox CreditAttribution: mangy.fox commentedPatch #18 seems to work ok for the built-in formatters, but not for additional formatters such as field_group_background_image. Does anyone have any idea why this might be, off the top of their head, before I spend ages trying to work it out myself?
Comment #25
aspilicious CreditAttribution: aspilicious commentedAnd what about #23?
Comment #26
stevesmename CreditAttribution: stevesmename commentedRe: @mangy.fox (#24),
Try this attached patch. I had been thinking that my #22 patch was wrong (removing that patch), we should be passing 'display' instead 'ds_extras' (makes a lot more sense). I personally didn't test this but will hopefully later in the day.
I took a quick look at field_group_background_image and noticed that they are using the 'display' functionality so I firmly believe this new patch should work for you (and others).
Comment #27
stevesmename CreditAttribution: stevesmename commentedRe: @mangy.fox (#24),
Try this attached patch. I had been thinking that my #22 patch was wrong (removing that patch), we should be passing 'display' instead 'ds_extras' (makes a lot more sense). I personally didn't test this but will hopefully later in the day.
I took a quick look at field_group_background_image and noticed that they are using the 'display' functionality so I firmly believe this new patch should work for you (and others).
Comment #28
mangy.fox CreditAttribution: mangy.fox commentedSimilar to my experience of #23, I end up with no field group or contents rendered and the following warning
I'll give it another run on a clean environment later to make sure it's not just this current setup.
Comment #29
stevesmename CreditAttribution: stevesmename commentedCan you list the verisons of the modules you are using on the current setup? (ds, ds_extras, field_group, field_group_background_image)
Comment #30
mangy.fox CreditAttribution: mangy.fox commentedThat environment was with...
ds 7.x-2.6
ds_extras 7.x-2.6
field_group 7.x-1.4
field_group_background_image 7.x-1.1
Comment #31
mangy.fox CreditAttribution: mangy.fox commentedAlso tested on an older environment with field_group 7.x-1.3, same result and warning.
Comment #32
mangy.fox CreditAttribution: mangy.fox commentedTested on simplytest.me with patch #26
ds 7.x-2.7
ds_extras 7.x-2.7
field_group 7.x-1.4
Same result and warning. It seem the field group does not even need to be in use. Even when there is one in the disabled section, the block region fails to render.
Comment #33
stevesmename CreditAttribution: stevesmename commentedI was able reproduce the error. Very strange that I didn't have problems initially, I am leaving #18 patch for the time being. I will attempt to work on this issue later tonight.
Comment #34
aspilicious CreditAttribution: aspilicious commented#18 helped me on a project. Is it save to commit that as is? Even if it doesn't fix *all* the issues.
Comment #35
stevesmename CreditAttribution: stevesmename commented@aspilicious, let's mark it "Needs review" and let's see. I am personally not sure because additional formmatters such as #24 hasn't been tested enough on my end.
Comment #36
bdimaggioJust wanted to chime in/thank @stevesmename: #18 saved my bacon.
Comment #37
xenophyle CreditAttribution: xenophyle commentedDitto the above comment.
Comment #38
stevesmename CreditAttribution: stevesmename at Avagate commentedComment #39
Pere OrgaI confirm patch in #18 does not fix the issue with Field Group Background Image module.
I would be happy to address the issue in that module if I had to, but as far as I can see it's OK there: the render array of the field group is build in a hook_field_group_pre_render() implementation.
Comment #40
stevesmename CreditAttribution: stevesmename at Avagate commentedI have confirmed that the Field Group Background Image module is a separate issue, likely related to module weight. It's important to also understand that the Field Group Background Image module is not dependent on Display Suite or Display Suite Extras modules, it's dependent on field_group module. Field Group (field_group) module is not dependent on Display Suite either.
We are sort of asking for a favor from the maintainers of Display Suite to add patch #18 - it will help address other issues related to field_group, its dependent modules, and display suite regarding the #pre_render issues we are seeing.
Comment #41
stevesmename CreditAttribution: stevesmename at Avagate commentedLet's push this!
Comment #42
mvwensen CreditAttribution: mvwensen commentedThis patch is not working in combination with the Display Suite Region Blocks. We should be checking if the $field (from the foreach above) value is present in the #fieldgroups as a key.
I will return with a patch later.
Comment #43
mvwensen CreditAttribution: mvwensen commentedPatch from #26 is not rendering all fields in Block regions correctly. It assumes in the if statement all "groups of fields" are fieldgroups, but this is not always the case. I altered the patch, adding an extra check before adding the #pre_render function. If a field is not in the "#fieldgroups", it's not a fieldgroup.
I created a new patch but it should be tested by others on different set-ups/use-cases so it can be fine-tuned to work well in all cases.
Created the patch on the latest dev and tested it on the latest stable.
Comment #44
mbroere CreditAttribution: mbroere at LimoenGroen commented#18 works for me
Comment #45
achapCan confirm that #18 fixed the issue, and also confirming that patch #26 gave me the same issues as mangy.fox (all my fields within that group disappeared and it threw an error). It would be great to get the fix from #18 committed.
Comment #46
stevesmename CreditAttribution: stevesmename at Avagate commentedComment #47
davidam CreditAttribution: davidam commented#18 helped me on a project.
Comment #48
lauriekap CreditAttribution: lauriekap commented#18 worked for me as well.
Comment #49
mducharme CreditAttribution: mducharme commented+1 for #18
Comment #50
Marty2081 CreditAttribution: Marty2081 at LimoenGroen commentedAnother +1 for #18.
Comment #51
victoriachan CreditAttribution: victoriachan at Torchbox commentedPatch in #18 worked for me too. Can we get this committed please?
Thanks!
Comment #52
scuba_fly+1 for #18
Comment #53
dbazuin CreditAttribution: dbazuin at LimoenGroen commented+1 for #18
Comment #54
BarisW CreditAttribution: BarisW at LimoenGroen commentedCan we have this committed please? Tnx.
Comment #55
missmobtown CreditAttribution: missmobtown commentedJust chiming in that #18 was the fix I needed.
Comment #56
dbazuin CreditAttribution: dbazuin commentedThe patch from #18 still applies to version 7.x-2.15
Comment #57
bskibinskiI think the assigned "stevesmename" isn't active anymore, and nothing will happen with this ticket unless we change the maintainer. Little bit busy now, but i'll try to fix this tomorrow.
Comment #58
scuba_flyCan we get this committed?
Comment #60
aspilicious CreditAttribution: aspilicious commentedCommitted 18 to dev. Sorry for the delay.