-
Notifications
You must be signed in to change notification settings - Fork 65
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1913 +/- ##
======================================
Coverage 100% 100%
======================================
Files 415 415
Lines 8746 8746
Branches 1292 1292
======================================
Hits 8746 8746 Continue to review full report at Codecov.
|
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.
LGTM
@Blackbaud-ToddRoberts - does this look ok to you?
Having a couple other designers look at this as well since we were discussing Select styling recently. |
Is there any accessibility impact here? |
Generally the style is good. I can't tell what the spacing is between the text and the icon - can we ensure it's 10px as it is for Dropdown? |
@Blackbaud-ToddRoberts took a few tweaks, but the padding is now 10px for all browsers above. |
Also, @Blackbaud-ToddRoberts re:accessibility > Great question. I compared this change with the prior state using JAWS screen reader, and its the same experience. It doesn't affect any aria labels or key strokes, since the change is limited to CSS only. JAWS still sees it as a combo box. I think we're good to go here. |
Fixes #1242