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

fix(unused-css-rules): remove stylesheet preview for sheets with URL #1496

Merged
merged 1 commit into from
Jan 19, 2017

Conversation

patrickhulce
Copy link
Collaborator

@patrickhulce patrickhulce commented Jan 19, 2017

R: @ebidel

addresses #1456

@patrickhulce
Copy link
Collaborator Author

Before:
image

After:
image

let url = stylesheetInfo.header.sourceURL;
const label = `${percentUsed}% rules used`;

if (!url || url === pageUrl) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I was curious about for later. When is url blank vs. the same as the page? Can you add a comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh whoops Brendan merged before I could :) it seems to be if it was in the original document or added later with script, which maybe a more sophisticated version later on could identify the URL of the script that added it too? I'll add the TODO in my upcoming rules -> bytes conversion PR for CSS.

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM2

@brendankenny brendankenny merged commit c05548e into master Jan 19, 2017
@brendankenny brendankenny deleted the css_usage_remove_stylesheet_preview branch January 19, 2017 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants