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 outline: 0 to flagged property/value combinations #89

Open
Volker-E opened this issue Mar 11, 2019 · 7 comments
Open

Add outline: 0 to flagged property/value combinations #89

Volker-E opened this issue Mar 11, 2019 · 7 comments

Comments

@Volker-E
Copy link
Contributor

outline: 0 is seen an accessibility anti-pattern and there are several places, where this was introduced for styling reasons and assumed lack of knowledge.
I'd propose for adding it to 'declaration-property-value-blacklist' which would need an extra override in case you're re-adding custom focus styles to your project.

@Volker-E
Copy link
Contributor Author

Volker-E commented Mar 11, 2019

@edg2s @Krinkle @jdlrobson @j4n-co your thoughts…?

@Volker-E
Copy link
Contributor Author

@edg2s
Copy link
Member

edg2s commented Mar 11, 2019

Seems reasonable. Are there any other examples in MW core, VE etc?

@Volker-E
Copy link
Contributor Author

Oh yeah:
https://codesearch.wmflabs.org/search/?q=outline%3A%200&i=nope&files=&repos=

  • lib/jquery.chosen
  • ContentTranslation
    • cardSearch
    • search tool
  • DonationInterface
  • Flow menu

and so forth and so on…

@edg2s
Copy link
Member

edg2s commented Mar 13, 2019

As discussed, let's add a custom warning message for disabled properties that links to a wiki page (on wiki or github) outlining the reasoning for each disabled property.

@Volker-E
Copy link
Contributor Author

Volker-E commented Jun 19, 2019

@edg2s It would look like this

"declaration-property-value-blacklist": [ {
			"outline": ["/none/", "/0/"]
		},
			"message": "No `outline` is an accessibility anti-pattern. Only use when focussed state is ensured." ]

Not sure how to deal with two different blacklist items though…

Update: Doesnt' seem to work for one rule:
stylelint/stylelint#551 (comment)

@Volker-E
Copy link
Contributor Author

@edg2s Let's move this forward, I'd suggest leaving a comment in the index.js instead. We've got a number of opinionated rules in here. And it's fair to expect certain engagement from devs with rules from a linter in our environment.
We've also been so far bringing the changes to the single repos, so that devs have an anchor point for orientation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants