-
Notifications
You must be signed in to change notification settings - Fork 87
Rewrite in Typescript: RadioGroup #308
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
Conversation
className?: string; | ||
label?: string; | ||
required?: boolean; | ||
error?: any; // FIXME: should be FormElementProps.error |
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.
Fix this after FormElement is converted to TypeScript
if (this.props.onChange) { | ||
this.props.onChange(e, value); | ||
} | ||
} | ||
|
||
renderControl(radio) { | ||
renderControl(radio: any) { |
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.
I couldn't figure out how I can give more strict type annotation here and onChange
prop.
It seems children
are expected to only <Radio>
component, but I didn't know how I express this.
@@ -60,7 +72,6 @@ export default class RadioGroup extends React.Component { | |||
: undefined | |||
: undefined; | |||
|
|||
delete props.onChange; |
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 TypeScript doesn't recognize that a certain property is deleted, a type error occured here
<fieldset className={grpClassNames} style={grpStyles} {...props}>
(related to this? microsoft/TypeScript#13783)
So I used destructuring assignment above.
Closed because the last PR #342 had been merged. |
ref. #289
TODO: after #307 is merged, change the base branch to
3.0
.