theme_form_element seems to have reversed conditionals [#90687]

Comment #1

he/him

commented 23 October 2006 at 23:04

Here's the offending code from 4.7.4:

function theme_form_element($title, $value, $description = NULL, $id = NULL, $required = FALSE, $error = FALSE) {

  $output  = '<div class="form-item">'."\n";
  $required = $required ? '<span class="form-required" title="'. t('This field is required.') .'">*</span>' : '';

  if ($title) {
    if ($id) {
      $output .= ' <label for="'. form_clean_id($id) .'">'. t('%title: %required', array('%title' => $title, '%required' => $required)) . "</label>\n";
    }
    else {
      $output .= ' <label>'. t('%title: %required', array('%title' => $title, '%required' => $required)) . "</label>\n";
    }
  }

  $output .= " $value\n";

  if ($description) {
    $output .= ' <div class="description">'. $description ."</div>\n";
  }

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

  return $output;
}

It looks okay to me. If there isn't a title, then it can't generate a label at all. If there is a title, but not an "id" attribute, then the label tag is generated but doesn't include a for="id-of-element-here". Is this not the problem described?

Log in or register to post comments

Comment #2

bmargulies commented 24 October 2006 at 00:53

If there's no title, why not put the id on what gets created? Here's what I put in my theme, since lots of elements in a webform end up title-less (like checkboxes). I confess that I'm not at all clear on what the use of the 'for' attribute is in a label.


// we override this to get css-able id's on the elements.
function phptemplate_form_element($title, $value, $description = NULL, $id = NULL, $required = FALSE, $error = FALSE) {

    if($id) {
        $output  = '<div class="form-item" id="form-item-' . form_clean_id($id) . '">'."\n";
    } else {
        $output  = '<div class="form-item">'."\n";
    }
  $required = $required ? '<span class="form-required" title="'. t('This field is required.') .'">*</span>' : '';

  if ($title) {
    if ($id) {
      $output .= ' <label for="'. form_clean_id($id) .'">'. t('%title: %required', array('%title' => $title, '%required' => $required)) . "</label>\n";
    }
    else {
      $output .= ' <label>'. t('%title: %required', array('%title' => $title, '%required' => $required)) . "</label>\n";
    }
  }

  $output .= " $value\n";

  if ($description) {
    $output .= ' <div class="description">'. $description ."</div>\n";
  }

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

  return $output;
}

Log in or register to post comments

Comment #3

bmargulies commented 24 October 2006 at 00:55

Sorry, I should also have noted that you correctly diagnosed that my complaint was not well-formed. I should have titled 'if you can't id a label, how about id-ing the div'?