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

[EuiDatePicker] defaultProps types #3427

Merged

Conversation

thompsongl
Copy link
Contributor

@thompsongl thompsongl commented May 5, 2020

Summary

Resolves a type mismatch problem between the exported component interface (EuiDatePickerProps) and defaultProps interface in EuiDatePicker.

The component makes use of ApplyClassComponentDefaults, which in this case applies the inferred default type of popoverPlacement (string | undefined) over the explicit type defined in _EuiDatePickerProps (union type).

Resolved by using as const with the popoverPlacement default value.

(Kibana extends this component and the ...rest interface errors as a mistype)

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 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

@thompsongl thompsongl changed the title direct defaultProps typing [EuiDatePicker] defaultProps types May 5, 2020
@kibanamachine
Copy link

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

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Needs a changelog, but otherwise this LGTM

@kibanamachine
Copy link

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

@thompsongl thompsongl merged commit 3a75f45 into elastic:master May 5, 2020
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