Posted by Crell on February 13, 2006 at 4:00am
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | forms system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
This wee patch assigns an id to a fieldset that is equal to its #title attribute. That makes it easier to latch onto it for CSSing if you only want to modify a single fieldset.
For instance, I have several multi-select taxonomy categories and want them to line up horizontally to save vertical screen space, but don't want to muck up any other fieldsets. This would allow a simple CSS rule:
#Categories .form-item { float: left; }
I'm pretty sure it doesn't break anything else. :-)
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| fieldset-id.patch | 739 bytes | Ignored: Check issue status. | None | None |
Comments
#1
"ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods (".")." http://www.w3.org/TR/html4/types.html#type-name
I don't think #title will consistently fulfill that at all.
#2
You know, I'm pretty sure this can be done with form_alter these days, so let's just do that and be done with it.
#3
Crell #2: I actually think that this is an issue.. almost every form-item has an id, except for fieldsets, and this is not very consistent.
drumm #1: I have re-written the patch using form_clean_id so that the string is of the correct format.
#4
Should the
drupal_strtolowercall become part of the form_clean_id() function?#5
I believe it should. Although the w3c does not specifically state so, it has become a standard throughout drupal and the web development community in general. I have attached a patch which still adds ids to fieldsets but also alters the form_clean_id function.
#6
I agree with UltimateBoy. It looks to be an unsaid standard in Drupal anyway, so this will just give things that extra level of consistency.
#7
I thought we should be nice to both $element['#id'] and $element['#attributes']['id'] if they already exist.
#8
Ah yes.. good point. Applies cleanly and gets the job done. This is a simple enough patch, so I am marking as rtbc.
#9
Shouldn't it be using the fieldset name instead of the title? Since the title can be translated, it makes the ID kind of useless.
#10
This adds a #key property to all the form elements, and then uses that to construct the ID of fieldsets. I was having a look at #name and #id, but we can't really use those because they're required special use cases for input elements.
#11
I like this patch, but is there a reason why you removed the
drupal_strtolowercall from form_clean_id()?#12
I removed the call to drupal_strtolower from the previous patch because now it's using the element's actual key rather then the element's title. So, there won't be any capitals being passed through it anyway. The call therefore isn't required.
The form element's key is what it's called when using $form['myelement']....The key for $form['myelement'] is "myelement".
#13
The last submitted patch failed testing.
#14
Linking the fieldset id to the title is not recommended IMO. What if the site is multilanguage? A better approach would be using a static variable in the theme_fieldset method that increments itself every time a fieldset is placed. This is what I came out with (tested D6)
function theme_fieldset($element) {
static $fieldsetNum = 0;
if (!empty($element['#collapsible'])) {
drupal_add_js('misc/collapse.js');
if (!isset($element['#attributes']['class'])) {
$element['#attributes']['class'] = '';
}
$element['#attributes']['class'] .= ' collapsible';
if (!empty($element['#collapsed'])) {
$element['#attributes']['class'] .= ' collapsed';
}
}
$element['#attributes']['class'] .= " fieldset-".$fieldsetNum++;
return '<fieldset'. drupal_attributes($element['#attributes']) .'>'. ($element['#title'] ? '<legend>'. $element['#title'] .'</legend>' : '') . (isset($element['#description']) && $element['#description'] ? '<div class="description">'. $element['#description'] .'</div>' : '') . (!empty($element['#children']) ? $element['#children'] : '') . (isset($element['#value']) ? $element['#value'] : '') ."</fieldset>\n";
}
Cheers
#15
The patch I posted does not use the title, it uses the form's key name instead, which is always the same no matter if the site is multilingual. The form_clean_id function that is used in the patch manages that static variable numbering that you're talking about for us.
#16
Re-roll.
#17
looks nice to me. bot is happy so rtbc
#18
I wonder if it makes sense to prefix the ID with something. I imagine if you named your fieldset 'messages' or 'content' or something similar we'd end up with duplicate IDs on the page.
I'd also love to get a FAPI guru to ensure the additions to form_builder() are okay.
#19
form_clean_id() ensures uniqueness and is used in this patch.
#20
That's some mighty convoluted patch. Why are we not setting #id right where this patch hands down key instead? This would give all #input FALSE an #id.
#21
#680196: Add specific classes to vertical tab li elements
#22
form_process_fieldset injects:
<?php$element['#attributes']['id'] = $element['#id'];
?>
#23
Automatically closed -- issue fixed for 2 weeks with no activity.
#24
Not sure which patch had a green light, but what I did is put the div id equal to class. To do that, I inserted the following code in lines 1515-1517 in form.inc:
if (!isset($element['#attributes']['id'])) {$element['#attributes']['id'] = 'fieldset-' . $element['#attributes']['class'];
}
I'm not a programmer so I'm not sure if it's the best way, but it doesn't rely on the #title so there will be no problems with i18n.