Add types for button components #4071
Conversation
| return isSmall ? ( | ||
| <Fab | ||
| component={Link} | ||
| color="primary" | ||
| className={classnames(classes.floating, className)} | ||
| to={`${basePath}/create`} | ||
| aria-label={label && translate(label)} | ||
| {...rest} | ||
| {...rest as any} |
There was a problem hiding this comment.
I had to disable type checking here because of a material-ui bug: Button doesn't recognize the component prop :/
|
to improve readability for non-ts readers, I chose to move types to the end of each file. I did the same for secondary functions, so that a component file starts by the component definition. |
| ...rest | ||
| }: any) => rest; | ||
|
|
||
| interface Props { |
There was a problem hiding this comment.
We should probably extract the two first properties in a reusable interface for all our components
There was a problem hiding this comment.
I think the code is hard enough to read like that, I prefer a bit of duplication to a lot of indirection.
| { name: 'RaDeleteWithConfirmButton' } | ||
| ); | ||
|
|
||
| interface Props { |
There was a problem hiding this comment.
Lots of duplication with DeleteButton. Couldn't we improve that ?
There was a problem hiding this comment.
That'd imply extracting the common part to another file (to avoid circular dependencies), and that would make all the Delete components harder to read... Again, I prefer some duplication for clarity.
| { name: 'RaDeleteWithUndoButton' } | ||
| ); | ||
|
|
||
| interface Props { |
In the process of making the code more robust at compile time, I converted most Button components to TypeScript.
It took me no less than 6 hours. Let it be said that I don't like TypeScript.