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

Change the as prop to allow all element types and button to allow as prop #1899

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

sungik-choi
Copy link
Contributor

@sungik-choi sungik-choi commented Jan 10, 2024

Self Checklist

  • I wrote a PR title in English and added an appropriate label to the PR.
  • I wrote the commit message in English and to follow the Conventional Commits specification.
  • I added the changeset about the changes that needed to be released. (or didn't have to)
  • I wrote or updated documentation related to the changes. (or didn't have to)
  • I wrote or updated tests related to the changes. (or didn't have to)
  • I tested the changes in various browsers. (or didn't have to)
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox

Related Issue

Summary

Change the as prop to allow all element types and button to allow as prop

Details

  • as 속성이 모든 엘리먼트 타입을 지원하도록 변경합니다. 단순 HTML element type 외에도, Link 컴포넌트를 as 속성에 주입하는 등의 케이스가 있습니다. 이를 지원하여 breaking change를 줄이기 위한 변경입니다.
  • Button 또한 마찬가지로 Link, a 태그 등으로 사용되는 케이스들이 있습니다. as 속성을 지원하도록 개선했습니다.

Breaking change? (Yes/No)

No

@sungik-choi sungik-choi added the enhancement Issues or PR related to making existing features better label Jan 10, 2024
@sungik-choi sungik-choi self-assigned this Jan 10, 2024
Copy link

changeset-bot bot commented Jan 10, 2024

⚠️ No Changeset found

Latest commit: 690bbc6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes changesets to release 1 package
Name Type
@channel.io/bezier-react Major

Click here to learn what changesets are, and how to add one.

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

Copy link
Contributor

github-actions bot commented Jan 10, 2024

Chromatic Report

🚀 Congratulations! Your build was successful!

@@ -113,6 +114,8 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(function Button
onClick,
...rest
}, forwardedRef) {
const Comp = as as 'button'
Copy link
Collaborator

Choose a reason for hiding this comment

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

타입 에러때문에 단언을 하신걸까요?

Suggested change
const Comp = as as 'button'
const Comp = as

이렇게만 해도 에러가 안뜨네요. 아니면 L101에서 as: Comp = 'button' 으로 해도 될거같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

에러는 안뜨는데, any로 추론되어 jsx 부분에서 컴파일러가 제대로 동작하지 않는 문제가 있어 추가했어요!

Copy link

codecov bot commented Jan 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0f9ee21) 86.40% compared to head (690bbc6) 86.41%.

Additional details and impacted files
@@           Coverage Diff           @@
##            alpha    #1899   +/-   ##
=======================================
  Coverage   86.40%   86.41%           
=======================================
  Files         249      249           
  Lines        3420     3422    +2     
  Branches      747      748    +1     
=======================================
+ Hits         2955     2957    +2     
  Misses        384      384           
  Partials       81       81           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sungik-choi sungik-choi merged commit 996fa9a into channel-io:alpha Jan 11, 2024
11 checks passed
@sungik-choi sungik-choi deleted the feat/loose-as branch January 11, 2024 09:09
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
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants