Hm.
I've installed Fences and I'm looking at the markup it creates vs the markup I created in Views (by hiding all the fields but the last one, and then rewriting the output of the last field to contain all the markup) for the same content. Fences by default has lots and lots of classes. It's printing all the classes that Drupal knows about everything. I'm not sure this is necessary.
For example:
<blockquote class="field field-name-field-review-quote field-type-text-long field-label-hidden field-item even"> <p>Spare ribs bacon pancetta, ham tenderloin cow ground round shankle pastrami turkey jerky. Turducken meatball ground round, tail rump shank hamburger venison frankfurter fatback pig ribeye filet mignon strip steak ham hock.</p> </blockquote> Jack Smith <cite class="field field-name-field-review-link field-type-link-field field-label-hidden field-item even"> <a href="http://example.com/79">That Awesome Blog Post</a> </cite> <time class="field field-name-field-review-date field-type-date field-label-hidden field-item even"> <span class="date-display-single">Thursday, November 24, 2011</span> </time>
vs.
<blockquote cite="http://example.com/node/79">Spare ribs bacon pancetta, ham tenderloin cow ground round shankle pastrami turkey jerky. Turducken meatball ground round, tail rump shank hamburger venison frankfurter fatback pig ribeye filet mignon strip steak ham hock.</blockquote> <footer> Jack Smith <cite><a href="http://example.com/79">That Awesome Blog Post</a></cite> <time datetime="2010-03-23T04:00:00-04:00 2010-03-23T04:00:00-04:00">March 23, 2010</time> </footer>
We've talked already about the need to be able to group fields and set an element on the group — like I used footer to wrap the source of the quote. That's one big difference between these two.
The other difference is that the second version doesn't have five or so classes on each element. Those classes can be helpful when you are first learning CSS. And having Fences pass all the classes along makes it go well with the Zen theme, since the Zen theme is in the same style. I'm not sure that's what everyone will want, however. Now that we have CSS 2.1 and 3 puedo-classes around, most CSS can be written without any extra classes on things. Nth-child replaces the need for zebra stripping classes. Using semantic elements instead of only divs means many things can be targeted without a class on the element. And anyone who wants to use object-oriented CSS (a la Nicole Sullivan) will want to be able to set custom classes on things, not be writing CSS to existing Drupal classes.
Also looking at the code above, it's frustrating to have [p] inside [blockquote], and [span] inside of [time]. It's like all this work to make this awesome Fences module, and we still have too much HTML markup.
These days, I'm wanting to strip out everything that's not being used. Or rather — I want to start with simple markup, and only add what's needed. Lean markup downloads faster and renders faster. It's easier to write and use.
So how could Fences handle classes?
1) Leave all the classes in, and say, "Hey that's Drupal!"
2) Remove all the print statements that put the classes into the markup, and give everyone lean markup all the time.
3) Give people an option to "print default classes" or not. Like Views does. A checkbox to toggle on and off.
4) Give people a way to add a custom class (or three). Perhaps alongside a 'default classes' toggle checkbox, so people can have all the Drupal classes and add more. Or have none of the Drupal verbiage, and simply add the ones they want or need as they go along.
Comments
Comment #1
johnalbinHere's what I think of the default classes for fields:
I think we should remove #1-4 by default. And I'm not sure we even need an option to add them back. Though, I suppose, we'd want to support contrib themes that may be using these classes.
#5 can be toggle-able. It's arguably useful.
#6 I wish was slimmer. Just the "field-ACTUALNAME" instead of "field-name-field-ACTUALNAME". Maybe a toggle to show slimmer version?
That's too many checkboxes for each field. I think these checkboxes should be on a global Fences configuration page. Set those options once and use them for all fields.
And adding a custom class per field makes sense.
Comment #2
jensimmons commentedAh, good idea to look at exactly what these classes are. I vote for stripping out 1–5. We don't need any of them. I don't see a common need to write CSS targeting a type of field (#5). I can't remember doing that ever, definitely not often. And if it's needed, developer can either target the (now-semantic) elements instead, write out several field name classes in a group, or add a custom class to those fields.
For #6, I agree let's have a field class that's "field-ACTUALNAME" (where the field's machine name is used, and all underscores become hyphens) instead of field-name-field-ACTUALNAME. This one class will be a nice amount of classes. Simple. Clear. Helpful. I don't think we need to be a toggle to ofter the longer version. When will 'field-NAME' fail, and 'field-name-field-NAME' be better?
And yeah, I agree having a universal 'use Drupal default classes on all fields' for Fences would be better than having that checkbox on every field. But actually, looking at this now, I don't think we need the option to toggle all six classes on and off at all. Let's just remove the five, simplify the sixth, and leave it. If a themer really misses the mass of six, they can preprocess to bring those classes back. I'd rather put the burden on the 0.05% of people, rather than on 98% of people. I do think Drupal will be much better served by giving people default markup that matches what they usually need, and not what every possible use-case needs.
Comment #3
jon betts commentedWow! I'm getting all teary-eyed and it's not just the pollen! Great work!
For what it's worth, I like Jen's reasoning regarding just stripping the classes out and putting the burden on the .05%. Most, if not all of the times I've used any of those classes, it was because they were there. Not because they were needed.
Lack of zebra stripping classes may be an issue for those targeting IE 7-8, but that could be remedied with some simple jQuery (Edit: and/or selectivizr)
Comment #4
johnalbinI think this should be 7.x-1.0 material.
Comment #5
johnalbinWhen I went looking, I found that the
field-itemandodd/evenclasses were being add by our field templates (just like core does it.) So I removed them from all of our template files.I was also reminded that field module conditionally adds the
clearfixclass if the label is inline. Fences now conditionally removes that class. :-)And, lastly, there is now an option (off by default) that strips all the other bad classes and replaces them with a simple
field-NAMEclass. This is true for field.module-generated fields (the field-name-field-NAME fields) and other module-generated fields (like comment.module's field-name-comment-body field).Fixed. http://drupalcode.org/project/fences.git/commitdiff/7df9ef9?hp=02127923d...
Comment #6
johnalbinThere's no more 7.x-1.0 release blockers according to my list.