-
Notifications
You must be signed in to change notification settings - Fork 842
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 TypeScript definitions for EuiRange and EuiRadio #1253
Conversation
Also correct the definitions for EuiRadioGroup.
|
||
export type EuiRadioGroupProps = CommonProps & | ||
Omit<HTMLAttributes<HTMLDivElement>, 'onChange'> & { | ||
disabled?: boolean; |
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.
Please add disabled
and name
to the radio group's proptypes as well, so they're included in the documentation & runtime checks.
src/components/form/radio/index.d.ts
Outdated
} | ||
|
||
export const EuiRadio: SFC< | ||
CommonProps & InputHTMLAttributes<HTMLInputElement> & EuiRadioProps |
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.
EuiRadio
applies its extra props to a div
, not any kind of input. The props here need to reflect that with HTMLAttributes<HTMLDivElement>
. This also means the input-related props (checked, value, disabled, autoFocus) on EuiRadio
need to be added to EuiRadioProps
. The onChange
definition is correct as-is (other than not being an override after the other changes), because it is forwarded to the underlying input
.
showTicks?: boolean; | ||
showValue?: boolean; | ||
tickInterval?: number; | ||
} |
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.
min
and max
are required in EuiRange
but are optional in InputHTMLAttributes
, so they must be overridden here. Also, our definition & usage of step
requires it to be a number, whereas InputHTMLAttributes
allows number or string values, so step
requires an override too.
@chandlerprall I believe I've addressed all your comments. Please take another look. |
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, thanks!
Add TypeScript definitions for
EuiRange
andEuiRadio
, and correct the definitions forEuiRadioGroup
.