Field labels are mostly untouched from the original Views theme. I don't use labels on this kind of display, but surely someone does. How could we do this better?

CommentFileSizeAuthor
#6 semantic_labels-572370-6.patch4.86 KBAnonymous (not verified)
#5 semantic_labels-572370-5.patch35 bytesAnonymous (not verified)
#1 semantic_labels-572370-1.patch4.97 KBAnonymous (not verified)

Comments

Anonymous’s picture

Status: Active » Needs work
StatusFileSize
new4.97 KB

I don't like this patch one bit, but I'm posting it to provoke feedback.

I don't know the ways people might use HTML elements other than label to wrap the label of their field. The most obvious semantic alternative would be to use definition lists, but this requires a different structure than the current template allows. At some point, I may be making the theme too complex once again.

seutje’s picture

http://www.w3schools.com/tags/tag_label.asp

The tag defines a label for an input element.

http://www.w3.org/TR/html401/interact/forms.html#h-17.9

Some form controls automatically have labels associated with them (press buttons) while most do not (text fields, checkboxes and radio buttons, and menus).

For those controls that have implicit labels, user agents should use the value of the value attribute as the label string.

The LABEL element is used to specify labels for controls that do not have implicit labels,

http://www.w3.org/TR/WCAG10-HTML-TECHS/#forms-labels

Associate labels explicitly with their controls. [Priority 2]

&

A label is implicitly associated with its form control either through markup or positioning on the page.

a label without an input will probably confuse users and in my personal opinion, a label without a for attribute is pretty much worthless fluff

I'm not trying to just tear down your proposal, just wanna point out that this is not the purpose of the <label> tag and there's a very good chance it will have more negative impact than it would have a positive one

frankly, I think this isn't related to semantics but accessibility, as these labels are usually read by a screenreader when the user tabs into the field and they act as a trigger for the element they are bound with (user clicks label, input gets focussed or changes)

unfortunately, I don't have another suggestion, apart from maybe using a heading of a high type (like <h5> or <h6>)

Anonymous’s picture

seutje:

The label is an artifact from the fork of Drupal Views fields output row style, and I agree that it's a funny usage of label though it isn't invalid HTML it's just bad semantics.

Either headings or (for some view themes) definition list terms? I should bring out the "label" element in the row style options (so users can use dt, label, headings, etc.), let users choose a class attribute too? I can't support 'for' even if I tried, because that would require each field to have a unique ID. Right?

seutje’s picture

oh I'm sorry, I just saw the example on the project page and the first thing that sprung to mind was "what the hell, a label for a... span?" and then I noticed this in the patch in #2:

+        $this->options['semantic_html'][$field] : array(
+          'label' => array('element_type' => 'label', 'class' => ''),
+          'content' => array('element_type' => 'div', 'class' => '')
+        );

but I didn't notice this was mearly a default setting so my apologies for jumping to conclusions

I think a <dl> would be rather hard to implement properly because the element for the label would then be <dt> and the item itself would be <dd> and then the entire row would have to be wrapped in a <dl>, so a row would look like

<dl>
  <dt>label1</dt>
  <dd>value1</dd>
  <dt>label2</dt>
  <dd>value2</dd>
</dl>

so that would mean that this checkbox (?) would override all the label and content element_type settings

this would possibly lead to other problems, imagine the following row with a single label & field:

<dl>
  <dt>Node body:</dt>
  <dd>
    <p>Lorem ipsum <a href="some/link">dolorem></p>
    <p>bla bla <img alt="some image" src="some/image.png" /></p>
    <table>
      <tr>
        <td>some content</td>
        <td>some content</td>
      </tr>
      <tr>
        <td>some content</td>
        <td>some content</td>
      </tr>
    </table>
  </dd>
</dl>

but I guess markup in the value will always be somewhat of a cause for worrying

so to recap, I think if u want to use definition lists, it should be a rowstyle or you would have to have a configurable row element and label/value elements so the user can manually set the row element to <dl>, the label element to <dt> and the value element to <dd>, which is really flexible and allows the user to make all the choices, but it also allows the user to easily make mistakes so I dunno... guess it's better to have it as a row style and then still allow the user to add an extra wrapper element to label/value

oh and yea, I think adding classes would be rly nice, so a user can use <div> for both the label and the value and simply give them a different class (but I think views adds a field-fieldname class by default)

Anonymous’s picture

Status: Needs work » Needs review
StatusFileSize
new35 bytes

Here's a new patch. It's basically the same as the previous patch except it respects site builders who are already using the module by not breaking their options arrays on existing views.

I want to play more with definition lists. It seems like it could be an easy thing if the CCK multiple value handler can wrap each field value's content.

Anonymous’s picture

StatusFileSize
new4.86 KB

Bad patch above. Review this one instead.

Anonymous’s picture

seutje:

Any feedback you can give on the patch above would be helpful. I want to release soon, and this patch doesn't need to be left out.

BenK’s picture

Subscribing....

Anonymous’s picture

Status: Needs review » Fixed

Because nobody gave any feedback on this patch, I rolled a release and committed my work to the DRUPAL-6--1 branch. It works, but it may not be what makes the most sense. Please let me know on a new issue.

http://drupal.org/cvs?commit=331290

Status: Fixed » Closed (fixed)

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