Skip to content
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

False positives for font-family in font-family-no-missing-generic-family-keyword #3848

Closed
hirokishirai opened this issue Dec 12, 2018 · 13 comments
Labels
status: needs discussion triage needs further discussion

Comments

@hirokishirai
Copy link

hirokishirai commented Dec 12, 2018

Clearly describe the bug

Thanks for great work of stylelint community.

There is False positives for font-family in font-family-no-missing-generic-family-keyword(I'm sorry If this is not Bug).
When I specified font-family with "my_original_font_family_name" !important, stylelint showed me bellow error.

path/to/_my_original_fonts.scss
 23:16  ✖  Unexpected missing generic font family   font-family-no-missing-generic-family-keyword

But, font-family: "my_original_font_family_name" is defined by @font-face.

When I specified font-family with my_original_font_family_name !important(without ""), stylelint test is passed.

This issue is similar to #3033 ?

Which rule, if any, is the bug related to?

font-family-no-missing-generic-family-keyword

What CSS is needed to reproduce the bug?

/* scss */
@font-face {
  font-family: "my_original_font_family_name";
  src: font-url("my_original_font_family_name.eot?fooooo");
  src:
    font-url("my_original_font_family_name.eot?fooooo#iefix") format("embedded-opentype"),
    font-url("my_original_font_family_name.woff2?fooooo") format("woff2"),
    font-url("my_original_font_family_name.ttf?fooooo") format("truetype"),
    font-url("my_original_font_family_name.woff?fooooo") format("woff"),
    font-url("my_original_font_family_name.svg?fooooo#my_original_font_family_name") format("svg");
  font-weight: normal;
  font-style: normal;
}

[class^="icon-"],
[class*=" icon-"] {
  font-family: "my_original_font_family_name" !important;
}

What stylelint configuration is needed to reproduce the bug?

{
  "rules": {
    "font-family-no-missing-generic-family-keyword": true
  }
}

Which version of stylelint are you using?

9.9.0

How are you running stylelint: CLI, PostCSS plugin, Node API?

CLI with stylelint "**/*.scss" --config stylelint.config.js

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

I guess No.

What did you expect to happen?

No warnings in this case.

What actually happened (e.g. what warnings or errors did you get)?

e.g. "The following warnings were flagged:"

path/to/_my_original_fonts.scss
 23:16  ✖  Unexpected missing generic font family   font-family-no-missing-generic-family-keyword
@jeddy3
Copy link
Member

jeddy3 commented Dec 12, 2018

@hirokishirai Thanks for the report and for using the template.

I believe this rule is correctly detecting a violation. It ensures that a generic font family is used every time a list of family names is used outside of a @fontface definition.

To appease the rule you'll need to add a generic font family to your list of family names e.g.

[class^="icon-"],
[class*=" icon-"] {
  font-family: "my_original_font_family_name", sans-serif !important;
}

The list of generic font families can be found in the specification.

When I specified font-family with my_original_font_family_name !important(without ""), stylelint test is passed.

That's strange. I'm unable to reproduce this behaviour. I get a violation message regardless of whether the family name is quoted or not. If this problem persists, can you create a reproducible test repo so we can investigate further, please?

@jeddy3 jeddy3 added the status: needs discussion triage needs further discussion label Dec 12, 2018
@jeddy3
Copy link
Member

jeddy3 commented Dec 12, 2018

Additionally, if the font is an icon font file then you might also be interested in #3288, which is an issue to add an ignoreValues option to the rule.

This will allow you to ignore instances of "my_original_font_family_name". That issue is labeled as "help wanted" and is waiting for someone to contribute it.

@hirokishirai
Copy link
Author

@jeddy3 Thanks for your rapid response.

To appease the rule you'll need to add a generic font family to your list of family names e.g.
The list of generic font families can be found in the specification.

I fixed with font-family: "my_original_font_family_name", sans-serif !important;!
I didn't know "What is the generic-family"...

That's strange. I'm unable to reproduce this behaviour. I get a violation message regardless of whether the family name is quoted or not. If this problem persists, can you create a reproducible test repo so we can investigate further, please?

There were nothing problem about this.

I tried skip font-family: "my_original_font_family_name" !important; line with /* stylelint-disable */../* stylelint-enable */.
I think I was using it when I got no warning.
I tried one more time, stylelint showed me warning. I can understand this is correctly behavior now:)

Sorry for my stupid reporting...!

Thanks for your great supporting.

@iorrah
Copy link

iorrah commented Mar 19, 2020

You can work around this false-positive by abstracting the iconography font-family into a variable:

$icon-font-family: 'font-awesome';
font-family: $icon-font-family; // Works!

@jeddy3
Copy link
Member

jeddy3 commented Mar 20, 2020

Thanks for sharing your workaround.

There is an open issue to add an ignoreValues :[] secondary option to the rule, which would allow this rule to be used without a workaround.

I've labelled that issue as ready to implement. There are steps on how to add a new option in the Developer guide.

@ux-engineer
Copy link

I had a problem with this after importing Fontawesome SCSS files.

Solved by configuring ignoreFontFamilies for the rule like:

{
  "extends": "stylelint-config-recommended-scss",
  "rules": {
     "font-family-no-missing-generic-family-keyword": [
        true,
        {
          "ignoreFontFamilies": ["Font Awesome 5 Free", "Font Awesome 5 Pro", "Font Awesome 5 Duotone", "Font Awesome 5 Brands"]
        }
    ]
  }
}

@dmrqx
Copy link

dmrqx commented Apr 21, 2020

You can work around this false-positive by abstracting the iconography font-family into a variable:

$icon-font-family: 'font-awesome';
font-family: $icon-font-family; // Works!

Hi :)
I'd just like to add a use case that may also be a false positive.
Regarding this comment, I'm using custom properties aided by PostCSS and it doesn't seem to work as $ variables.

The rule applied to html tag within typography.css works fine and doesn't raise any issues with stylelint.
The rule applied to .repository-link class also works in the browser, but is not validated.

My stylelint config is simple, only extending stylelint-config-standard and this rule is not overwritten by standard config.

postcss.config.js

module.exports = {
  modules: false,
  plugins: {
    'postcss-preset-env': {
      autoprefixer: { grid: 'autoplace' },
      features: {
        'custom-properties': false
      }
    },
    'postcss-css-variables': {
      preserve: true
    },
    'postcss-modules': {
      generateScopedName: '[local]___[sha1:hash:hex:8]'
    },
    'postcss-import-url': {
      modernBrowser: true
    }
  }
}

typography.css (some styles are ommited for clarity)

@import 'https://fonts.googleapis.com/css?family=Roboto+Condensed&family=Roboto:400;700&subset=latin&display=swap';

@font-face {
  font-weight: 400;
  font-family: system;
  font-style: normal;
  src: local(".SFNSText-Light"), local(".HelveticaNeueDeskInterface-Light"), local(".LucidaGrandeUI"), local("Ubuntu Light"), local("Segoe UI Light"), local("Roboto-Light"), local("DroidSans"), local("Tahoma");
}

:root {
  --default-font-stack: 'Roboto', system, sans-serif;
}

html {
  font-family: var(--default-font-stack);
}

file that is "violating" rule 'font-family-no-missing-generic-family-keyword'

.repository-link {
  font-family: 'Roboto Condensed', var(--default-font-stack);
}

@jeddy3
Copy link
Member

jeddy3 commented Apr 22, 2020

@dmrqx Thanks for adding your use case.

The option mentioned in #3848 (comment) was added in 13.3.0. You can now use the rule's ignoreFontFamilies secondary option for your use case:

{
  "extends": ["stylelint-config-standard"],
  "rules" {
     "font-family-no-missing-generic-family-keyword": [true, 
       { "ignoreFontFamilies": ["var(--default-font-stack)"] }
     ]
  }
}

The rules built into stylelint as strict by default but can be made more permissive using the optional secondary options. This is one of those cases as stylelint does static analysis of your source code. It doesn't know whether your default-font-stack variables contains a generic font-family or not. You must explicitly tell it using the ignoreFontFamilies option.

@AndrewKvalheim
Copy link

{ "ignoreFontFamilies": ["var(--default-font-stack)"] }

I don't think this works; var(…) isn't considered to be a font family to begin with—both functions and variables are excluded—so no configuration will ever match it.

@jeddy3
Copy link
Member

jeddy3 commented May 12, 2020

@AndrewKvalheim You are correct.

I've created a new issue to ignore by default any values that contain variables (dollar variables, custom properties etc).

@dmrqx or @AndrewKvalheim Please consider contributing a fix to that issue if you have time.

@dmrqx
Copy link

dmrqx commented Jun 5, 2020

@jeddy3 Thanks for your reply.

I'm very sorry for the late response but I had to be away for a while.
I also have yet to test the option you mentioned.

Moreover, I'd love nothing more than contributing, or to at least try to.
I'll read the contrib guide and see how I may be of help.

@jeddy3
Copy link
Member

jeddy3 commented Jun 5, 2020

I also have yet to test the option you mentioned.

The new issue was fixed in 13.6.0, so you should no longer get false positives when using a variable.

Moreover, I'd love nothing more than contributing,

That'll be fantastic as new contributors are most welcome! Feel free to jump into any other open issue that interests you.

@SyedShahjahan
Copy link

Thank you so much it worked for me too :D . I'm happy.

You can work around this false-positive by abstracting the iconography font-family into a variable:

$icon-font-family: 'font-awesome';
font-family: $icon-font-family; // Works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs discussion triage needs further discussion
Development

No branches or pull requests

7 participants