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

Migrate useStep and Step to TypeScript #1505

Merged
merged 6 commits into from
Apr 5, 2022
Merged

Conversation

connor-baer
Copy link
Member

@connor-baer connor-baer commented Apr 4, 2022

Addresses #516.

Purpose

When I started to remove the use of lodash in Circuit UI, I quickly found that it was used heavily in the step component and hook. I took the opportunity to migrate them over to TypeScript.

Approach and changes

  • Migrate the useStep hook and Step component to TypeScript. The useStep hook now has the same default options as the Step component (⚠️ technically, this is a breaking change, although it doesn't change the behavior of the hook as far as I can tell)
  • Add a missing accessibility label to the Carousel component

Definition of done

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

@connor-baer connor-baer requested a review from a team as a code owner April 4, 2022 12:11
@connor-baer connor-baer requested review from amelako and removed request for a team April 4, 2022 12:11
@vercel
Copy link

vercel bot commented Apr 4, 2022

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/Evp317yNXChK9CMsPp6CqZGK6eHL
✅ Preview: https://oss-circuit-ui-git-feature-step-typescript.sumup-vercel.app

@changeset-bot
Copy link

changeset-bot bot commented Apr 4, 2022

🦋 Changeset detected

Latest commit: 42ce33a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sumup/circuit-ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@connor-baer connor-baer marked this pull request as draft April 4, 2022 12:12
@codecov
Copy link

codecov bot commented Apr 4, 2022

Codecov Report

Merging #1505 (42ce33a) into main (2f7537f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1505   +/-   ##
=======================================
  Coverage   92.33%   92.33%           
=======================================
  Files         201      202    +1     
  Lines        4213     4213           
  Branches     1321     1351   +30     
=======================================
  Hits         3890     3890           
+ Misses        304      303    -1     
- Partials       19       20    +1     
Impacted Files Coverage Δ
...ackages/circuit-ui/components/Carousel/Carousel.js 100.00% <ø> (ø)
packages/circuit-ui/components/Step/Step.ts 100.00% <100.00%> (ø)
packages/circuit-ui/components/Step/StepService.ts 100.00% <100.00%> (ø)
...ckages/circuit-ui/components/Step/hooks/useStep.ts 100.00% <100.00%> (ø)
packages/circuit-ui/util/helpers.ts 100.00% <100.00%> (ø)
packages/circuit-ui/util/type-check.ts 100.00% <100.00%> (ø)
...es/circuit-ui/components/Pagination/Pagination.tsx 91.42% <0.00%> (ø)

Copy link
Contributor

@amelako amelako left a comment

Choose a reason for hiding this comment

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

🌟

@connor-baer connor-baer merged commit bbd20fd into main Apr 5, 2022
@connor-baer connor-baer deleted the feature/step-typescript branch April 5, 2022 10:47
@github-actions github-actions bot mentioned this pull request Apr 5, 2022
@connor-baer connor-baer mentioned this pull request Apr 5, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants