-
Notifications
You must be signed in to change notification settings - Fork 839
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
[EuiDataGrid] Implement remaining keyboard shortcuts #2482
Comments
Hello @myasonik I'm giving a shot with this one. What I managed to do so far is:
What is still missing:
I could think of an easy way to identify the bottom most visibile row. My plan is to iterate the rows comparing each one's offset top ( The thing is, as I understand I'll only be able to get the elements position inside the body part of the data grid component. I'll probably have to pass a new function down, similar to the I would have a similar approach for the upper most row (
I didn't undertand that. What do you mean by "the first one"? Woudn't the each cell have only one widget? Or do you mean "the first cell of the row which have a widget" (in the case of a tabular form, for instance) "Inserts key"? Do you mean inserts the value of the alphanumeric key in the widget, replacing its current value? |
For Page Down & Page Up, the spec says
Since our grid is paginated instead of a giant sheet with all the values, I think it makes more sense for page up / page down to move to the next logical page, and the correct cell should scroll into view once it is focused. Alphanumeric input is a bit odd to make work in the DOM. On alphanumeric keypress:
|
For Page Down & Page Up, I like the idea of switching to the next/previous page. I'd then set focus onto the first cell of the grid body. For alphanumeric keys it's actually a bit trickier than that. We can't rely only on what is focusable because if the cell contents is something like |
For the For the alphanumeric keys should I give it a try? Check the current cell for a input-text/textarea element and then set the focus and change its value accordingly to the pressed key. Is that it? |
Yup, that sounds correct! If you want to give it a go, that'd be awesome! But you can also punt on that and leave it unimplemented. Not all the keyboard shortcuts have to be implemented at the same time. Totally up to you. |
|
Yeah I think I'm gonna wrap it around here... Perhaps the remaining feature could be moved to a new issue. But if you have some tip about the problem of losing focus after page change, I'd be happy to fix it. (perhaps tomorrow I'll give a new look at this particular issue) Here is what I've done in the last commit:
|
With many of these shortcuts about to land, we should also think about how we can let users know they're available. We can let screen reader users know with some hidden text but I think the keyboard shortcut info would probably be beneficial to all users so we should do something that everyone can use. @snide What do you think to about adding a little "info" popover maybe in the top right of the toolbar where we can let people know about all the shortcuts? (Can use either the ⓘ icon or the keyboard icon for the button.) |
Home/End/PageUp/PageDown were added with #2519 |
@myasonik Yep. I can make something like that. I'll add it as a checklist item for this issue. |
- [ ] [alphanumeric keys] if a cell contains an input widget, places focus on the first one, and inserts keySpec reference: Keyboard Interaction For Data Grids
The text was updated successfully, but these errors were encountered: