[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

report: remove unused css vars #9144

Merged
merged 2 commits into from
Jun 7, 2019
Merged

report: remove unused css vars #9144

merged 2 commits into from
Jun 7, 2019

Conversation

connorjclark
Copy link
Collaborator

I ran this to determine what variables are unused (I verified each):

echo 'text-font-family
monospace-font-family
body-background-color
body-text-color
body-font-size
body-line-height
subheader-font-size
subheader-line-height
subheader-color
header-bg-color
header-font-size
header-line-height
title-font-size
title-line-height
caption-font-size
caption-line-height
default-padding
section-padding
section-indent
audit-group-indent
audit-item-gap
audit-indent
text-indent
expandable-indent
secondary-text-color
informative-color
medium-75-gray
medium-50-gray
medium-100-gray
warning-color
report-secondary-border-color
metric-timeline-rule-color
display-value-gray
report-width
report-min-width
report-width-half
report-header-height
report-header-color
navitem-font-size
navitem-line-height
navitem-hpadding
navitem-vpadding
lh-score-highlight-bg
lh-score-icon-background-size
lh-group-icon-background-size
lh-export-icon-size
lh-export-icon-color
lh-score-margin
lh-table-header-bg
lh-table-higlight-bg
lh-sparkline-height
lh-sparkline-thin-height
lh-filmstrip-thumbnail-width
lh-category-score-width
lh-audit-vpadding
lh-audit-index-width
lh-audit-hgap
lh-audit-group-vpadding
lh-section-vpadding
chevron-size
inner-audit-left-padding
color-black-100
color-black-200
color-black-400
color-black-500
color-black-600
color-black-800
color-black-900
off-black
color-black
color-blue
color-green-700
color-green
color-orange-700
color-orange
color-red-700
color-red
color-white
color-blue-200
color-blue-900
color-cyan-500
color-teal-600
audits-margin-bottom
env-item-bg
env-name-min-width
env-tem-padding
expandable-padding
gauge-circle-size-big
gauge-circle-size
header-padding
highlighter-bg
icon-square-size
plugin-badge-bg
plugin-badge-size-big
plugin-badge-size
plugin-icon-size
pwa-icon-margin
pwa-icon-size
score-container-padding
score-container-width
score-number-font-size-big
score-number-font-size
score-shape-margin-left
score-shape-margin-right
score-shape-margin
score-shape-size
score-title-font-size-big
score-title-font-size
score-title-line-height-big
score-title-line-height
scorescale-height
scorescale-width
section-padding
topbar-bg
topbar-height
topbar-icon-size
topbar-padding
metrics-toggle-color
color-average-secondary
color-average
color-fail-secondary
color-fail
color-pass-secondary
color-pass
color-sticky-header-bg
color-highlighter-bg
color-hover
color-metric-toggle-lines' | xargs -I {} bash -c "grep -q '.*:.*\-\-{}' ./lighthouse-core/report/html/templates.html ./lighthouse-core/report/html/report-styles.css || echo {}" | sort

Result:

audit-group-indent
audit-indent
caption-font-size
expandable-indent
header-bg-color
highlighter-bg
lh-audit-hgap
lh-audit-index-width
lh-category-score-width
lh-filmstrip-thumbnail-width
lh-group-icon-background-size
lh-score-highlight-bg
lh-score-margin
lh-sparkline-thin-height
lh-table-header-bg
medium-100-gray
metric-timeline-rule-color
navitem-font-size
navitem-hpadding
navitem-vpadding
off-black
report-header-color
report-width-half
subheader-font-size
subheader-line-height
title-font-size
title-line-height
warning-color

Noticed this one too, which wasn't caught by the simple command above:

navitem-line-height

patrickhulce
patrickhulce approved these changes Jun 7, 2019
View reviewed changes
Copy link
Collaborator
@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

https://youtu.be/jG5hrWveKe8?t=13

(though you'll have to explain that regex to me sometime, why not look for var(--{}), how the, etc 😃)

@connorjclark
Copy link
Collaborator Author
connorjclark commented Jun 7, 2019 •
edited
Loading

Each line of the input files is tested against .*:.*\-\-{} - I wanted to match any rule declarations that had --some-var on the right hand side. Checking for just \-\-{} (\- because - needs to be escaped) will count the variable declaration line as a usage, which is bad. Checking for var(--{}) will miss anything that does something more complex with calc(...). So I just check for a colon followed by the variable. This works well since grep processes one line of input at a time.

Note, this will miss an unused variables that is only used to define another unused variable. Wasn't meant to be exhaustive :)

@patrickhulce
Copy link
Collaborator

Checking for var(--{}) will miss anything that does something more complex with calc(...)

Oh I didn't realize that! What's the scenario where you can use the variable without var()? Have we been using var in our calcs unnecessarily 😆

@connorjclark
Copy link
Collaborator Author

ahhh yeah you're right, I overcomplicated it. was focussing too much on the case I wanted to ignore I failed to see the obvious pattern. yay regex.

@connorjclark connorjclark merged commit aef01e9 into master Jun 7, 2019
@paulirish paulirish deleted the styles-refactor-part-1 branch June 10, 2019 17:26
@snyk-bot snyk-bot mentioned this pull request Mar 21, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrickhulce patrickhulce patrickhulce approved these changes

@paulirish paulirish Awaiting requested review from paulirish

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@connorjclark @patrickhulce @googlebot