'#group' Form API property only works with <details> elements

While working on the details patch, I noticed that Views UI apparently implements its own #fieldset_group property to move form controls into the three columns that make up the Views view admin UI.

The logic that is going on in there seems to be fairly similar to core's #group for details (formerly fieldsets). I suspect that this was added to Views, either because its new interface design got implemented before #group existed, or because Views was not aware of it.

Anyway, my bottom line is:

We should turn the #group facility into a ultra-generic thing.

It has proven itself to work really well for grouping into vertical tabs, and its implementation is mature - the logic accounts for many special cases (e.g., handling for the case where the specified #group does not exist, properly handling access, and properly handling empty groups [i.e., not rendering the grouping container when it does not contain any [accessible] groups], etc.pp.).

Therefore, I think we want to move the entire logic belonging to #group from the current details-related callbacks into separate callbacks for #group handling, and then, add the callbacks to the element info that need them (i.e., details, fieldset, perhaps even container).

Going to try this, because it's probably going to be a blocker to commit #1838114: Change node form’s vertical tabs into details elements. See this lovely screenshot.

Here's a patch that allows container also to group elements.

API changes

• form_process_details() is renamed to form_process_groups()
• form_pre_render_details() is renamed to form_pre_render_groups()

Two things that Sun can probably answer or help with:

• form_pre_render_groups() and form_pre_render_groups() still contain references to 'details' in the documentation and inline comments. Those can be removed I guess
• form_pre_render_groups() does a check on #collapsed and then adds an 'open' attribute which doesn't make sense on the 'container' element. Should this be split more up or just add checks or a property on the container element ?

Screenshot posted here

#4: 1856178-4.patch queued for re-testing.

Wow! @swentel, you rock hard. I didn't expect the baseline for this to be *that* simple. :)

Though perhaps that only underlines even more what I stated in #1 already — the entire #group functionality is pretty mature and relatively abstracted already.

To complete this patch, we need to add isolated (DUTB) tests, which prove that we're able to assign #group nilly-willy and get what we want.

As an addition test case, we could eliminate Views' custom #fieldset_group property - although that's a lot of code (but perhaps also only a search+replace?) - so not sure whether that should be a separate follow-up.

re: #4:

1) Yeah, let's just remove/replace the references to details.

2) form_pre_render_details() actually contains an early-return right before the #group processing, since the entirety of that depends on #parents, which exists within a Form API context only. So I think what we want to do is:

- Split that #pre_render callback into two: form_pre_render_details() + form_pre_render_group()

- Apply form_pre_render_details() + form_pre_render_group() to the #type definition for details (in this order).

- Apply form_pre_render_group() to the #type definition for container.

3) Not sure why you chose "groups" (plural). Since the render property is #group (singular), I'd actually prefer to go with singular callback names, too, so that there's a direct relationship.

4) The general concept seems to be that #group works for "container"-alike element #types. Should this also work for #type fieldset? Anything else?

New patch. I think I addressed all points, except for 4 (only applied it to type #container right now) and no tests yet. I'm assuming DUTB is for unit tests or something alike, and do we have them for details already somewhere ? Don't have time for tests though today anymore, will be something for this weekend (or possible maybe a little tomorrow), unless someone beats me to it.

I've do the views as a follow up though.

And now with tests. It took me a while to find a good place to add tests, hope it's ok :)

And fieldset can now use #group as well. Added tests for vertical tabs as well. This could go RTBC fast imo.

Looks good to me.

The test should have been a DrupalUnitTestBase (DUTB), since we don't really need a browser/request to build + process + render a form, but we can clean this up later on.

Thanks!

Nice! Great to see these patches to help along the content creation page.

Committed and pushed to 8.x. Yay! :)

I think we need a change notice for this?

Change notice added here: http://drupal.org/node/1891074

Untagging. Please remove the "Needs change notification" tag when the change notice task is complete.

Updated issue summary.