[EuiSelectable] Handle more keyboard scenarios #5613
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5613/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5613/ |
1Copenut
left a comment
1Copenut
left a comment
There was a problem hiding this comment.
Thanks @thompsongl, code is working really well in keyboard and screen reader tests. I left some suggestions to swap Cypress synthetic events for real events in the spec file. I'm recommending we use real browser events as much as possible, but am 💯 making this a follow on PR if that's more appropriate.
| .should('have.attr', 'title', 'Enceladus'); | ||
| }); | ||
|
|
||
| cy.get('[data-test-subj="clearSearchButton"]') |
There was a problem hiding this comment.
What do you think about duplicating this test for Space keypress? It's not the dryest code I've ever suggested, but I can live with that for the improved Clear button checking.
| }, | ||
| ]; | ||
|
|
||
| describe('EuiSelectable', () => { |
There was a problem hiding this comment.
Great call adding a Cypress spec! I'd like to use cypress-real-events helpers for the click and keyboard interactions as much as possible. I'll add notes to individual lines to change, and I'm 99% sure they'll pass on first try. They worked on my machine anyhow :)
I swapped cy.type(string) for cy.realType(string) || cy.realPress(string) and the cy.click() for cy.realClick() while testing. I opted for realPress when it was a single key, and kept realType for multiple characters like "esc".
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5613/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5613/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5613/ |
3 participants
Summary
Following up on some keyboard-related requests in #5157:
Checklist