Add typescript definitions for more components #326
Conversation
src/components/form/radio/index.d.ts
Outdated
| Omit<HTMLAttributes<HTMLDivElement>, 'onChange'> & { | ||
| options?: EuiRadioGroupOption[]; | ||
| idSelected?: string; | ||
| onChange: (id: string) => void; |
There was a problem hiding this comment.
should be of type EuiRadioGroupChangeCallback ?
|
|
||
| export type EuiRadioGroupProps = CommonProps & | ||
| Omit<HTMLAttributes<HTMLDivElement>, 'onChange'> & { | ||
| options?: EuiRadioGroupOption[]; |
There was a problem hiding this comment.
It is listed in defaultProps though, which suggests the intention is for it to be optional.
There was a problem hiding this comment.
good point, I didn't see the default props... it's still marked as a required propType (not sure how that behaves along with the default prop... would it still shout if one is not supplied?). In any case, we need to have a decision - it's either required or optional with a default... it make little sense to be both :)
/cc @snide ^
There was a problem hiding this comment.
The defaultProps lead to isRequired not shouting.
|
|
||
| export type EuiLoadingChartProps = CommonProps & | ||
| HTMLAttributes<HTMLDivElement> & { | ||
| mono?: boolean; |
There was a problem hiding this comment.
shall we take this opportunity to also add mono to the propTypes?
src/components/progress/index.d.ts
Outdated
| | 'accent' | ||
| | 'danger' | ||
| | 'primary' | ||
| | 'secondar' |
| color?: EuiProgressColor; | ||
| position?: EuiProgressPosition; | ||
| max?: number; | ||
| }; |
There was a problem hiding this comment.
shall we use this opportunity to remove unused indeterminate from propTypes?
There was a problem hiding this comment.
Looks like that should be a safe change. The tests did not cover it anyway. @snide, any objections?
There was a problem hiding this comment.
Indeterminate progress bars were still being used last time I checked. Why do you want to remove it?
There was a problem hiding this comment.
The property does not seem to be used anywhere in the component itself. It is passed on to the <progress /> (as part of {...rest}), but the html specs don't recognize that attribute either. Instead the react code interprets the lack of a max property as being indeterminate and the html spec uses the value attribute for that. That's something we might want to align as well.
|
@uboness, thanks for the great feedback. I made the changes except for the optional |
weltenwort
force-pushed
the
enhancement/ts-types-3
branch
from
c593b30 to
ade6fb6
Compare
This adds type definitions for
<EuiFormRow><EuiRadioGroup><EuiSwitch><EuiLoadingSpinner><EuiLoadingChart><EuiProgress>