-
Notifications
You must be signed in to change notification settings - Fork 840
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
[EuiKeyPadMenu] Add selection state #4000
Comments
I do have some a11y concerns about this... Either EUI would have to add checkbox/radio item semantics to the keypadmenu (which wouldn't strictly be hard technically, but EUI has rightfully avoided adding multiple semantic meanings to the same visual component) or Kibana would have to create the correct semantics on top of an EUI component that wasn't designed for it (which often can lead to hacky code and/or misuse). Neither issue is insurmountable at all, but wanted to present them earlier rather than later. |
Thanks for writing up this issue, @legrego. And thanks for the thoughtful feedback, @myasonik. For further context, in some of my more recent designs (such as the example Larry provided), I was hoping to promote the expansion of the current capabilities of the
Engineering-wise, what I'm proposing would likely ask that:
Larry has already provided one example above that I've worked up in Figma, but I'd be happy to provide others or to discuss further. |
I think this would be a nice addition to the component. I've been eyeing some other examples of nice radio/selection groups and thinking of ways we could enhance our own. As @myasonik was mentioning this is quite trickier on the implementation side when it comes to ensuring proper accessibility (keyboard and screen-reader output). If it were just a quick change of style, ✅ done! But we've already been in a summer-long conversion of EuiButtonGroup for better accessibility which will have the same implications as this component. Mostly I'm just setting up expectations that this will take more effort and time than it seems. We'll probably need to completely re-think how we handle visual selections and allowing any component to render in the proper context. |
This same design has been proposed for the custom header/footer feature: elastic/kibana#17298 (comment) It's not officially targeted to a specific release, but is a possibility for 7.12 |
I think the best way to handle this one is to create a "checkable" version of EuiKeyPadMenu (similar to EuiCheckableCard) which will display a visible checkbox or radio (depending on multi or single select). However, this input element will only be present when interacting (hover/focus) or when selected. So the unselected item will continue to look similar as it does today. Using the inputs gives us 2 things:
Here's the mocks that I'll use to build off of. The top row is Amsterdam vs default theme below. Each row shows, unselected, selected, and hover states. |
👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment. |
@elastic/kibana-security is still interested in this! |
Yeah I've got a WIP local branch that's gone a bit stale on this one. I'll try to pick it back up before or right after 7.14 FF. @legrego What's your timeline for needing it? |
That's great news! We don't have an explicit timeline -- we would like this for some a11y and ux improvements to the Spaces Management page, but this isn't blocking any critical work for us. |
Would it be possible to add a selection state to
EuiKeyPadMenu
?I'm working through a redesign of Kibana's spaces management page, and one of the suggestions that @MichaelMarcialis proposed was a toggleable set of these components to choose which type of Space Avatar the user wants (image based or text based).
Not a blocker for my work to continue, but wanted to raise this as a possible enhancement.
Link to Figma: https://www.figma.com/proto/dBta1q3JgFe3Cfhw5h76Oq/%5BAwaiting-Dev%5D-Spaces-%26-Roles-Solution-Grouping?node-id=20%3A2087&scaling=min-zoom
The text was updated successfully, but these errors were encountered: