Collapsible fieldsets and vertical tabs do not work without form_builder() (Form API)

Duplicate of #740834: Form elements cannot be rendered without form_builder() ? Just a bit more focused, it seems. Perhaps we should keep 'em separate.

I was able to get it rendered that way:

  $render = array(
    '#type' => 'fieldset',
    '#title' => t('Data selectors'),
    '#pre_render' => array(),
    '#attributes' => array(),
  );
  // Make it manually collapsible as we cannot use #collapsible without the
  // FAPI element processor.
  $render['#attached']['js'][] = 'misc/collapse.js';
  $render['#attributes']['class'][] = 'collapsible';
  $render['#attributes']['class'][] = 'collapsed';

So I wonder whether all of the current collapsible logic shouldn't be moved entirely into a (new) #pre_render for #type 'fieldset' ?

From a technical POV, I think this would make sense. If you'd feed me with enough ice-cream, I'd probably fix vertical tabs very similarly in a few hours, too.

subscribing

I'm resolving #740834: Form elements cannot be rendered without form_builder() currently. But let's keep the general issue of being able to render form elements separate from the two specific collapsible fieldsets + vertical tabs cases, as those are much more complex. I.e., #740834 will only prepare theme_fieldset() to work without form_builder(), but that does not solve the problem that only #pre_render and no #process callbacks are invoked -- which should be solved for the two use-cases here.

Changing title and component accordingly.

It does seem like an easy fix for the fieldsets by simply moving the few lines of code in the #process function into the #pre_render function. This additional behavior is conceptually just a decoration to the element and it has no bearing on functionality.

The patch with limited testing is found here in #740834: Form elements cannot be rendered without form_builder().

Fixing the groups seem like a slightly bigger job, maybe it is worth creating two separate issues to get these fixes in, with minimal changes to the API to keep webchick happy. :)

subscribe

In field_group module, i've got the vertical tabs working (without form_state memory). I'll commit the code very shortly.
So if i can help here in this issue. I don't like ofcourse what i needed to do, but had to see it working for myself.
Screenshot attached.

Happy to review a patch. ;)

@sun well, that's when i need guys like you. What can be done? I see a couple of opportunities but mostly i am fond of what you have said in another issue: let the vertical tabs be "processed" in the theme layer, certainly not in the form.api.

So, since the fieldset stuff is calculated in the form api only AND the bad part, keeping track in the form_state, my first proposition is to move those tracking variables in the form as #properties. Can this be done you think?

But i would be happy to try to write a patch, but i would have to know a couple of things:
- what is covered? vertical tabs and fieldset processing for non-form builds.
- What are the boundaries? I forsee that some code needs to go out form.inc and goes into ... ? common.inc ?
- Can we introduce something? E.g. having the $form_state as special exception of a build_state if you know what i mean. So non-form builds could always have a tracking state object too (or array). Vertical tabs default tab could depend on this.

What do you think?

This is not a patch yet, but i want somebody to walk with me here.
The following array just work as expected while building a non-form.

$element['additional_settings'] = array(
    [#type] => vertical_tabs
    [#theme_wrappers] => Array(vertical_tabs)
    [group_categories] => Array (
            [#type] => fieldset
            [#title] => Categories
            [#collapsible] => 1
            [#collapsed] => 1
            [#attributes] => Array(
               [class] => Array  (
                            [0] => tabs
                            [1] => collapsible
                            [2] => collapsed)
            )
            [#group] => additional_settings
            [#parents] => Array (additional_settings)
            [group_categories] => Array (
                    [field_page_tags] =>  ... this is an element
                    [field_page_tags2] =>  ... this is an element
            )
    [group_links] => Array (
            [#type] => fieldset
            [#title] => Links
            [#collapsible] => 1
            [#collapsed] => 1
            [#attributes] => Array(
               [class] => Array  (
                            [0] => tabs
                            [1] => collapsible
                            [2] => collapsed)
            )
            [#group] => additional_settings
            [#parents] => Array (additional_settings)
            [group_links] => Array (
                    [field_page_links] =>  ... this is an element
                    [field_page_links2] =>  ... this is an element
            )
    [#attached] => Array (
            [js] => Array (
                    [0] => misc/form.js
                    [1] => misc/collapse.js
                    [2] => misc/vertical-tabs.js)
            [css] => Array (
                    [0] => misc/vertical-tabs.css)
        )
)

So what exactly is the key here.
[#parents] => Array (additional_settings) This made it work for non-forms, however, if this property exists on the form build ... i got an endless loop. So a little fix while building the element on my behalf fixes that, no problem there.

So the question remains, if we want to remember default tabs and so for non-form things, we need a way like form properties or an equivalent of form_state for non-form builds.

subscribe

Subscribing

I just hit the same issue again. This was the patch from the mentioned thread above for the fieldset part of this issue.

It is a bit stale as it was created a year ago. Seeing if it applies and passes current tests.

Duh. That patch was really old. Here is D8 git patch

Tagging as novice for the task of rerolling the Drupal 8.x patch on account of #22336: Move all core Drupal files under a /core folder to improve usability and upgrades.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

Hey hey,

attached is the rerolled patch as requested. Let's go testbot!

Thanks @30equals, the reroll looks great!

Can we add an automated test for this? The test would fail without the patch in #20 and pass with it.

Looking into making a test for this issue.

ok, here's a test that checks for the presence of the class "collapsible" which is added after the patch is applied. I'm not sure if this is enough as a test for this issue, so pleas advise.

I'm attaching the patch with tests only (should fail) and the patch from #20 plus the test, and this should pass.

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1821,6 +1821,15 @@ class CommonDrupalRenderTestCase extends DrupalWebTestCase {
     ));
+    ¶
+    $element = array(

Trailing whitespace here (and in the test only patch too, of course)

Not setting back to CNW yet.

thanks @tim.plunkett , I had seen that and corrected it on my local copy but didn't upload a new patch yet. Good to have it on the record though.
Besides that, do you think it's enough to test for the collapsible class or we'd need to check for the inclussion of the javascript as well?

Here's the patch again, with tests and the white space removed.

Thanks @lucascaro. Looks like the patch is malformatted; it has some doubles of some lines--could you try recreating it?

#23: drupal-non_form_fieldsets_rerolled-857124-23.patch queued for re-testing.

w00t!
sorry, I don't know why that happened.. here's a new patch

Alright, I think this test and fix are sufficient. Thanks @lucascaro!

great!

#30: drupal-non_form_fieldsets_rerolled-857124-30.patch queued for re-testing.

Just broken because the file was moved in #1299424: Allow one module per directory and move system tests to core/modules/system, no change.

Thanks. Committed/pushed to 8.x.

This looks backportable to me.

Straight backport.

Tagging for posterity.

The backport matches and there are no UI changes other than "it's not broken."

This looks safe, but since it smells of API change, I'd like to hold it until after this week's release.

Looked this over, and I agree with @webchick that this smells of a possible API change.

For Drupal 7, wouldn't it be possible to duplicate the code in form_pre_render_fieldset(), rather than moving it there? (And by "duplicate" I don't necessarily mean duplicate exactly, of course; rather, "check if X was already added to the array, and if not, add it now", that kind of thing.)

Seems like that would be a safer way to achieve the same effect, no?

A workaround for people who want to render collapsible fieldsets in D7 now: you can make it work by adding the right classes and loading the javascript files:

  $output = array(
    'fieldset' => array(
      '#type' => 'fieldset',
      '#title' => t('Test'),
      '#attributes' => array('class' => array('collapsible', 'collapsed')),
      'content' => array(
        '#markup' => t('Test'),
      ),
    ),
    '#attached' => array('js' => array('misc/collapse.js', 'misc/form.js')),
  );

Looks like we need a patch with the workaround.

I believe this patch is for fieldsets to work without forms right?

Can someone help me regarding where i can find info about making vertical tabs work without form builder?

Thanks a ton,
Sukanya

@sukanya.ramakrishnan I managed to add vertical tabs outside of a form using the following code in a page callback function (invoked through hook_menu()).

I'm guessing the same logic also can be applied in other places, such as preprocess functions etc. since this just uses the render and theming engine.

<?php

// Init.
$html = '';
$fieldsets = '';

// Set content for the first vertical tab.
$my_first_fieldset_content = 'This will be displayed in your tab';

// Wrap the content inside the first fieldset.
$first_fieldset = array(
  '#type' => 'fieldset',
  '#title' => t('Fieldset 1'),
  '#value' => $my_first_fieldset_content,
  '#weight' => 0,
  '#collapsible' => TRUE,
  '#collapsed' => FALSE,
);

// Set content for the second vertical tab.
$my_second_fieldset_content = 'This will be displayed in your tab';

// Wrap the content inside the second fieldset.
$second_fieldset = array(
  '#type' => 'fieldset',
  '#title' => t('Fieldset 1'),
  '#value' => $my_second_fieldset_content,
  '#weight' => 0,
  '#collapsible' => TRUE,
  '#collapsed' => FALSE,
);

// Render the fieldsets.
$fieldsets .= render($first_fieldset);
$fieldsets .= render($second_fieldset);

// Wrap the fieldsets as vertical tabs.
$vertical_tabs = array(
  '#theme' => 'vertical_tabs',
  '#children' => $fieldsets,
);

// Render the vertical tabs.
$html .= render($vertical_tabs);

return $html;

?>

I also think its sad ;) that I cannot use vertical tabs in D8 outside of forms. Whats the current state of this issue?