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(icon-button): Remove unused styles and update docs #2957

Merged
merged 4 commits into from
Jun 20, 2018

Conversation

williamernest
Copy link
Contributor

@williamernest williamernest commented Jun 18, 2018

Address: #2918
Also, fixes a bug where the foundation was always initializing as 'off'.

@williamernest williamernest changed the title Fix/icon button/remove unused styles fix(icon-button): Remove unused styles and update docs Jun 18, 2018
@codecov-io
Copy link

codecov-io commented Jun 18, 2018

Codecov Report

Merging #2957 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2957      +/-   ##
=========================================
- Coverage    98.3%   98.3%   -0.01%     
=========================================
  Files         101     101              
  Lines        4375    4368       -7     
  Branches      564     564              
=========================================
- Hits         4301    4294       -7     
  Misses         74      74
Impacted Files Coverage Δ
packages/mdc-icon-button/index.js 100% <100%> (ø) ⬆️
packages/mdc-icon-button/foundation.js 96.29% <100%> (-0.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01abc11...925a243. Read the comment docs.

@williamernest williamernest force-pushed the fix/icon-button/remove-unused-styles branch from 0ea9ec0 to c1c5714 Compare June 20, 2018 15:53
@@ -23,10 +23,4 @@
@include mdc-states;
}

.mdc-icon-button--disabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be replaced with a :disabled selector instead of removed completely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The :disabled selector is in the mixins file. This was left over from icon-toggle and was used when the icon-toggle was on an element other than a button (span, div, etc). We dropped support for everything except for button and a tags, so this is no longer needed at all. Like mdc-button you can use :disabled for button elements to disable them. Anchor tags don't support disabled at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sweet, thanks for explaining!

@patrickrodee patrickrodee self-assigned this Jun 20, 2018
@williamernest williamernest merged commit 32b5b9d into master Jun 20, 2018
@williamernest williamernest deleted the fix/icon-button/remove-unused-styles branch June 20, 2018 23:10
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.

3 participants