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

Add ignoreFontFamilyName: [] option to font-family-no-duplicate-names #2086

Closed
ntwb opened this issue Nov 20, 2016 · 5 comments
Closed

Add ignoreFontFamilyName: [] option to font-family-no-duplicate-names #2086

ntwb opened this issue Nov 20, 2016 · 5 comments
Labels
good first issue is good for newcomers status: wip is being worked on by someone type: new option a new option for an existing rule

Comments

@ntwb
Copy link
Member

ntwb commented Nov 20, 2016

Follow up to #1284

There should be a way to ignore, or optionally ignore duplicate monospace font names in font-family-no-duplicate-names

Ways to approach this:

  1. Ignore monospace, monospace by default
  2. Add an option to ignore only the monospace duplicates, i.e. monospace, monospace
  3. Add an option to add an array of font names using regex/string to be ignored.

For context the above is based on the following comments in #1284:

Over in #1284 @ben-eb made the following comment:

"Just FYI, this rule should not affect monospace, monospace, as specifying this twice corrects rendering issues across browsers. http://code.stephenmorley.org/html-and-css/fixing-browsers-broken-monospace-font-handling/"

I added a note below that:

"One of the many reasons I dislike SEO and their practices, this said post has no date, no comments and makes it impossible to determine if this was posted 10 years ago or yesterday. Hence I place next to zero value on content of this manner."

I've still no idea if the above is a legitimate concern for modern browsers but though a rudimentary search of GitHub CSS for monospace, monospace returned 1 million plus results. A high probability that this issue would come up in the future based on those results. The article cites Google Chrome & Safari together, Chrome split from Webkit in Chrome 28 in mid 2013. As such mid 2013 browser versions would be my best guess at trying to guess the versions of browsers that this issue refers to, what has changed in regard to this issue for each browser since then I'm also unaware of /shrug

Thoughts @stylelint/core ?

@ntwb ntwb added the status: needs discussion triage needs further discussion label Nov 20, 2016
@ntwb
Copy link
Member Author

ntwb commented Nov 20, 2016

Some further background on this I just noticed that this was included in normalize.css:

https://github.com/necolas/normalize.css/blob/master/README.md#pre-code-kbd-samp:

pre, code, kbd, samp

The font-family: monospace, monospace hack fixes the inheritance and scaling of font-size for preformatted text. The duplication of monospace is intentional. Source.

From the source cited above: https://en.wikipedia.org/wiki/User:Davidgothberg/Test59:

Explanation: Some browsers on some operating systems show <tt>, <code>, <pre> and <source> tagged text in a too small font. At the time I write this (21 December 2009) we have applied the code below to MediaWiki:Common.css and MediaWiki:Geshi.css, which seems to fix it for those browsers, and cause no problems in the rest of the browsers. If it works well for the English Wikipedia, then we might deploy the fix for all Wikimedia projects. See discussion at {MediaWiki talk:Common.css#Teletype style fix for Chrome](https://en.wikipedia.org/wiki/MediaWiki_talk:Common.css#Teletype_style_fix_for_Chrome).

@davidtheclark
Copy link
Contributor

davidtheclark commented Nov 20, 2016

I think there is no harm in adding an option ignore: ["monospace"] for people who want to use this hack.

@jeddy3
Copy link
Member

jeddy3 commented Nov 20, 2016

Originally I said:

This is what the disable comment are for i.e. one-off instances (e.g. hacks or workarounds). The ignore option is there for repeating patterns that occur more than once within a codebase.

But I guess you could specify monospace, monospace in more than one place in the code and so it is kind of a repeating pattern.

So, I'm easy either way. If we use an option, can we make it one of those more generic ones that accept an array of user-defined options? e.g. `ignoreFontFamilyName: ["monospace"]. It addresses this use-case and, at little cost, adds some flexibility to the rule.

@davidtheclark
Copy link
Contributor

^^ 👍

@jeddy3 jeddy3 changed the title Add ignore font names option font-family-no-duplicate-names Add ignoreFontFamilyName: [] option to font-family-no-duplicate-names Nov 20, 2016
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: new option a new option for an existing rule and removed status: needs discussion triage needs further discussion labels Nov 20, 2016
@davidtheclark davidtheclark added the good first issue is good for newcomers label Nov 26, 2016
@m-allanson m-allanson added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Feb 2, 2017
@jeddy3
Copy link
Member

jeddy3 commented Feb 12, 2017

Done in #2314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue is good for newcomers status: wip is being worked on by someone type: new option a new option for an existing rule
Development

No branches or pull requests

4 participants