TASK: Convert Badge to Typescript #2106
Conversation
let's see if this works
| import React from 'react'; | ||
| import mergeClassNames from 'classnames'; | ||
|
|
||
| export interface BadgeProps { |
There was a problem hiding this comment.
Somehow I want to have this called Props, but I did it because of consistency
There was a problem hiding this comment.
Let's call it Props then. Also why do we export it? Let's not export until we actually need it.
| /** | ||
| * An optional `className` to attach to the wrapper. | ||
| */ | ||
| className: string | null | undefined; |
There was a problem hiding this comment.
This feels a bit weird, wouldn't it be enough to check for two of these values in the future? (null OR undefined)
There was a problem hiding this comment.
For some reason when I tried className?: string (which is the equivalent to className: string | undefined) it complained about typo error, because themr was providing it as a possible return type... but when I'm trying it out locally it works just fine... So try className?: string and see if it complains.
| /** | ||
| * An optional `className` to attach to the wrapper. | ||
| */ | ||
| className: string | null | undefined; |
There was a problem hiding this comment.
For some reason when I tried className?: string (which is the equivalent to className: string | undefined) it complained about typo error, because themr was providing it as a possible return type... but when I'm trying it out locally it works just fine... So try className?: string and see if it complains.
| import React from 'react'; | ||
| import mergeClassNames from 'classnames'; | ||
|
|
||
| export interface BadgeProps { |
There was a problem hiding this comment.
Let's call it Props then. Also why do we export it? Let's not export until we actually need it.
| }; | ||
| } | ||
|
|
||
| const Badge: React.SFC<BadgeProps> = ({ |
There was a problem hiding this comment.
Since I fixed the themr package, it's now possible to use proper react classes again. Could you give it a try, since before we have agreed not to use SFC's?
| */ | ||
| theme: PropTypes.shape({ | ||
| badge: PropTypes.string | ||
| }).isRequired |
There was a problem hiding this comment.
Why was the theme required before? I think it should indeed be optional.
|
#2128 makes this obsolete, therefore I close this. |
let's see if this works