Comments

bryancasler’s picture

Was wondering why my webform components looked jacked. From what I can tell...

Anywhere a "webform-component" is we need "form-group"

acbramley’s picture

+1 on this, FAPI forms have correctly added .form-group but not webform. I guess we could add logic to _bootstrap_process_input or _bootstrap_process_element to add more classes in the same way as it's adding stuff at the moment.

By using the Styleguide module, I was able to see that all normal form components get the .form-group class EXCEPT singular checkbox fields. I'll look into a way of patching this and upload something soon.

acbramley’s picture

More digging, the form-group class is added in bootstrap_form_element() in form-element.func.php, the only webform elements that run through this are date selects, radios, and checkboxes. The strange thing is actual select lists don't pass through here, nor do text fields or text areas.

The Webform module renders these other elements through theme_webform_element, trying to come up with a good way to override this and add the wrapper classes.

One way we could do this would be bootstrap_preprocess_webform_element(), however there's no way we could currently add classes into $variables and get them added to the wrapper output of the element. This is due to webform explicitly rendering out hard coded wrapper classes in theme_webform_element (line 2710 webform.module). I propose we submit a patch to webform.module allowing a #wrapper_attributes key to be added to the element array which is merged with the $wrapper_classes in theme_webform_element() and output using drupal_attributes().

Turns out this wrapper_attributes is available in webform 4.x :D https://drupal.org/comment/7487060#comment-7487060

acbramley’s picture

Ok, I'm at a loss, there's some really odd things going on with the rendering of the different types of components. I've got a preprocess hook working for all of the components and it's adding wrapper classes correctly for everything _except_ textfields and text areas. These for some reason don't seem to run through the preprocess function even though they are being rendered by the same function as the other components.

Even stranger is if I clear cache, the textfields and textareas are being run through the preprocess function but only for that first request on a fresh cache.

Turns out this is due to something with the inclde and file loading as I can put the preprocessor in template.php and it works perfectly.

acbramley’s picture

StatusFileSize
new878 bytes

The bug mentioned above can be seen by applying this patch, installing webform 4.x and creating a form with textfields and textareas. Clearing cache and reloading the page will add the form-group class, but subsequent requests won't run the preprocess function.

markhalliwell’s picture

Status: Active » Needs work
index edb6a7e..5e9530a 100644
--- a/theme/system/form-element.func.php

@@ -4,6 +4,26 @@
+function bootstrap_preprocess_webform_element(&$variables) {

This function should be created in added in a new .vars.php file specific to the module, not injected into this one:
/theme/webform/webform-element.vars.php

acbramley’s picture

StatusFileSize
new914 bytes

@Mark Carver, thanks a lot for the fix that worked a treat. I guess I should've looked into how bootstrap was loading files a bit more. I would have thought that it the preprocess function was included on the page load then all fields would've happily been passed through it no matter where the function was placed. Here's the patch anyway, thanks again!

acbramley’s picture

Status: Needs work » Needs review
markhalliwell’s picture

Status: Needs review » Needs work

Sorry, one last thing I just saw:

+++ b/theme/webform/webform-element.vars.php
@@ -0,0 +1,28 @@
+  $wrapper_attributes = array();
...
+  $variables['element']['#wrapper_attributes'] = $wrapper_attributes;

This effectively overrides any existing #wrapper_attributes that may have been provided by a module before reaching the theming level. While there may not be any current wrapper attributes, it's best if we account for the possibility in the future.

Something like:

$wrapper_attributes = array();
if (isset($element['#wrapper_attributes']) {
  $wrapper_attributes = $element['#wrapper_attributes'];
}
acbramley’s picture

Status: Needs work » Needs review
StatusFileSize
new1 KB

Ah of course, good catch

  • Commit 21dc9b4 on 7.x-3.x authored by acbramley, committed by Mark Carver:
    Issue #2150725 by acbramley | Lasac: Webforms are missing .form-group.
    
markhalliwell’s picture

Version: 7.x-3.x-dev » 8.x-3.x-dev
Assigned: Unassigned » ryan.armstrong
Category: Bug report » Feature request

  • Commit f23dcec on 7.x-3.x by Mark Carver:
    Issue #2150725 by acbramley | Lasac: Webforms are missing .form-group....

Status: Needs review » Needs work

The last submitted patch, 10: 2150725-webform-element-form-group-class-10.patch, failed testing.

  • Commit 21dc9b4 on 7.x-3.x, 8.x-3.x authored by acbramley, committed by Mark Carver:
    Issue #2150725 by acbramley | Lasac: Webforms are missing .form-group.
    
  • Commit f23dcec on 7.x-3.x, 8.x-3.x by Mark Carver:
    Issue #2150725 by acbramley | Lasac: Webforms are missing .form-group....
acbramley’s picture

Status: Needs work » Closed (fixed)

Silly test bot, this is fixed.

DieterAtWork’s picture

I'm using 7.x-4.1 and am experiencing this issue. The form-group class is missing. So this issue isn't solved. (or broken again)
Sorry, it's the webforms module that's the problem...

hkirsman’s picture

There was a typo in 2150725-webform-element-form-group-class-10.patch on line 13.

if (isset($element['#wrapper_attributes']) {
-->
if (isset($element['#wrapper_attributes'])) {

neardark’s picture

@hkirsman I'm showing this fixed in the latest dev branch.