Skip to content

EuiDroppable now accepts multiple children (TypeScript)#2634

Merged
thompsongl merged 1 commit intoelastic:masterfrom
seeruk:master
Dec 12, 2019
Merged

EuiDroppable now accepts multiple children (TypeScript)#2634
thompsongl merged 1 commit intoelastic:masterfrom
seeruk:master

Conversation

@seeruk
Copy link
Copy Markdown
Contributor

@seeruk seeruk commented Dec 11, 2019

Summary

The type definitions for EuiDroppable don't seem to allow multiple children. You can't use examples from the docs, without surrounding multiple children in their own <></> fragment node - this change attempts to remedy that issue.

I'll be honest, I'm not sure if ReactElement[] is the best type for this, I did try to use ReactNode, but that prevented other parts of the type definition from working, whereas ReactElement[] didn't stop anything else from working.

A test case has been added.

Checklist

- [x] Checked in dark mode
- [x] Checked in mobile
- [x] Checked in IE11 and Firefox
- [x] Props have proper autodocs
- [x] Added documentation examples
- [x] Checked for breaking changes and labeled appropriately
- [x] Checked for accessibility including keyboard-only and screenreader modes

  • Added or updated jest tests
  • A changelog entry exists and is marked appropriately

@elasticmachine
Copy link
Copy Markdown
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@snide
Copy link
Copy Markdown
Contributor

snide commented Dec 11, 2019

Thanks for the contribution! We'll get someone to give a review.

@snide snide requested a review from thompsongl December 11, 2019 21:46
@thompsongl
Copy link
Copy Markdown
Contributor

jenkins test this

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.

Thanks, @seeruk! ReactElement[] appears to be the correct way to handle this.

@seeruk
Copy link
Copy Markdown
Contributor Author

seeruk commented Dec 12, 2019

Just updated to resolve the conflict with the changelog.

@thompsongl
Copy link
Copy Markdown
Contributor

jenkins test this

@thompsongl thompsongl merged commit 3516212 into elastic:master Dec 12, 2019
thompsongl added a commit that referenced this pull request Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants