-
Notifications
You must be signed in to change notification settings - Fork 59
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
[FEAT] migrate atom/label to TS #2738
Conversation
components/atom/label/src/index.tsx
Outdated
@@ -0,0 +1,57 @@ | |||
/* eslint-disable react/prop-types */ |
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.
We need to update the lint config for TS files in order to remove this rule, not needed anymore.
|
|
components/atom/label/mod.d.ts
Outdated
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.
Pending to be moved to root directory.
components/atom/label/index.d.ts
Outdated
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.
Pending to be moved to root directory.
components/atom/label/src/index.tsx
Outdated
*/ | ||
text: string | ||
/** | ||
* Allows label to be displayed inline to de left |
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.
Typo Allows label to be displayed inline to de left
-> Allows label to be displayed inline to the left
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.
Copypasted, sorry 😂
components/atom/label/src/index.tsx
Outdated
/** | ||
* Allows label to be displayed inline to de left | ||
*/ | ||
inline?: string |
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 see in the code that this prop can be either left or right so maybe we can add some Type checking with these 2 values instead of using a string.
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.
Yep, it makes sense, it's not used for any other reason ;)
|
|
Description
Migrate the first component (
AtomLabel
) to TypeScript. Behaviour of the component remains the same.Type of changes
Complete list of changes:
AtomLabel
migration to TypeScript.