Skip to content

Conversation

@chandlerprall
Copy link
Contributor

@chandlerprall chandlerprall commented Jul 2, 2020

Summary

Creates a work-around path for #3679 by expanding EuiBasicTable's default action's name to allow any React node.

  • updated actions example to include react nodes in action names
  • tested aria-label -> aria-labelledby change in Mac VoiceOver + Chrome
  • added unit test

This does not fix the bug reported in #3679 where 3+ custom actions will trigger a React warning about nested button elements.

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked in IE11 and Firefox
- [ ] Props have proper autodocs
- [ ] Added documentation
- [ ] Checked Code Sandbox works for the any docs examples

  • Added or updated jest tests
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Functionally works. I think you might have a small docs issue

@chandlerprall
Copy link
Contributor Author

@snide updated the action's props table

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

LGTM; tested locally with a more complex name element (tooltip like what ML will do)

@kibanamachine
Copy link

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

@chandlerprall chandlerprall merged commit 03b2249 into elastic:master Jul 2, 2020
@chandlerprall chandlerprall deleted the bug/3679-custom-action-renderers branch July 2, 2020 17:42
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.

4 participants