Skip to content
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

[EuiSelectableTemplateSitewide] Pass keyboard/mouse event to onChange handler #5926

Merged
merged 4 commits into from
May 25, 2022

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented May 24, 2022

Summary

This PR attempts to partially address #5408 - or at least unblock Kibana so they can get a basic 'open this link if the user was holding down the shift/cmd key' behavior.

This is not intended to be a best/perfect solution, which would involve changing the API & markup of EuiSelectable significantly to allow <a> links and native a link behavior. This is also not a perfect screen reader solution as Kibana will likely need to provide some hidden or visible text hint to users that shift+click is even an option.

QA

  • Go to https://eui.elastic.co/pr_5926/#/templates/sitewide-search
  • Click into input to open selectable popover
  • Hold down the shift key and click any item. Confirm an alert pops up
  • Release the shift key and click any item. Confirm an alert does not pop up
  • Use the up/arrow keys to start keyboard navigating
  • Hold down the shift key and press the Enter key. Confirm an alert pops up
  • Release the shift key and press the Enter key. Confirm an alert does not pop up

Checklist

- [ ] Checked in both light and dark modes
- [ ] Checked in mobile
- [ ] Added documentation
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Checked for breaking changes and labeled appropriately
- [ ] Checked for accessibility including keyboard-only and screenreader modes
- [ ] Updated the Figma library counterpart

  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs and playground toggles
  • Added or updated jest and cypress tests
  • A changelog entry exists and is marked appropriately
  • Revert [REVERT ME] commit

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5926/

@cee-chen cee-chen marked this pull request as ready for review May 24, 2022 20:56
@cee-chen cee-chen requested a review from 1Copenut May 24, 2022 20:57
@cee-chen
Copy link
Contributor Author

@1Copenut do you mind code reviewing this real quick? I don't think there's any accessibility implications here, so this should be a fairly quick QA / stopgap solution for Kibana until we get a chance to refactor EuiSelectable to allow <a> tags.

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I tested briefly with keyboard and VoiceOver. No regression and the behavior worked as expected.

Your comment about Kibana needing to inform users of the new behavior is excellent.

…e` event to react to shift key"

This reverts commit 1d893f4.
@cee-chen cee-chen enabled auto-merge (squash) May 25, 2022 15:07
@cee-chen cee-chen disabled auto-merge May 25, 2022 15:07
@cee-chen cee-chen enabled auto-merge (squash) May 25, 2022 15:07
@cee-chen
Copy link
Contributor Author

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5926/

@cee-chen cee-chen merged commit 33fd48f into elastic:main May 25, 2022
@cee-chen cee-chen deleted the sitewide-search/shift-clicks branch May 25, 2022 17:10
cee-chen pushed a commit to cee-chen/eui that referenced this pull request May 26, 2022
… handler (elastic#5926)

* Enhance EuiSelectable to send back click/keyboard events

- which will allow consumers to, e.g. react to shift clicks

* [REVERT ME] Example of how Kibana would update their `onChange` event to react to shift key

* changelog

* Revert "[REVERT ME] Example of how Kibana would update their `onChange` event to react to shift key"

This reverts commit 1d893f4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants