Make a consistent wrapper around submit buttons

subscribing

This would be pretty sweet and we could then remove the "unnecessary" div inside the forms ( http://drupal.org/node/495480)+ would give us some better control over the submit, delete cancel button (if they are put in the bottom of the page)

+1

I totally agree..
Let's combine it together with #165567: Usability: Float default submit buttons to the right and the other to the left which I submitted ages ago. Let the buttons which reflect the default action of the page ("Save $foo", etc) on the right (when the theme is LTR, or the other way around when it's RTL) and the other buttons on the left (like "Cancel", or "Reset to defaults)'.

Sorry, but no. Please stay on focus. Form buttons can appear everywhere in a form, see admin/settings/performance as an example. This issue should focus on wrapping form buttons into a container - to allow for consistent styling. If you mix this issue with any other, I can promise you it won't make it into D7.

#theme_wrapper might work out. Maybe it also works with some magic applied, but probably not. (see also example hack in #116939-48: Add a Cancel link on forms)

Ultimately, a consistent wrapper for form buttons will probably depend on developers implementing it (i.e. '#theme_wrapper' => 'form_actions'). However, to do that, module developers might have to rework their form structure - referring to aforementioned hack again:

  $form['preview'] = array('#type' => 'button', '#value' => t('Preview'));
  $form['submit'] = array('#type' => 'submit', '#value' => t('Save'));
  $form['cancel'] = array('#markup' => l(t('Cancel'), referer_uri()));

becomes:

  $form['actions'] = array('#theme_wrapper' => 'form_actions');
  $form['actions']['preview'] = array('#type' => 'button', '#value' => t('Preview'));
  $form['actions']['submit'] = array('#type' => 'submit', '#value' => t('Save'));
  $form['actions']['cancel'] = array('#markup' => l(t('Cancel'), referer_uri()));

This would be awesome!

Introducing a new tag for feature freeze: API clean-up.

#557272: FAPI: JavaScript States system introduces a "container" type. So instead, you could have:

  $form['actions'] = array('#type' => 'container');
  $form['actions']['preview'] = array('#type' => 'button', '#value' => t('Preview'));
  $form['actions']['submit'] = array('#type' => 'submit', '#value' => t('Save'));
  $form['actions']['cancel'] = array('#markup' => l(t('Cancel'), referer_uri()));

Looking for this feature. Thanks

#622432: DX: Start using the "container" form type demonstrates use of this.

I like rob's approach of using a simple container.

For another use case...

In jQuery UI dialog, you can specify a buttons object, which the Dialog will display in a consistent area, at the bottom left of the dialog. Having a consistent wrapper would allow you to hide the default buttons and delegate the submission of the form to the jQuery UI dialog buttons. Without a consistent wrapper though, there is no way to do this reliably.

Checkout the d6 version of dialog api (http://drupal.org/project/dialog) for an example of this. In order to achieve this in d6 though, i've resorted to form_altering a class onto the buttons that I want to use.

This adds a container around all form buttons with a class named "form-actions". We need this in, badly. Those lost form buttons drive me nuts. I'm probably missing some buttons here, so some reviews would be nice.

yay, thanks!

That was still quite inconsistent though... So let's go with this instead? :)

This one should fix the exceptions.

So what's left?

+++ modules/user/user.admin.inc	10 Dec 2009 00:59:00 -0000
@@ -72,16 +72,17 @@ function user_filter_form() {
-  $form['filters']['buttons']['submit'] = array(
+  $form['filters']['actions'] = array('#type' => 'container', '#attributes' => array('class' => array('form-actions')));
+  $form['filters']['actions']['submit'] = array(
     '#type' => 'submit',
     '#value' => (count($session) ? t('Refine') : t('Filter')),
   );

@@ -916,7 +920,7 @@ function theme_user_filters($variables) 
-  $output .= '<div class="container-inline" id="user-admin-buttons">' . drupal_render($form['buttons']) . '</div>';
+  $output .= '<div class="container-inline" id="user-admin-buttons">' . drupal_render($form['actions']) . '</div>';

We want to consolidate extra DIVs like this one. I.e. we can apply the #id and extra .container-inline class to the container's #attributes already :)

Aside from that, this looks very RTBC-ish!

This review is powered by Dreditor.

To make this change, we needed to add support for the "#id" property....

@@ -72,16 +72,21 @@
     );
   }
 
-  $form['filters']['buttons']['submit'] = array(
+  $form['filters']['actions'] = array(
+    '#type' => 'container',
+    '#attributes' => array('class' => array('form-actions', 'container-inline')),
+    '#id' => 'user-admin-buttons',
+  );
+  $form['filters']['actions']['submit'] = array(
     '#type' => 'submit',
     '#value' => (count($session) ? t('Refine') : t('Filter')),
   );
@@ -916,7 +924,7 @@
   $output .= '</dd>';
 
   $output .= '</dl>';
-  $output .= '<div class="container-inline" id="user-admin-buttons">' . drupal_render($form['buttons']) . '</div>';
+  $output .= drupal_render($form['actions']);
 
   return $output;

I also noticed we were missing some buttons in Locale module, so I added those.

Merged that last patch with mine, as we worked on this at the same time.

Additionally:

- Removed totally senseless theme functions for filter forms.

- Applied consistent pattern: One-line syntax for 'actions' if it's the regular thing, multi-line if there are special tweaks contained (such as .container-inline)

Furthermore, I double-checked the actual output of most forms.

Looks ready to fly for me.

Very interesting effect, when you remove a theme function without the registry entry in hook_theme(). Is that a bug elsewhere?

Anyway, this one fixes the notices.

Definitely like the clean up of the theme functions. Unless there's anything else missing from this patch, I'm setting this to RTBC.

This patch is backed 100% by themers.

Yes, we want. :)

Nice. This is a sensible improvement

Re-rolled against HEAD.

Hm. This came after code freeze and is not fixing a critical bug. OTOH, I noticed Dries committed a patch that added such a container to node forms, so I suppose we could do this too, to be consistent....

Ok. Committed to HEAD.

Needs to be documented in the module upgrade guide as a new standard.

Thank you! This is really exciting and will drastically improve #D7TX! TX? Theming experience! ;)

On admin/content 'Filter' now has a container, but 'Update' does not.

I think buttons at the bottom of a form should have a different class so they can have a margin-top.

Adding margin-top to buttons within the form screws up the layout.

There may be some more buttons to wrap, searching for $form['submit'] or $form['save'] in all files reveals a bunch of them.
But there may be reasons for those to have been left unwrapped ?

Nope, not intentionally. It's also possible that forms were changed in the meantime, without knowledge of this new standard here.

Subscribing, how can I help?
Btw, grep -Ern "\['(submit|save|preview|delete|undo|reset|)'\] = " . | grep -Fv -e "['actions']" gives the below list of hits (minus a comment or two). Should all those be wrapped?

Whoa, list was a lot longer than I thought. Attaching instead.

When that gets in, what do you think about making form['actions'] weight to 100 everywhere by default ?

This has been done for system_settings_form() in #645758: system_settings_form() should set a high weight for buttons, so it would be the good place for that.

subscribe

I have absolutely no idea how all of those were able to slip through.

Attached patch should fix all of them.

Sorry, one wrong change in menu.admin.inc.

#38: drupal.form-actions.38.patch queued for re-testing.

This one should fix the failing tests.

The $form['actions'] keeps getting collapsed onto one line when it shouldn't. It's an array that exceeds 80 characters, so it should be wrapped properly.

Plus theres a few places where it's even changed unnecesssarily:
+++ modules/contact/contact.pages.inc	17 Feb 2010 12:13:12 -0000
@@ -235,10 +235,7 @@ function contact_personal_form($form, &$
-  $form['actions'] = array(
-    '#type' => 'container',
-    '#attributes' => array('class' => array('form-actions')),
-  );
+  $form['actions'] = array('#type' => 'container', '#attributes' => array('class' => array('form-actions')));

Here

+++ modules/path/path.admin.inc	17 Feb 2010 00:22:10 -0000
@@ -122,10 +122,7 @@ function path_admin_form($form, &$form_s
-  $form['actions'] = array(
-    '#type' => 'container',
-    '#attributes' => array('class' => array('form-actions')),
-  );
+  $form['actions'] = array('#type' => 'container', '#attributes' => array('class' => array('form-actions')));

Here as well.

Powered by Dreditor.

@Dave: See #18 for the reasoning.

I see the reasoning but it still doesn't make sense why it needs to violate a coding standard. We don't gain anything by having it on one line.

@Dave: AFAIK, there is no such coding standard. The only remotely related is "wrap at 80 chars", but that doesn't necessarily apply to form arrays. I think the one-line notion makes sense in this case, especially provided the reasoning above.

Testbot seems to be stuck in the retest request, so I'm re-attaching the last patch.

Sorry sun. I hate to fight about a minor detail of an important patch, but standards matter to me. :/

http://drupal.org/coding-standards#array:

Note that if the line declaring an array spans longer than 80 characters (often the case with form and menu declarations), each element should be broken into its own line, and indented one level:

I've noticed that as an unwanted side effect, this makes it difficult to inject new stuff above the actions.... Check this out:

function mymodule_form_block_admin_configure_alter(&$form, &$form_state) {
  $form['Additional Awesome'] = array(
    '#type' => 'fieldset',
    '#title' => t('Additional Awesome'),
    '#description' => t('Choose how awesome this block is...'), 
    '#collapsed' => TRUE,
    '#collapsible' => TRUE,
    // Not putting this in will place it after the buttons.
    // Putting in -1 will put it all the way at the top of the page.
    // Putting 1 will place it below the buttons.
    '#weight' => -1,
  );
}

Hmmm, maybe we need a consistent #weight that's applied to the action buttons?

I agree. Let's come back to the previous patch after ruling this out.

Did I forget anything in here?

RTBC?

oh YAY! This revealed a quite critical bug in form API! :)

Prior to handling, to is missing.

And http://webchick.net/patch-reviewers-are-not-clairvoyant

+1 for a high default weight, see #35.

@chx : something missing in your comment ?

#50: drupal.form-actions.50.patch queued for re-testing.

Great idea guys. I'm looking for a way to make a wrapper in D6. I'm using image replacement for buttons quite a lot using this technique: http://drupal.org/node/692782#comment-2508628 . It works great, except is difficult to theme the button positions when their positions have to be set to 'absolute'. This is why I need a wrapper around buttons.

Thumbs up!

@Danny_Joris: http://drupal.org/project/button_style

- Fixed the grammar in that inline comment.

- Fixed previously missing/new occurrences of $form['submit']. Entirely left out test modules though.

Fixed that failing test.

Checked a bunch of forms and the wrapper is there. Setting this to RTBC.

Committed to CVS HEAD.

Couldn't find the moving the node submit buttons to the actions array committed by Dries (mentioned by webchick in #28), but i think documentation can be covered here.

Added the below to Converting 6.x modules to 7.x.

Form submit buttons consistently grouped in actions array

(Issue) All forms in Drupal now have an actions array to hold the buttons at the bottom of forms relating to submission. Use this in forms provided by contributed modules also. In node forms, this replaces the submit grouping array, which modules implementing hook_form_alter() or hook_form_FORM_ID_alter() to modify, remove, or add to these buttons will have to take into account.

For instance, this is an edited excerpt from node.pages.inc's adding a Save button to the node submission form:

  $form['actions'] = array('#type' => 'actions');
  $form['actions']['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Save'),
    '#submit' => array('node_form_submit'),
  );

Removing Needs Documentation tag but please review this documentation!