Improve focus state of compressed EuiSwitch #2745
Conversation
| @@ -46,6 +46,16 @@ | |||
| } | |||
| } | |||
|
|
|||
| @keyframes focusRingAnimateMini { | |||
There was a problem hiding this comment.
Curious: What was your thought process behind creating new sized focus ring besides using an existing one?
There was a problem hiding this comment.
My first thought was to continue using focusRingAnimate and just increase the value of $euiFocusRingSize. However, then I realized there's a number of components using euiCustomControlFocused (and therefore euiFocusRing with the default size of small) that would be unnecessarily affected so I decided to create a new sized focus ring (focusRingAnimateMini) for this case.
There was a problem hiding this comment.
I agree with @myasonik here that I actually had to zoom into the screenshot to see it at all. I'm wondering if the main problems stems back to the fact that the focus state only rings the switch thumb and not the entire thing. Being that it's on the thumb makes it blue on top of blue in parts and so makes it hard to see.
Maybe consider if we should move the focus to the whole switch and then maybe it can continue to use the default size of the focus ring instead of making an in-between size? Or if we think the default size is too small, maybe we should just bump it for everything?
|
So I tried giving the focus state to the whole switch instead of just the thumb and I like it a lot better. I also looked at some references and that seems to be the standard.
Now, we'd have these two options: B or C. B uses the current default ring size (
Like I mentioned in a previous comment, there are multiple components using
All in all, I'm leaning towards option C where we increase the value of |
|
I definitely agree, I like C for the switches too! For the checkbox item below, it looks like you might have switched the "before" and "after" labels? Either way, I like what is currently labelled as "before" which, if they're not switched, does make a tricky situation... |
Whoops, yes I had those switched. I updated my original comment to include a screenshot with proper labels. |
|
Just move the focus from the thumb to the whole switch makes a HUGE difference and we should definitely move that direction. I am also on the side of making these focus states more obvious and bumping the ring size by 1px really helps do that (surprisingly). I think we should be ok bumping the original What you may want to do is a quick search in project for any components using that mixin and just do a quick test of the focus to make so there are no regressions. |
|
Just passing by to say this looks really great! 👋 |
Summary
This PR improves the focus state of
EuiSwitch compressedandEuiSwitch minito make it more perceivable.Focus state of
EuiSwitch miniinEuiDataGridFixes #2420
Checklist
- [ ] Props have proper autodocs- [ ] Added documentation examples- [ ] Added or updated jest tests- [ ] Checked for breaking changes and labeled appropriately