Skip to content
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

Merged
merged 16 commits into from
May 15, 2024
Merged

[FEAT] migrate atom/label to TS #2738

merged 16 commits into from
May 15, 2024

Conversation

kikoruiz
Copy link
Member

@kikoruiz kikoruiz commented May 13, 2024

Description

Migrate the first component (AtomLabel) to TypeScript. Behaviour of the component remains the same.

Type of changes

  • 🧠 Refactor
  • πŸ› οΈ Tool

Complete list of changes:

  • AtomLabel migration to TypeScript.
  • Add new CI job for typechecking.
  • Define missing types and module declarations for both 3rd-party and our own packages.
  • Adapt some TS configurations.

@@ -0,0 +1,57 @@
/* eslint-disable react/prop-types */
Copy link
Member Author

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.

Copy link

STATEMENTS BRANCHES FUNCTIONS LINES
≍ ≍ 0.05↓ ≍ 0.02↓ ≍ 0.07↓ ≍ 0.07↓
% 75.96 64.37 65.95 77.73
ABS 3337 / 4393 2044 / 3175 653 / 990 3159 / 4064

Copy link

STATEMENTS BRANCHES FUNCTIONS LINES
≍ ≍ 0.05↓ ≍ 0.02↓ ≍ 0.07↓ ≍ 0.07↓
% 75.96 64.37 65.95 77.73
ABS 3337 / 4393 2044 / 3175 653 / 990 3159 / 4064

Copy link
Member Author

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.

Copy link
Member Author

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.

@kikoruiz kikoruiz changed the title [FEAT] migrate atom label to TS [FEAT] migrate atom/label to TS May 14, 2024
*/
text: string
/**
* Allows label to be displayed inline to de left
Copy link
Collaborator

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copypasted, sorry πŸ˜‚

/**
* Allows label to be displayed inline to de left
*/
inline?: string
Copy link
Collaborator

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.

Copy link
Member Author

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 ;)

Copy link

STATEMENTS BRANCHES FUNCTIONS LINES
≍ ≍ 0.05↓ ≍ 0.02↓ ≍ 0.07↓ ≍ 0.07↓
% 75.96 64.37 65.95 77.73
ABS 3337 / 4393 2044 / 3175 653 / 990 3159 / 4064

Copy link

STATEMENTS BRANCHES FUNCTIONS LINES
≍ ≍ 0.05↓ ≍ 0.02↓ ≍ 0.07↓ ≍ 0.07↓
% 75.96 64.37 65.95 77.73
ABS 3337 / 4393 2044 / 3175 653 / 990 3159 / 4064

@kikoruiz kikoruiz merged commit efb1fb3 into master May 15, 2024
7 checks passed
@kikoruiz kikoruiz deleted the migrate-atom-label-to-ts branch May 15, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants