-
Notifications
You must be signed in to change notification settings - Fork 0
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
TS updates #34
TS updates #34
Conversation
Ahhh... hmmm. So I was hoping that EuiButtonDisplay would try to be as element agnostic as possible so that the consuming component, EuiButton, would be passing the appropriate combination of props, ie |
I can take another pass move the logic around. |
Yeah I think the |
Looking again, if |
At some point |
Since EuiButtonDisplay is an internal component only, I'd say let's TS ignore the ref/return in THAT component, but then ensure that EuiButton is appropriately assigning props to HTML element |
@@ -102,7 +102,7 @@ export interface EuiButtonProps extends CommonProps { | |||
} | |||
|
|||
export interface EuiButtonDisplayProps extends EuiButtonProps { | |||
element: 'a' | 'button' | 'label'; | |||
element: keyof JSX.IntrinsicElements; |
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.
Out of curiosity, what is this/does this mean?
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.
Also, would React.createElement
help here in any way?
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.
keyof JSX.IntrinsicElements
It will allow for basically any HTML element
Also, would React.createElement help here in any way?
That's an interesting idea. Let me try it out; I'm not actually sure what TS will do it.
disabled
is not available ona
andlabel
, but it looks like the styling is handled. I didn't visually test, though