Adding an alter hook after $group has been loaded would allow developers to inject e.g. classes to the $group object, which would then be passed to field_group_pre_render_
functions. Since these functions use the loaded $group object, it would be useful that the $group object used there could have additional / altered properties.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww’s picture

Issue tags: +project, +drupal.org D7

We need this for the D7 port of Project* for Drupal.org. For example, Project Issue Tracking is defining some field groups for various fields on the issue node type. But we need to alter those fieldgroups inside Drupal.org Customizations to do things like place the Issue tags field (which is a d.o-specific customization) into the right place in the UI.

Basically, we need an alter hook for hook_field_group_info(). For comparison, Views provides both hook_views_default_views() and hook_views_default_views_alter(). Field Group should do the same.

Thanks!
-Derek

drumm’s picture

I'm not sure we really need this for Drupal.org D7. We can always keep moving the field around with old-fashioned form_alter(). This is definitely a good improvement for later, to reduce our custom code down the line. Or am I missing something?

dww’s picture

Assigned: Unassigned » dww
Status: Active » Needs review
FileSize
1.1 KB

In my experience, using form_alter() for this stuff just becomes incredibly confusing and hard. For example, trying to get the weights right becomes impossible since anytime someone touches the field UI, table-drag can re-weight everything in unpredictable ways. Plus, having form_alter() magically move things out from under what the field UI is saying is confusing for site admins.

Anyway, after 10 minutes of trying (and sadly, failing) to find an existing ctools-provided alter hook that will work for this, here's an extremely trivial patch that I've tested works great for our needs.

DuaelFr’s picture

Assigned: dww » Unassigned
Category: task » feature
FileSize
1.34 KB

As usual, I needed it so I did it.
I hope it will help someone.

This patch is part of the #1day1patch initiative.

dww’s picture

Assigned: Unassigned » dww

Sad that we duplicated effort here.

I prefer my patch (#3) since we only invoke the alter hook once on all groups, instead of invoking it over and over for each group.

Also, my patch would allow a module to entirely remove another module's field group if desired, whereas #4 doesn't enable that kind of flexibility.

But I think #4 would work for Drupal.org's needs, so if for some reason the maintainers preferred that, we can do it that way instead.

Cheers,
-Derek

swentel’s picture

Status: Needs review » Reviewed & tested by the community

Yeah #3 looks better to me. I'll let stalski or zuuperman do the commit.

DuaelFr’s picture

I was so focused on this need that I didn't even see your patch before your last message Derek.

Yours looks greater than mine so I hope this will be the one chosen by the maintainer, even if I have to change some code on my project :)

Stalski’s picture

I am much more in favour of the patch in #3 because of the reasons mentioned above. It also opens up more doors to extremity.
Let's commit this.

drumm’s picture

Stalski - sounds good. Is anything holding you back from committing?

swentel’s picture

Status: Reviewed & tested by the community » Needs work

@dww - against which branch is this patch, because I couldn't apply the patch on 7.x-1.x

Also getting file mode errors

swenteldoos-2:field_group swentel$ git apply 1369536-3.hook_field_group_info_alter.patch 
warning: field_group.api.php has type 100644, expected 100755
warning: field_group.module has type 100644, expected 100755
error: patch failed: field_group.module:1449
error: field_group.module: patch does not apply

I could do it manually of course, but we need to make sure we know which branch you're using - 7.x-2.x isn't actually supported and we should close that one down :/

drumm’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.47 KB

Here is a reroll. I applied it against 7.x-1.1, which we are using in our BZR tree for the site, and rebased it to 7.x-1.x.

dww’s picture

Ugh, sorry, my local git clone was at the 7.x-1.1 tag, not the end of the branch. Thanks, drumm, for the re-roll. ;) Confirming the RTBC.

Thanks,
-Derek

swentel’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed and pushed!

Status: Fixed » Closed (fixed)

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

DuaelFr’s picture

This patch does not play well with features exported groups.
If you alters an exported group, it will be marked as overriden in the feature.

It is not a real bug but it can be painful when trying having an overview of your features to know which ones have been overriden.

  • Commit d10a619 on 7.x-1.x, 8.x-1.x by swentel:
    Issue #1369536 by dww, DuaelFr, drumm: Added an alter hook on $group...