Skip to content

Expand table's default action type to allow reactnode names#3688

Merged
chandlerprall merged 3 commits intoelastic:masterfrom
chandlerprall:bug/3679-custom-action-renderers
Jul 2, 2020
Merged

Expand table's default action type to allow reactnode names#3688
chandlerprall merged 3 commits intoelastic:masterfrom
chandlerprall:bug/3679-custom-action-renderers

Conversation

@chandlerprall
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

@snide updated the action's props table

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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