New Radio & Checkbox styles #407
Conversation
There was a problem hiding this comment.
This is fantastic. Love the bits of animation you added. I went ahead and checked this against uri's table stuff as well just to make sure there weren't any gotchas and it worked really well. Disabled states in particular are much more clear.
I also checked this out in IE11 / edge. Works great.
Minor comments below.
| @@ -1,8 +1,12 @@ | |||
| .euiScreenReaderOnly { | |||
| @mixin euiScreenReaderOnly() { | |||
src/components/form/_index.scss
Outdated
| background-color: $euiColorPrimary; | ||
|
|
||
| @if $type == 'check' { | ||
| background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='10' height='8' viewBox='0 0 10 8'%3E%3Cpath d='M.375 2.625L3.375 5.625M3.375 5.625L8.625.375' fill='none' fill-rule='evenodd' stroke='#{hexToRGB($euiColorEmptyShade)}' stroke-linecap='round' stroke-width='1.5' transform='translate(.5 1)'/%3E%3C/svg%3E"); |
There was a problem hiding this comment.
Should we put data-uris somewhere specific in global maybe? I could see this being reused? Though I guess the width / height is pretty specific huh. Prolly fine. Just talking about loud.
There was a problem hiding this comment.
It is possible to create a mixin or function to return the url() but it can't be stored in a variable (because of the variable needed for the coloring). Might make it easier to manage even just these 2 duplicated instances.
| background-image 0s ease-out $euiAnimSpeedFast; | ||
| } | ||
|
|
||
| @mixin euiCustomControl--selected($type: null){ |
| &[disabled] { | ||
| ~ .euiCheckbox__label { | ||
| color: $euiColorDarkShade; | ||
| cursor: default !important; |
There was a problem hiding this comment.
I think I'm in the minority with using cursor: not-allowed for disabled states. I'm not tied to it, but if you do go with default, can you change it on the switch as well?
There was a problem hiding this comment.
Yeah I don't really have hard feeling either way. Kind of nice to reiterate the disabled-ness with the cursor. I'll change it to no-allowed.
| @@ -6,8 +6,8 @@ $euiFocusRingAnimStartColor: rgba($euiColorPrimary, 0); | |||
| $euiFocusRingSize: 2px; | |||
| $euiFocusRingSizeLarge: 4px; | |||
|
|
|||
| @mixin euiFocusRing($size) { | |||
| @if $size = "large" { | |||
| @mixin euiFocusRing($size: "small") { | |||
There was a problem hiding this comment.
I used this in a separate PR and I kind of wanted to pass it a pixel value and color. Don't think it needs to be changed (and if it does, it can wait), just doing a gut-check to see what you think before we start using it in more places.
There was a problem hiding this comment.
My only worry is that we will start having so many different focus states which can be jarring while tabbing around. Can you tell me your use case?
There was a problem hiding this comment.
This is great!! What a nice improvement. I noticed two minor things:
-
I'm still seeing
cursor: pointerwhen hovering over a disabled checkbox in a table -
More of a comment about dark theme, but I think the color contrast between our grays is too low for some users to discern checkboxes, table row lines, and the shape of the switch. The active switch state also looks odd in dark mode -- we may just need to make the toggle stand out against the background more?
|
- cursor: not-allowed for disabled states with/without label - increasing brightness of border in dark mode - Moved data-uri images to mixin
This PR reduces the size and updates the styles for the different states of the Checkboxes and Radios. It also updates the Switch and Range to match.
Fixes #364 #303 #266
elastic/kibana#16260
Adds style support for #239
Default
Disabled
Dark
Dark & Disabled
In List/Table mode