-
Notifications
You must be signed in to change notification settings - Fork 65
Added icon style checkboxes #1644
Added icon style checkboxes #1644
Conversation
Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED Commit: 8d1f47f (Please note that this is a fully automated comment.) |
Codecov Report
@@ Coverage Diff @@
## master #1644 +/- ##
==========================================
+ Coverage 99.98% 99.98% +<.01%
==========================================
Files 395 395
Lines 8145 8151 +6
Branches 1198 1199 +1
==========================================
+ Hits 8144 8150 +6
Misses 1 1
Continue to review full report at Codecov.
|
@Blackbaud-AnandBhat Thanks for the pull request! Is there an issue associated with this request? Any background info you can provide? |
} | ||
|
||
.sky-checkbox-wrapper.sky-checkbox-icon.sky-checkbox-icon-info input:hover + .sky-checkbox { | ||
border-color: $sky-background-color-info; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Border colors should be the $sky-highlight-color-X vars instead of the background color vars, for all states (hover, active and checked).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
height: 34px; | ||
width: 34px; | ||
line-height: 34px; | ||
border-radius: 2px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be $sky-border-radius
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Tests failed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 588c552 (Please note that this is a fully automated comment.) |
Tests failed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: df79073 (Please note that this is a fully automated comment.) |
Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED Commit: 9ce9d5a (Please note that this is a fully automated comment.) |
|
||
.sky-checkbox-wrapper input:disabled + .sky-checkbox { | ||
background-color: $sky-background-color-neutral-light; | ||
@include sky-border(light, top, bottom, left, right); | ||
} | ||
|
||
.sky-checkbox-wrapper input:focus + .sky-checkbox { | ||
.sky-checkbox-wrapper:not(.sky-checkbox-icon) input:focus + .sky-checkbox { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The icon version still needs the focus styling here so that keyboard users can see when the checkbox is focused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED Commit: d9050c1 (Please note that this is a fully automated comment.) |
Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED Commit: 28bd943 (Please note that this is a fully automated comment.) |
I created a contribution pull request that includes the style changes. We'll circle-back with the icon feature soon. |
Closing in favor of: #1892 |
No description provided.