Make a consistent wrapper around submit buttons
yoroy - June 5, 2009 - 09:32
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | theme system |
| Category: | task |
| Priority: | normal |
| Assigned: | sun |
| Status: | reviewed & tested by the community |
| Issue tags: | API clean-up, d7ux-design-question, markupmarines, needs themer review |
Description
I'll quote part of a comment from #116939: Add a Cancel button on forms that triggered this one:
Form buttons should be wrapped in an additional container to achieve a consistent styling, but also allow for a "Cancel" link, as contained in confirmation forms (which use additional tweaks to get the styling right). Wrapping form buttons into a parent element 'actions' (or similar) also allows for additional styling, for example, applying CSS Sliding Doors to buttons and links to achieve a more modern look.
(read #43 to #47 in the other issue for the full backstory, it has sun, quicksketch and me agreeing enthousiastically :-)
#1
subscribing
#2
asked around a bit on how this would work, chx and yhahn replied:
currently a form element has no knowledge of its children
we can add a #type 'buttom-wrapper'
and then have $form['buttons']['#type'] = 'button-wrapper';
and put all the relevant submit buttons under that element
and then $form['buttons']['submit'] = array(....)
if you hit system_settings_form, node / add / edit, and a few other you've covered a good portion of core forms
and then you can theme_button_wrapper as much as you want,.
#3
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
#4
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)'.
#5
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 button 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:<?php$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:
<?php$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()));
?>
#6
This would be awesome!
#7
Introducing a new tag for feature freeze: API clean-up.
#8
#557272: FAPI: JavaScript States system introduces a "container" type. So instead, you could have:
<?php$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()));
?>
#9
Looking for this feature. Thanks
#10
#622432: DX: Start using the "container" form type demonstrates use of this.
#11
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.
#12
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.
#13
The last submitted patch failed testing.
#14
yay, thanks!
That was still quite inconsistent though... So let's go with this instead? :)
#15
This one should fix the exceptions.
#16
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.
#17
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.
#18
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.
#19
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.
#20
#21
Definitely like the clean up of the theme functions. Unless there's anything else missing from this patch, I'm setting this to RTBC.
#22
I'm just glad I opened this issue :-)
Sure hope to see this committed.
#23
This patch is backed 100% by themers.
Yes, we want. :)
#24
Nice. This is a sensible improvement