-
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
Generic font family check #334
base: master
Are you sure you want to change the base?
Conversation
It is like #275 but has some more checks and avoiding some rough edges, which, as far as i can see, will fail in aforementioned task. |
Can you provide some examples that this rule would catch? @stubbornella thoughts? |
Rule checks for font family declarations with no generic alternative /* Warning! No generic! */
.clazz {
font-family: Tahoma;
}
/* Warning! No generic! */
.clazz2 {
font: 12px Georgia;
} This is how we can fix above mentioned rules... /* OK */
.clazz {
font-family: Tahoma, sans-serif;
}
/* OK */
.clazz2 {
font: 12px Georgia, serif;
} Following lines will not cause a warning /* OK */
@font-face {
font-family: Pompadur; /* a custom name declaration */
src: url(fonts/pompadur.ttf); /* a custom font file */
} But the following usage - does /* Warning! No generic! */
.myFont {
font-family: Pompadur;
} And how to fix it... /* OK */
.myFont {
font-family: Pompadur, sans-serif;
} Also, using an /* OK */
.child {
font-family: inherit;
} The another important thing is about quotes. "serif" and serif - this is not the same! /* Warning! Quoted Generic */
.iLikeQuotes {
font-family: Tahoma, "serif";
} To fix it - just remove the quotes /* OK */
.iLikeQuotes {
font-family: Tahoma, serif;
} |
I mentioned this in another thread about this issue, but fallback fonts aren't needed for pseudo elements. For example, if I define the following rule:
This would be fine. If the font isn't found or there was an error in loading, it would fall back to the font defined on the parent rather than falling back to the system font. |
@philipwalton sounds reasonable... I'll fix it and update the pull request. |
@Olegas I remember testing it with :before and :after, but you might want to check :first-letter and :first-line as well, just in case. |
@philipwalton What do you think, how to deal with such rules: .clazz,
p:hover {
font-family: Tahoma;
}
|
I'd say require a backup, since |
@Olegas , could you please give fixing @philipwalton 's comments a try? |
Will take a look soon, this weekend. Sorry for delay. |
New rule 'generic-font-family'
This rule checks, that each font/font-family declaration contains a 'generic' font family (one of the following: serif, sans-serif, cursive, monospace, fantasy).
Also, it is checks that 'generic' font family is specified as an identifier, not as a string, cause this prevents correct rendering.
Specifying a 'generic' font family is needed to keep text rendering as near as possible to desired style on systems, where other specified fonts are not exists, some kind of a fallback.