Skip to content

Conversation

@sungik-choi
Copy link
Contributor

@sungik-choi sungik-choi commented Feb 10, 2023

Self Checklist

  • I wrote a PR title in English.
  • I added an appropriate label to the PR.
  • I wrote a commit message in English.
  • I wrote a commit message according to the Conventional Commits specification.
  • I added the appropriate changeset for the changes.
  • [Component] I wrote a unit test about the implementation.
  • [Component] I wrote a storybook document about the implementation.
  • [Component] I tested the implementation in various browsers.
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox
  • [New Component] I added my username to the correct directory in the CODEOWNERS file.

Related Issue

없음

Summary

RadioGroup, Radio 컴포넌트를 개선합니다.

Details

  • 컴포넌트 타입에 제네릭을 적용합니다. stringenum 등의 케이스에서도 타입 에러 없이 사용할 수 있도록 합니다.
  • RadioGroupchildren 타입을 React.ReactNode 로 변경합니다. 다양한 유즈케이스에서도 타입 에러 없이 잘 동작하도록 변경합니다.
  • Radio 에 children이 존재하는 케이스에만 내부 라벨을 렌더하도록 변경합니다. 라벨이 없는 유즈케이스를 커버하기 위한 변경입니다.
  • chore: 레거시 라디오 컴포넌트에 deprecated 주석을 추가합니다.

Breaking change or not (Yes/No)

No

References

없음

@sungik-choi sungik-choi added enhancement Issues or PR related to making existing features better refactoring Issue or PR related to refactoring with no functional changes labels Feb 10, 2023
@sungik-choi sungik-choi self-assigned this Feb 10, 2023
@changeset-bot
Copy link

changeset-bot bot commented Feb 10, 2023

🦋 Changeset detected

Latest commit: 50bfcf5

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

This PR includes changesets to release 2 packages
Name Type
@channel.io/bezier-react Patch
bezier-figma-plugin 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

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Base: 72.88% // Head: 72.87% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (50bfcf5) compared to base (137c2a4).
Patch coverage: 71.42% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           next-v1    #1131      +/-   ##
===========================================
- Coverage    72.88%   72.87%   -0.02%     
===========================================
  Files          366      366              
  Lines         4404     4405       +1     
  Branches       854      855       +1     
===========================================
  Hits          3210     3210              
  Misses        1119     1119              
- Partials        75       76       +1     
Impacted Files Coverage Δ
.../bezier-react/src/components/Forms/Radio/Radio.tsx 85.00% <ø> (ø)
...react/src/components/Forms/FormGroup/FormGroup.tsx 94.44% <50.00%> (-5.56%) ⬇️
...act/src/components/Forms/RadioGroup/RadioGroup.tsx 90.00% <66.66%> (ø)
...er-react/src/components/Forms/RadioGroup/Radio.tsx 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023

Chromatic Report

🚀 Congratulations! Your build was successful!

@sungik-choi sungik-choi changed the title refactor(radio-group): apply generic value type to component props Enhance RadioGroup component Feb 12, 2023
@sungik-choi sungik-choi marked this pull request as ready for review February 12, 2023 13:29
@sungik-choi sungik-choi requested review from Dogdriip, annie1229 and yangwooseong and removed request for annie1229 February 13, 2023 05:02
* </RadioGroup>
* ```
*/
export const RadioGroup = forwardRef(RadioGroupImpl) as <Value extends string>(
Copy link
Contributor

@yangwooseong yangwooseong Feb 13, 2023

Choose a reason for hiding this comment

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

여기서 generic 이 제대로 동작안해서 assertion 이 필요한 이유가 궁금해서 찾아봤더니 이런 이유가 있었네요..! (#)
#1126 이것도 assertion 을 주거나 여기 나온 다른 방법을 써야할 듯 하군요

We only make higher order function type inferences when the source and target types are both pure function types, i.e. types with a single call signature and no other members.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 공유 감사합니다. 저도 꼼꼼히 확인해봐야할듯.

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

Labels

enhancement Issues or PR related to making existing features better refactoring Issue or PR related to refactoring with no functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants