-
Notifications
You must be signed in to change notification settings - Fork 843
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
Add direction prop for EuiFlexGroup typing #1196
Add direction prop for EuiFlexGroup typing #1196
Conversation
src/components/flex/index.d.ts
Outdated
component?: FlexGroupCmponentType; | ||
direction?: FlexGroupDirection; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you're in this file already, mind adding the other missing prop type definitions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing 👍
src/components/flex/index.d.ts
Outdated
@@ -43,11 +48,12 @@ declare module '@elastic/eui' { | |||
export type FlexGroupCmponentType = 'div' | 'span'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chandlerprall should we rename this type to FlexGroupComponentType
? At first I thought it was an abbreviation but now I'm wondering if it wasn't meant to be that name all along?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small type change
src/components/flex/index.d.ts
Outdated
alignItems?: FlexGroupAlignItems; | ||
children?: React.ReactNode; | ||
className?: FlexGroupClassName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
className should be a string type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely.. I read over className
and saw componentType
was PropTypes.oneOf(['div', 'span'])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Summary
Add support for the
direction
prop of theEuiFlexGroup
component for TypeScript.Checklist
Note: any checks not completed were deemed unnecessary by me. Please let me know if any further check work needs to be done before this is ready to merge.