Compatibility with Field group module

That would be nice indeed ! I have been surprised to find out field groups where not rendered by default. As per saif, it's a top-most used module and one can expect it to work out-of-the-box.

Patch attached with proposed solution.

I have been surprised to find out field groups where not rendered by default. As per saif, it's a top-most used module and one can expect it to work out-of-the-box.

Well, they should be providing this hook on behalf of other modules, then.

I tested this on 2.11 and groups are still not rendering. Is this supposed to be a working patch?

Edit: How would this work if the Variations field (Add to cart form) isn't positionable in the Display settings?

@fatmarker: I am surprised you can't create and render groups using this patch since I am currently using it.
About the field display question, it requires a second trick, a couple lines of code that are not the purpose of this issue here.

Also: I just saw a project with 6 vertical groups on a product form using Field Group

What is not working?

@mglaman: The problem is that Field Group can't know the non-standard render element key that Commerce uses for Product/Variation content render elements. The new Field Group alter hook provides a way to specify that, and the implementation you saw may be using that alter hook, but surely it shouldn't be the responsibility of Field Group to implement that on behalf of every possible entity?

I think ideally Core should be providing a way to discover the entity content render element key, via annotation or config? I haven't found any issue for that though.

Meanwhile, Commerce could easily implement this for all entities it provides, with no side-effects whatsoever.

Wait - is the problem _display mode_ not _form mode_? I've never used Field Group for display mode control, only forms. The issue hasn't clarified that, I guess, which is why I was very confused.

Yeah definitely display mode, not sure if a custom entity render element key also applies to form modes? I really think core needs a way to transparently discover and provide the element key for whatever is being rendered, so contrib doesn't need to care.

But field groups can also be used in form modes, so if the render element there is not ['form'] I think this issue can and should apply to both, assuming it doesn't get bumped to core.

Ah okay. We don't preprocess the form display and merge things into a different element than form.

The real problem is this:

    'commerce_product' => [
      'render element' => 'elements',
    ],
...
    'node' => [
      'render element' => 'elements',
    ],

This works and is correct. But, Drupal core set a pattern to preprocess these values out of elements and move them somewhere else (content.)

So it sounds like Field Group might be broken for anything which does not follow the template_preprocess_node pattern of moving elements children into content OR it does, but uses a different key.

Field Group looks for ['content'] by default, but if the entity doesn't follow that pattern, it can't work, and Field Group can't discover the correct render element to use because there is no mechanism to do so (I think?).

I don't know if there is a core specification for this? Is everyone just assuming that what node entity does what is some kind of standard? If we can reliably expect [entity_machine_name]['elements'] or [entity_machine_name]['form'] to contain the render elements, perhaps that is a reliable solution for Field Group module. Or is the entity itself expected to preprocess those to ['content']? It would be nice to know this is a specification, not just a convention.

I don't know if there is a core specification for this?

Nope. Just a pattern people copy.

If we can reliably expect [entity_machine_name]['elements'] or [entity_machine_name]['form'] to contain the render elements, perhaps that is a reliable solution for Field Group module.

That's what "render element" is for :) Technically we could always just have "{{ elements }}".

See

    'html' => [
      'render element' => 'html',
    ],
    'page' => [
      'render element' => 'page',
    ],

The problem is people are now preprocessing and altering the content key or whatever, not the render element holder. Honestly, the node pattern ruined the core theme system pattern.

Hi,

It still does not work. I think it is not a minor but rather a major feature. Having the possibility to use field_group to organize the display of product and product variation. #2 did not work for me.

just FYI

function MYMODULE_field_group_content_element_keys_alter(&$keys) {
  if (!isset($keys['commerce_product_variation'])) {
    $keys['commerce_product_variation'] = 'product_variation';
  }
}

does not work too.

It is minor, the problem is with field_group, which was not maintained. Please try again with field_group rc1.

I am already working with 3.x-dev of field_group... but still field_group are not rendered on commerce_product.

See my comments in #10 and #12. Field Group needs to solve this and its assumptions.

Ok, I wrote on the dedicated field group issue... The problem is very annoying as in order to print product variations fields into a requested field group, we must move every field programmatically to the product corresponding field_group... A great pain in the...

Imo, in terms of UX (both for editors and developers) this should be critical for both modules as there is no easy way to create a proper display for commerce product right now.

I find out, that commerce product manage display works with "Display suite". This can be used as alternative until fix.

Until fixed, I find using Drupal Core's view modes and the 2 code changes listed at the following link renders a product's field groups: https://www.drupal.org/project/commerce/issues/2910391#comment-12492686

The patch seems to have fixed the issue for me.

The patch works for me.

Added a second key to the patch so that it works for both products and product variations.

Note: it has been a bit strange and required several cache rebuilds before it decided to work, but now it is perfect and no other match is required. Works against field_group 3.2RX2

I've updated the patch in #23 and added another key so it works for products bundle.

#23 patch works for me for both product and product variations

I think patch #24 is missing an extra line feed at the end; when applied with composer-patches, the closing curly brace is not applied.

-mike

Re-roll patch from 24 with additional line feed.

Re-roll patch of #28 with additional line feed - PhpStorm truncating blank line feed.

Patch 29 works for me when applied with composer.

The functionality of the Field group module is now available for products.

Patch 29 works for me too. Tested with Drupal Commerce 8.x-2.20 and Field Group 8.x-3.1
+1 RTBC
Thanks!

Again, per #17, can someone please fix Field Group?

I'm not quite sure what you want to have fixed there? To be honest, the way commerce_product is handling its template variables is strange and unlike everything else. it does the same copy render keys from A to B, it just has different variables. And yes, there is no enforced standard, but $entity_type + content as keys has been the default pattern since D7 for all core entity types. Pretty much since before entity API even existed :)

And the only reason core doesn't do it by default is because my issue for adding default preprocessing and entity templates was blocked: #2808481: Introduce generic entity template with standard preprocess and theme suggestions and I'm still annoyed about that ;)

And copying back from Slack what I remembered while pinging @mglaman there why this pattern exists:

I don't see how this could be fixed in field_group. the reason for copying the keys is IIRC that printing just {{ elements }} in this case would basically be a recursion, because that again has #theme => something. in D7, sometimes drupal_render_children() was used for that, sometimes this. your html/page examples are different because those do not contain a dynamic list of fields, you always explicitly print the regions, never just {{ page }}

@berdir: if #29 really solves the compatibility issue, why isn't that committed to field_group? would fit there better imho. I see no reason, why Commerce should implement an field_group specific alter hook

That's something you can always discuss when two contrib modules have to work togeteher. IMHO, it makes sense to have this in commerce, field_group does provide an extension point and commerce is the one that's not following the standard.

I needed this and the patch in #29 works for me.

I believe that implementing a simple hook to solve compatibility issues with a popular module should not be discouraged, and it is not the first time this will occur in commerce (see commerce_product_search_api_views_handler_mapping_alter).

I also believe that providing a solution should come first. If we want to promote changing the logic of field_group, maybe open an issue there and reference it in the comments of this hook implementation. Let's move this forward, it's a simple, working solution.

Patch #29 working for me too.

I also feel Commerce could provide this solution instead of field_group.

We have a simple, working, reviewed patch here, can we have this committed?

I removed

$keys['commerce_product_bundle'] = 'product_bundle';

From the patch since Commerce core doesn't define product bundles.

This still does not work for me. Somehow field groups are not rendered at all no matter what I do. Tried to update Commerce to latest 2.28 and still no luck. Patch was commited as I can see in the code but it does nothing. Everything is rendered based on order number of product variation manage display page but no field groups are rendered at all.

Seems broken for me too - field groups defined for the customer profile are not shown during checkout 😟

use this guide
https://docs.drupalcommerce.org/commerce2/developer-guide/products/displ...

@bolaghi, the guide is not usable in Field groups context

However I could get the proper result by this preprocess code in custom module:

function mytheme_preprocess_commerce_product(&$variables){
    
    if ($variables['elements']['#view_mode'] == 'full') {
        $variables['elements']['#fieldgroups']['group_right']->children[] = 'variation_list_price';
        $variables['elements']['#fieldgroups']['group_right']->children[] = 'variation_price';

        $variables['elements']['#group_children']['variation_list_price'] = 'group_right';
        $variables['elements']['#group_children']['variation_price'] = 'group_right';
    }
}

For stores, see related issue.

Been struggling with this issue today and in case its helpful to anyone else I have finally managed to get it working..... I wanted a field group for a product variation type.

Thanks Hamulus you pointed me in the right direction!

First I don't think its possible to use Field Groups that are created on Commerce Product Variations, you must create them on the Commerce Product.

Once you have created the required groups use the example Hamulus provided to move variation fields into the field groups.

Not great for a site builder but at least it works!

Re #48 any ideas from the Commerce maintainers / Centarro if this should be a follow-up and if there's a good way to solve that?

Is #3121339: Widget "Single variation (Product information)" ignores field groups evantually related?