Skip to content

feat(components): Migrate Badge component to typescript #624

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 23, 2020
Merged

feat(components): Migrate Badge component to typescript #624

merged 5 commits into from
Jun 23, 2020

Conversation

hleote
Copy link
Contributor

@hleote hleote commented Jun 23, 2020 •
edited
Loading

Addresses #516

Purpose

Increase the usage of TypeScript in our code base.

Approach and changes

  • Migrated Badge component to TypeScript

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests
  • Meets minimum browser support
  • Meets accessibility requirements

@vercel
Copy link

vercel bot commented Jun 23, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sumup-oss/circuit-ui/m6idiny2r
✅ Preview: https://circuit-ui-git-tsrefactor-badge-component.sumup-oss.vercel.app

@hleote hleote changed the title Ts refactor/badge component feat(components): migrate Badge component to typescript Jun 23, 2020
@hleote hleote changed the title feat(components): migrate Badge component to typescript feat(components): Migrate Badge component to typescript Jun 23, 2020
@herberthenrique herberthenrique mentioned this pull request Jun 23, 2020
Closed
49 tasks
@hleote hleote self-assigned this Jun 23, 2020
@hleote hleote added the feature A new feature or enhancement label Jun 23, 2020
@hleote hleote added this to the v2.0 milestone Jun 23, 2020
@codecov
Copy link

codecov bot commented Jun 23, 2020

Codecov Report

Merging #624 into beta will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             beta     #624      +/-   ##
==========================================
- Coverage   93.87%   93.84%   -0.04%     
==========================================
  Files         135      135              
  Lines        2203     2208       +5     
  Branches      631      632       +1     
==========================================
+ Hits         2068     2072       +4     
- Misses        112      113       +1     
  Partials       23       23
Impacted Files Coverage Δ
src/components/Badge/Badge.tsx 96.29% <100.00%> (ø)

connor-baer
connor-baer approved these changes Jun 23, 2020
View reviewed changes
Copy link
Member

@connor-baer connor-baer left a comment

Choose a reason for hiding this comment

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

Fantastic! 💯

src/components/Badge/__snapshots__/Badge.spec.tsx.snap
@@ -130,8 +130,8 @@ exports[`Badge should have hover/active styles only when the onClick handler is
border: 0;
}

<div
class="circuit-0 circuit-1"
<button
Copy link
Member

@connor-baer connor-baer Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

😍 I get really excited about semantic markup

Copy link
Contributor Author

@hleote hleote Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@connor-baer any idea why is the codecov complaining? 🤔 🙂

Copy link
Member

@connor-baer connor-baer Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You can view the coverage diff by clicking on the "Impacted files" in the comment above. Looks like it's this line in the Badge.tsx file:

Captura de tela 2020-06-23 10 22 28

The !currentColor condition is made impossible by TypeScript, but I'd leave it in for projects that use JavaScript. So, ignore this coverage drop and merge anyway :)

marielakas
marielakas approved these changes Jun 23, 2020
View reviewed changes
Copy link
Contributor

@marielakas marielakas left a comment

Choose a reason for hiding this comment

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

🚀

@hleote hleote merged commit f90c47d into beta Jun 23, 2020
@hleote hleote deleted the tsRefactor/badge-component branch June 23, 2020 09:06
robinmetral
robinmetral reviewed Jun 23, 2020
View reviewed changes
@robinmetral
Copy link
Contributor

Oops sorry, 2 minutes late 🙈

@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.0.0-beta.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

@connor-baer
Copy link
Member

@robinmetral Thanks for the thorough review! 🙌

@hleote Do you wanna address these points in the next PR to update the design?

@hleote
Copy link
Contributor Author

hleote commented Jun 23, 2020

@robinmetral Thanks for the thorough review! 🙌

@hleote Do you wanna address these points in the next PR to update the design?

Yeah that's probably a good idea...
Can you send me issue/mockups for the design update?

Thanks for the review @robinmetral 🙂 👍

@connor-baer
Copy link
Member

Can you send me issue/mockups for the design update?

Here you go: https://www.figma.com/file/aORNKPp6e3xbpkYrkfoVhW/WIP-Circuit-UI-Mobile?node-id=1802%3A479

@connor-baer
Copy link
Member

@robinmetral I addressed your PR comments in #633.

@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@connor-baer connor-baer connor-baer approved these changes

@robinmetral robinmetral robinmetral left review comments

@marielakas marielakas marielakas approved these changes

Assignees

@hleote hleote

Labels
feature A new feature or enhancement
Projects
None yet
Milestone
v2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@hleote @robinmetral @connor-baer @marielakas