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

Audit: CSS used selectors improvements #1456

Closed
ebidel opened this issue Jan 12, 2017 · 5 comments
Closed

Audit: CSS used selectors improvements #1456

ebidel opened this issue Jan 12, 2017 · 5 comments
Assignees

Comments

@ebidel
Copy link
Contributor

ebidel commented Jan 12, 2017

Seeing this in the DBW tester:

screen shot 2017-01-11 at 8 02 02 pm

What about listing the rules that are not used instead of file snippets? Also, should URLs that have "0% unused rules" show up in the list?

@ebidel ebidel changed the title CSS used selectors improvements Audit: CSS used selectors improvements Jan 12, 2017
@patrickhulce
Copy link
Collaborator

@ebidel showing the unused rules was the original behavior but the consensus was that it was too overwhelming and so instead it just shows you the contents of the file (also helps in cases where there are many inline scripts in sites built with webpack). 0% rules used should show up but only if there's content and it has rules IMO :) will fix

patrickhulce added a commit that referenced this issue Jan 13, 2017
* filter out duplicate stylesheets
* don't report stylesheets that have no rules

addresses #1456
@ebidel
Copy link
Contributor Author

ebidel commented Jan 13, 2017

Yea, just trying to think what is most useful for developers to take action from.

What I'm finding confusing now is that we're always showing general code snippets from the CSS file but that snippet won't contain the unused rule(s). That's a departure from other places in the report we only show violations.

Guess I'm wondering how valuable the snippets are if they're not violations? For example, below I'm using all these rules (chromestatus.com):

screen shot 2017-01-13 at 10 41 25 am

For the "100% usage entry"

  • it makes it seem like I should remove the [hidden] rule.
  • I'm already 100% awesome on that file. Maybe we shouldn't show a snippet at all?

ebidel pushed a commit that referenced this issue Jan 17, 2017
* fix: less noisy CSS unused rules

* filter out duplicate stylesheets
* don't report stylesheets that have no rules

addresses #1456

* add smokehouse length expectation

* feedback
@patrickhulce
Copy link
Collaborator

I think the snippets sole value is for identification of the stylesheet in cases where the URL is not sufficient and in all of these cases you've mentioned the URL is relatively self-explanatory and thus the snippet is just noise. I'm somewhat in favor of bringing back listing the unused selectors but truncating the output at some threshold (maybe just first 50 or something?), or maybe just showing the stylesheet snippet for inline stylesheets where there's no useful URL information?

@ebidel
Copy link
Contributor Author

ebidel commented Jan 18, 2017

maybe just showing the stylesheet snippet for inline stylesheets where there's no useful URL information?

+1. Good balance of noise / usefulness ratio. I have the same problem with the deprecations audit. Some deprecations don't return url/line combos. I used "Unable to determine URL":
#1470 (comment), but we can figure out something better if you want.

@brendankenny
Copy link
Member

fixed by #1496

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

No branches or pull requests

3 participants