-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[core] Fix some errors when porting demos to TypeScript #12417
Conversation
Porting https://github.com/mui-org/material-ui/blob/master/docs/src/pages/demos/autocomplete/IntegrationDownshift.js to typescript does not works in line 281: ``` <Popper open={isOpen} anchorEl={popperNode}> <Paper square style={{ width: popperNode ? popperNode.clientWidth : null }}> ```
Porting https://github.com/mui-org/material-ui/blob/master/docs/src/pages/demos/dialogs/ConfirmationDialog.js to typescript does not works in line 83: ``` <RadioGroup ref={ref => { this.radioGroupRef = ref; }}```
Porting https://github.com/mui-org/material-ui/blob/master/docs/src/pages/demos/text-fields/TextFields.js to typescript does not works in line 106: ``` <TextField id="read-only-input" label="Read Only" defaultValue="Hello World" className={classes.textField} margin="normal" InputProps={{ readOnly: true, }} /> ```
@@ -7,6 +7,7 @@ export interface RadioGroupProps | |||
name?: string; | |||
onChange?: (event: React.ChangeEvent<{}>, value: string) => void; | |||
value?: string; | |||
ref?; |
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.
Something is wrong, ref
is a native React property. We shouldn't have to declare it.
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.
Removed, my mistake
@@ -23,6 +23,7 @@ export interface InputProps | |||
multiline?: boolean; | |||
name?: string; | |||
placeholder?: string; | |||
readOnly?: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.
You can run yarn prettier
to fix the code style.
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.
done
@@ -20,12 +20,12 @@ export type PopperPlacementType = | |||
export interface PopperProps extends React.HTMLAttributes<HTMLDivElement> { | |||
transition?: boolean; | |||
anchorEl?: null | HTMLElement | ReferenceObject | ((element: HTMLElement) => HTMLElement); | |||
children: ( | |||
children: React.ReactElement<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.
It can be a node, I believe we should be using React.ReactNode
.
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.
done
props: { | ||
placement: PopperPlacementType; | ||
TransitionProps?: TransitionProps; | ||
}, | ||
) => React.ReactElement<any>; | ||
) => React.ReactElement<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.
React.ReactNode
most likely.
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.
done
@PavelPZ Thank you |
Porting https://github.com/mui-org/material-ui/tree/master/docs/src/pages/demos to typescript arrise some errors, see my commit's comments.
Thanks for merging