twitter_bootstrap_form_element does not properly theme radios/checkbox sets [#1451510]

Hi again,

twitter_bootstrap_form_element is adding too many controls and control-groups to radio elements and checkboxes. The problem is that this theme function is called for both the group of elements and each individual element.

We should take this into account and adding the TB classes for the individual elements in a set.

Thanks!

Corrected function

function twitter_bootstrap_form_element(&$variables) {
  $element = &$variables['element'];
  // This is also used in the installer, pre-database setup.
  $t = get_t();

  // This function is invoked as theme wrapper, but the rendered form element
  // may not necessarily have been processed by form_builder().
  $element += array(
    '#title_display' => 'before',
  );

  // Add element #id for #type 'item'.
  if (isset($element['#markup']) && !empty($element['#id'])) {
    $attributes['id'] = $element['#id'];
  }

  // Add bootstrap class
  $group = !($element['#type'] == "radio" || $element['#type'] == "checkbox");
  if ($group) {
    $attributes['class'] = array('control-group');
  }

  // Check for errors and set correct error class
  if (isset($element['#parents']) && form_get_error($element)) {
    $attributes['class'][] = 'error';
  }

  if (!empty($element['#type'])) {
    $attributes['class'][] = 'form-type-' . strtr($element['#type'], '_', '-');
  }
  
  if (!empty($element['#name'])) {
    $attributes['class'][] = 'form-item-' . strtr($element['#name'], array(' ' => '-', '_' => '-', '[' => '-', ']' => ''));
  }
  // Add a class for disabled elements to facilitate cross-browser styling.
  if (!empty($element['#attributes']['disabled'])) {
    $attributes['class'][] = 'form-disabled';
  }
  $output = '<div' . drupal_attributes($attributes) . '>' . "\n";

  // If #title is not set, we don't display any label or required marker.
  if (!isset($element['#title'])) {
    $element['#title_display'] = 'none';
  }
  $prefix = isset($element['#field_prefix']) ? '<span class="field-prefix">' . $element['#field_prefix'] . '</span> ' : '';
  $suffix = isset($element['#field_suffix']) ? ' <span class="field-suffix">' . $element['#field_suffix'] . '</span>' : '';

  switch ($element['#title_display']) {
    case 'before':
    case 'invisible':
      $output .= ' ' . theme('form_element_label', $variables);
      if ($group) {
        $output .= '<div class="controls">';
      }
      $output .= ' ' . $prefix . $element['#children'] . $suffix . "\n";
      break;

    case 'after':
      if ($group) {
        $output .= '<div class="controls">';
      }
      $variables['#children'] = ' ' . $prefix . $element['#children'] . $suffix;
      $output .= ' ' . theme('form_element_label', $variables) . "\n";
      break;

    case 'none':
    case 'attribute':
      // Output no label and no required marker, only the children.
      if ($group) {
        $output .= '<div class="controls">';
      }
      $output .= ' ' . $prefix . $element['#children'] . $suffix . "\n";
      break;
  }

  if ( !empty($element['#description']) ) {
    $output .= '<p class="help-block">' . $element['#description'] . "</p>\n";
  }

  if ($group) {
    $output .= '</div>';
  }

  $output .= "</div>\n";

  return $output;
}

Comment	File	Size	Author
#17	bootstrap-exclude-control-wrapper-checkboxes-radios-1451510.patch	1.93 KB	natted
#17
#3	D7.Tbs_.Quiz_.CheckboxFail.jpg	657.71 KB	Drubuntu7User

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

nagiek CreditAttribution: nagiek commented 23 February 2012 at 05:00

Actually, not sure if this correctly fixes checkbox sets, as you can have a standalone checkbox.

Does correctly fix radios though, as there is a radios/radio relation.

Comment #2

nagiek CreditAttribution: nagiek commented 23 February 2012 at 05:12

Same sort of change also required in twitter_bootstrap_form_element_label. See below.

Corrected function.

function twitter_bootstrap_form_element_label(&$variables) {

  ...

  // @Bootstrap: Add Bootstrap control-label class.
  $group = ($element['#type'] != "radio");
  if ($group) {
    $attributes['class'][] = 'control-label';
  }

Comment #3

Drubuntu7User CreditAttribution: Drubuntu7User commented 23 April 2012 at 18:21

Priority:

Normal

» Major

File	Size
D7.Tbs_.Quiz_.CheckboxFail.jpg	657.71 KB

Hello,
I am currently putting the finishing touches on a Drupal based Learning Management System this week and I have run into a brick wall with this issue which I can't seem to resolve on my own. If anyone is available to help with a patch for this issue it would be most greatly appreciated!

Site, Module, & Theme Versions:

Drupal 7.12 Site

Quiz 7.x-4.0-alpha4 Module

Twitter bootstrap 7.x-2.x-dev Theme

7.x-2.0-alpha2+1-dev Libraries

7.x-2.x-dev Twitter Bootstrap UI

The checkboxes function perfectly for me when using the Garland theme. But when the Twitter Bootstrap Theme is enabled the quiz multiple choice questions do not display the check boxes. The attached image shows Chrome's inspected element on the Checkbox for Garland on the left & Twitter Bootstrap on the right.

As you can see it seems to have to do with twitter using <div class="controls"></div> in place of where Garland uses <input type="checkbox" id="edit-tries-answer-3" name="tries[answer][3]" value="3" class="form-checkbox">

Copying the code from the Garland theme right into Google Chrome to replace the <div class="controls"></div> tag on the bootstrap theme does temporarily resolve the issue on screen, however doesn't address the underlying issue that makes the issue persist.

After a few days of pulling my hair out I stumbled on this thread and was relieved in knowing that I wasn't alone with this problem. Unfortunately though I have tried nagiek's two posts and I still turn up empty.

If there is anyone out there who can help bail me out of this rut I'm stuck in for this error I would be so grateful!!!

Thank you

D7TBSQuizCheckBoxBug

Comment #4

Drubuntu7User CreditAttribution: Drubuntu7User commented 24 April 2012 at 01:37

Category:	bug	» support
Priority:	Major	» Critical

Comment #5

STCilla CreditAttribution: STCilla commented 24 April 2012 at 22:33

I was really disappointed too when I was trying to use bootstrap on my first Drupal site when I came across this issue, looks like I'm going to end up going Joomla after all...

Comment #6

Drubuntu7User CreditAttribution: Drubuntu7User commented 9 May 2012 at 21:50

Hello,
I'm just following up on this issue. I have tried to replicate the radio box code referenced above for the checkboxes but still am not able to make any progress. Has anyone else been able to resolve this issue?

Comment #7

gagarine CreditAttribution: gagarine commented 22 May 2012 at 13:15

@STCilla Please post useful comment than bring more information about the issue. This is not a forum. (for you first drupal website I will advice you to use a theme with a stable release... )

Comment #8

yuriy.babenko CreditAttribution: yuriy.babenko commented 20 June 2012 at 18:28

@Drubuntu7User - the issue you're experiencing is a different bug, which I ran into earlier today and fixed in this issue: #1649392: Checkbox or radio inputs without titles don't render

Comment #9

jim_at_miramontes CreditAttribution: jim_at_miramontes commented 11 August 2012 at 15:39

Subscribe -- the patches are mostly working for me. Aside from cleaner HTML, the main issue I was running into was that the extra markup was breaking the standard LESS way of doing inline checkboxes and radio buttons. The new functions and a bit of css, on the order of --

.form-type-checkbox, .form-type-radio {
	display: inline-block;
}

got things working; I'm not sure if there's a more systematic way of doing this or not. Thanks for the theme in any case!

Comment #10

andregriffin CreditAttribution: andregriffin commented 11 November 2012 at 05:11

Project:

Twitter's Bootstrap

» Bootstrap Framework

Comment #11

andregriffin CreditAttribution: andregriffin commented 11 November 2012 at 19:50

Project:

Bootstrap Framework

» Twitter's Bootstrap

Comment #12

natted CreditAttribution: natted commented 13 November 2012 at 23:54

Project:

Twitter's Bootstrap

» Bootstrap

Comment #13

frankbaele CreditAttribution: frankbaele commented 1 December 2012 at 09:08

Priority:	Critical	» Minor
Status:	Active	» Closed (won't fix)

Comment #14

nagiek CreditAttribution: nagiek commented 1 December 2012 at 19:17

Priority:	Minor	» Normal
Status:	Closed (won't fix)	» Active

Um, I'm sorry. Minor? Won't fix?

Justification please, because it's obviously broken.

Comment #15

frankbaele CreditAttribution: frankbaele commented 2 December 2012 at 09:45

Ok you want to help us fix this, just add a use-case where the radio/checkboxes break and maybe a patch for the current dev if you can/want to help us with the code part.

Comment #16

frankbaele CreditAttribution: frankbaele commented 3 December 2012 at 08:16

ok, first off sorry, i didn't read you post well, because off all the post hijacking with unrelated issues. Now i see what you meant in your orignal post and i see the issue. But i can hardly recover working code from you original post because the file changes in theme. So could you re roll your patch

thanks in advance

frank

Comment #17

natted CreditAttribution: natted commented 11 December 2012 at 02:59

Status:

Active

» Needs review

File	Size
bootstrap-exclude-control-wrapper-checkboxes-radios-1451510.patch	1.93 KB

Ok, I'm testing the attached patch which includes adjustments as per #1 (although executed in a slightly different manner).

Next I will look into #1469182: Allow control-group in theme_form_element to be overriden and hope this will resolve both issues.

Comment #18

natted CreditAttribution: natted commented 11 December 2012 at 08:27

Status:

Needs review

» Fixed

Committed to dev along with a patch for #2

Comment #19

25 December 2012 at 08:30

Status:

Fixed

» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Comment #20

skriptble CreditAttribution: skriptble commented 22 October 2013 at 22:25

Version:	7.x-2.x-dev	» 7.x-3.x-dev
Status:	Closed (fixed)	» Active

This issue seems to be occurring in the Bootstrap 3, in that it wraps a div around the radio or checkbox elements in a table select. This causes the radios or checkboxes to render as small and unelectable. I'm not exactly sure how to go about it, but removing the wrapping div makes the radios/checkboxes work correctly again. Also, adding text to the label tag makes the radios/checkboxes function again.

example:
NOT WORKING:

<div>
  <td>
    <label for="example-radio-button"> <input type="radio" id="example-radio-button name="example-radio-button" value="example_radio" class="form-radio"></label> 
  </td>
</div>

WORKING:

<td>
 <label for="example-radio-button"> <input type="radio" id="example-radio-button name="example-radio-button" value="example_radio" class="form-radio"> </label>
</td>

Comment #21

markhalliwell

English

CreditAttribution: markhalliwell commented 22 October 2013 at 22:32

Version:	7.x-3.x-dev	» 7.x-2.x-dev
Status:	Active	» Closed (fixed)

Please don't open existing issues that have already been closed and fixed. Create a new issue with references to where you think it was introduced, which isn't this issue (7.x-2.x).