-
Notifications
You must be signed in to change notification settings - Fork 483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Duplicate H element rule might force duplication #80
Comments
Interesting case. Perhaps we should track with properties are being set in order to determine if something is really a duplicate or not. Nicole? |
Hmm, this makes a good point. I think the rule may need to be more clever... e.g. check that each text and font property is declared only once per heading. would that work? |
Should it be limited to just text and font properties? Or would it make sense to track them all? |
I think it would be good if one could check for duplicate properties all over. Not just text and font properties. Tough I do might see another challenge (not sure if it's worth a new issue in the tracker): As we know headings are intended to describe the section it introduces. In HTML4 (and earlier) we normally use the H1 element to describe the whole page due to the fact that we have no good way to divide our pages into more fine grained sections. I assume this CSS lint rule to be bound to this and for me it makes much sense. Please correct me if I'm wrong. But in HTML5 this changes: In other words; in HTML5 it will be perfectly OK to have multiple SECTION elements on a page where each start with a new set of headings. In such a case; wouldn't it be desirable to assign different styles for heading in each section? |
Even though HTML5 allows you to nest heading elements, I think it's still good advice to try to keep your headings unique and use H1 at the highest level, H2 underneath, and so on. In any event, if you disagree with the rule, you'll be able to turn it off. :) |
There are also good reasons for duplicating properties. For example, all the headings are the same weight except one: h1, h2, h3, h4, h5, h6 {font-face: Verdana;font-weight:bold;} I think we need to keep in mind that what we are trying to do is point out places where a human should check if there might be an error. It isn't to say that the linter won't sometimes catch things that look like an error but are actually a choice. If we change it to check by property value pair we might miss a lot of duplicate heading declarations. In fact, they can write as many as they like as long as they use different properties each time. The current rule gives warnings when sometimes it shouldn't, but I think simply increasing the number of allowed heading declarations to 2 would solve the problem better without the risk of missing dups. Regarding HTML5, styling those headings will still be completely impractical unless you define a set of reusable heading classes. You don't want css that looks like this: section h1{color:red;} |
What also applies is if you had a css reset at the top of your css file, and then later setting h rules |
@hojberg - if we use normalize.css instead of reset.css, we won't have that problem. The headings in normalize are meant to be adapted for every project. |
Nesting-level dependent headings do work really great for some kinds of documents though. It's very nice for technical documentation, ebooks, and thinks like that. Structured text documents, basically. In that case you do want CSS which looks like that. But for regular websites it's indeed somewhat awkward. |
Regarding checking all properties, consider this code: h1 { font-family: Arial } I would say the error here is not multiple heading definitions, but rules that can (should?) be combined. |
Let's see...
This should allow you to use a reset (or not), a baseline setup (or not), and individual declarations. Wait a minute. There is an easier way to enforce Nicole's intention. It's only necessary to check if some heading element is used as key selector in a more complex combined selector. ("Combined" because over-qualification is checked by a separate rule.) Basically, the idea is to turn this "duplicate H" check into a "location dependent H" check.
This should be handled separately. Fine-grained one-trick ponies offer more control. |
@mahonnaise - so you mean check for location dependent selectors like this? .foo h1{} |
Yes, it A) uses a H tag as key selector and B) it's a combined selector (descendant combinator). As such, it's deemed nag-worthy. |
Some of my thoughts on headings in HTML5 first:
Markup wise I do not quite agree on that. There are a lot of cases where one would like to start a new H1 inside a section. I think the frontpage of http://www.bbc.co.uk/ are a good example of where H1 does not have a natural place as the highest level of the whole page. In BBC's case most people would place the H1 around the logo at the top of the page. I find that wrong. Its tough interesting to see what BBC actually does now; they have a top level text saying BBC Homepage in a H1 with a css class set to hidden.
I do agree on that. But I might want to go like this: .fooBox h1{color: red;}
That's a good point. I agree on that one.
I do agree its very nice to be able to turn rules of. What I like about such lint tools are that they can point out things which will cause errors and also give personal advices / flavors of the author(s). In the case of the linter warning about errors there are little doubt about the reason for the warning. When it comes to advices from the author(s) I find it good to be able to read up on the author(s) advice and upon that decide if I should turn the warning off or not. I think good documentation and reasoning for each warning in the lint tool are a key factor for the tool. |
Re: HTML5, the spec says: “Sections may contain headings of any rank, but authors are strongly encouraged to either use only h1 elements, or to use elements of the appropriate rank for the section's nesting level.” but ‘ …fwiw :) @nzakas rather than a simple case of combination
it’s more like
Some properties defined for all headings, some for only a few headings, and the occasional override. |
Hey Nicole, if I should not qualify headings then what about these bits from oocss? Along with some other tiny things that don't validate against csslint. No accusation, just asking :) |
Part of the missing documentation for CSS Lint is that most rules don't apply to your foundational CSS bits (reset stylesheets, grids stylesheets, etc.). The oocss library is also foundational. |
I see why you would regard it as foundational, but oocss contains the same selections oli mentioned in his comment beforehand (#80 (comment)) As well as the selections I already mentioned. |
I'm not sure what you're trying to say. I'm saying that oocss is a foundational library so many of the CSS Lint rules actually wouldn't apply. Are you trying to say it's not a foundational library? Or something else? |
The code mentioned by oli is similar to the headline declarations in oocss: h1, h2, h3, h4, h5, h6, ul, ol,dl, p,blockquote {padding:10px;} h1, h2, h3, h4, h5, h6,img{padding-bottom:0px;} pre{margin: 10px;} table h1,table h2,table h3, table h4, table h5, table h6, table p, table ul, table ol, table dl{padding:0;} and h1, .h1{font-size:196%; font-weight:normal; font-style: normal; color:#AE0345;} h2, .h2{font-size:167%; font-weight:normal; font-style: normal; color:#AE0345;} h3, .h3{font-size:146.5%; font-weight:normal; font-style: normal; color:#DF2B72;} h4, .h4{font-size:123.1%; font-weight:normal; font-style: normal; color: #333;} h5, .h5{font-size:108%; font-weight:bold; font-style: normal; color:#AE0345;} h6, .h6{font-size:108%; font-weight:normal; font-style: italic; color:#333;} and you say I should just skip it? I don't think that this is a reasonable solution. |
I'm sorry, I'm having a hard time understanding the point you're trying to make. Your original question asked why there were CSS Lint warnings in oocss, and I've answered that. If you like me to answer another question, please ask it directly. |
I thought I did that :D You say: OOCSS is foundational and does not have to validate. Why? So, once again: Sidenote: |
Okay, you're arguing that the oocss library should pass CSS Lint 100%. As I said, foundational libraries have different characteristics than non-foundational, so not all CSS Lint rules will apply and therefore this won't happen. I'm sure other foundational libraries such as YUI CSS foundation also would see quite a few warnings. That's expected because foundational libraries abstract away ugliness so you don't have to worry about it. In any event, this isn't the right forum for debating whether oocss should or should not pass CSS Lint. I'd suggest either using the CSS Lint mailing list or the oocss mailing list for that purpose. |
ok, maybe you should add a link to google groups in the docs then or am I too blind to see it? |
Not every rule is compatible with every approach. You should use and/or create rules which enforce the conventions you are using. Decide for yourself where you want to draw the line when it comes to accuracy. |
I don't quite understand the conclusion... It sounds like people are agreeing that the current rule is somewhat specialized, but it should be left on by default? That seems like a recipe for false positives. My case: I'm styling Markdown-generated HTML so headings can't nest. Here's my markup: h1,h2,h3,h4,h5,h6 {
font-family: 'Lucida Grande', Helvetica, Verdana, sans-serif;
font-weight: bold;
}
h1,h2 { color: rgb(52,101,164); }
h3,h4,h5,h6 { color: rgb(114,159,207); }
h1 { margin-top: 1.3em; font-size: 1.5em; }
h2 { font-size: 1.3em; } Is the linter suggesting that there's a better way to write this? |
Pardon me if I'm wrong, but it appears to me that this rule—and most of the discussion of it—even its documentation—misses the entire point of the rule:
The documentation gives this example of "bad code": /* Two rules for h3 */
h3 {
font-weight: normal;
}
.box h3 {
font-weight: bold;
} But if the idea is to make heading elements appear the same anywhere in the page, then it takes only one rule to break this: .box h3 {
/* There. Now h3 inside of a .box and outside will differ. With only one rule
declared. The h3 is no longer atomic, but no warnings are shown. */
font-weight: bold;
} Even this code follows the spirit of the rule: h1, h2 {
color: black;
font-weight: bold;
}
h1 {
font-size: 48pt;
}
/* There. Now h1 and h2 will look the same no matter where on the page.
The h1 and h2 headings are still atomic, but the linter complains about multiple
rules. */ It makes no difference whatsoever whether there is one rule or a thousand rules. What matters is whether the headings are qualified, making them context-dependent, and therefore non-atomic. In fact, I cannot think of a single instance where this rule would catch non-atomic headings that wouldn't be caught by a "don't qualify headers rule" (Of course things like Am I missing something? Is there a legitimate reason to count the number of rules instead of whether rules are qualified? |
The following style:
h1, h2 {font-face: Verdana;}
h1 {font-size: 2.1em;}
h2 {font-size: 1.8em;}
does prompt a warning due to the recommendation that one should only define H elements only once in a style. Tough; In this example one would have to replicate the font-face for all H tags to conform with the lint rule. That would increase the size of the css file.
Isn't it most common to set some styles which apply to all H elements and then override only the differences, like the font-size, for each H element?
The text was updated successfully, but these errors were encountered: