channel-io / bezier-react

React components library that implements Bezier design system.
https://main--62bead1508281287d3c94d25.chromatic.com
Apache License 2.0
181 stars 45 forks source link

Allow `iconName` prop of `Button`, `Banner`, `SectionLabel` component to include `BezierIcon` type #1562

Closed yangwooseong closed 8 months ago

yangwooseong commented 9 months ago

Self Checklist

Related Issue

Partial Fix of #762

Summary

Details

Breaking change or not (Yes/No)

References

changeset-bot[bot] commented 9 months ago

🦋 Changeset detected

Latest commit: f32961b2b1b7f8737053467a8e5a637469c2af58

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

This PR includes changesets to release 3 packages | Name | Type | | -------------------------- | ----- | | @channel.io/bezier-codemod | Minor | | @channel.io/bezier-react | Minor | | 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

github-actions[bot] commented 9 months ago

Chromatic Report

🚀 Congratulations! Your build was successful!

codecov[bot] commented 9 months ago

Codecov Report

Patch coverage: 78.12% and project coverage change: -0.18% :warning:

Comparison is base (0d54036) 87.18% compared to head (f32961b) 87.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1562 +/- ## ========================================== - Coverage 87.18% 87.00% -0.18% ========================================== Files 281 281 Lines 3855 3879 +24 Branches 809 817 +8 ========================================== + Hits 3361 3375 +14 - Misses 421 430 +9 - Partials 73 74 +1 ``` | [Files Changed](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io) | Coverage Δ | | |---|---|---| | [...bezier-react/src/components/Banner/Banner.types.ts](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io#diff-cGFja2FnZXMvYmV6aWVyLXJlYWN0L3NyYy9jb21wb25lbnRzL0Jhbm5lci9CYW5uZXIudHlwZXMudHM=) | `100.00% <ø> (ø)` | | | [...bezier-react/src/components/Button/Button.types.ts](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io#diff-cGFja2FnZXMvYmV6aWVyLXJlYWN0L3NyYy9jb21wb25lbnRzL0J1dHRvbi9CdXR0b24udHlwZXMudHM=) | `100.00% <ø> (ø)` | | | [...ages/bezier-react/src/components/Banner/Banner.tsx](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io#diff-cGFja2FnZXMvYmV6aWVyLXJlYWN0L3NyYy9jb21wb25lbnRzL0Jhbm5lci9CYW5uZXIudHN4) | `82.35% <50.00%> (-10.24%)` | :arrow_down: | | [...ages/bezier-react/src/components/Button/Button.tsx](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io#diff-cGFja2FnZXMvYmV6aWVyLXJlYWN0L3NyYy9jb21wb25lbnRzL0J1dHRvbi9CdXR0b24udHN4) | `97.36% <80.00%> (-2.64%)` | :arrow_down: | | [...react/src/components/SectionLabel/SectionLabel.tsx](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io#diff-cGFja2FnZXMvYmV6aWVyLXJlYWN0L3NyYy9jb21wb25lbnRzL1NlY3Rpb25MYWJlbC9TZWN0aW9uTGFiZWwudHN4) | `90.74% <83.33%> (-4.92%)` | :arrow_down: | | [...ezier-react/src/components/Banner/Banner.styled.ts](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io#diff-cGFja2FnZXMvYmV6aWVyLXJlYWN0L3NyYy9jb21wb25lbnRzL0Jhbm5lci9CYW5uZXIuc3R5bGVkLnRz) | `100.00% <100.00%> (ø)` | | | [...-react/src/components/Modals/Modal/Modal.styled.ts](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io#diff-cGFja2FnZXMvYmV6aWVyLXJlYWN0L3NyYy9jb21wb25lbnRzL01vZGFscy9Nb2RhbC9Nb2RhbC5zdHlsZWQudHM=) | `100.00% <100.00%> (ø)` | | | [...src/components/SectionLabel/SectionLabel.styled.ts](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io#diff-cGFja2FnZXMvYmV6aWVyLXJlYWN0L3NyYy9jb21wb25lbnRzL1NlY3Rpb25MYWJlbC9TZWN0aW9uTGFiZWwuc3R5bGVkLnRz) | `95.45% <100.00%> (+0.45%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/channel-io/bezier-react/pull/1562/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=channel-io)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sungik-choi commented 9 months ago

또한 Button 다음으로 Banner 에서 use case 가 많은데 @channel.io/bezier-codemod 에서는 Button 만 지원하고 있습니다. 그래서 살짝 번거롭긴 하지만 icon-name-to-bezier-icons.ts 에서 meta 를 Banner 로 변경, 빌드 한 후에 데스크에서 아래 커맨드로 패키지를 로컬로 실행하면 마이그레이션이 가능합니다.

Button, Banner 모두 지원하도록 마이그레이션 스크립트도 변경되면 좋을 거 같습니다

yangwooseong commented 9 months ago

iconName을 사용할 경우, 런타임에 console.warn 등으로 에러를 띄워주는 것도 좋을 거 같아요. (참고)

console.warn('
  Deprecation: `IconName` as a value for the `leftContent` property of a `Button` has been deprecated. Use the Icon of bezier-icons instead.
')

좋네요! 여기 보니 메이저 버전 바뀔 때마다 가이드 해주고 있는데 저희도 나중에 해보면 좋겠네요!

yangwooseong commented 9 months ago

iconName을 사용할 경우, 런타임에 console.warn 등으로 에러를 띄워주는 것도 좋을 거 같아요. (참고)

console.warn('
  Deprecation: `IconName` as a value for the `leftContent` property of a `Button` has been deprecated. Use the Icon of bezier-icons instead.
')

이거 개발 환경에서만 콘솔을 띄우는 게 좋을 것 같은데 어떻게 생각하시나요? process.env.NODE_ENV 로 검사하는게 맞을지 의문이네요

sungik-choi commented 8 months ago

iconName을 사용할 경우, 런타임에 console.warn 등으로 에러를 띄워주는 것도 좋을 거 같아요. (참고)

console.warn('
  Deprecation: `IconName` as a value for the `leftContent` property of a `Button` has been deprecated. Use the Icon of bezier-icons instead.
')

이거 개발 환경에서만 콘솔을 띄우는 게 좋을 것 같은데 어떻게 생각하시나요? process.env.NODE_ENV 로 검사하는게 맞을지 의문이네요

좋은 거 같아요. process.env.NODE_ENV === 'development 로 검사하면 거의 모든 상황에서 괜찮지 않을까? 싶네요 정확히는 모르겠지만, 대부분 케이스에서 예약어처럼 사용되는 거 같아서요. 이거 빠르게 별도 PR로 작업해서 올려볼게요

sungik-choi commented 8 months ago

1602

sungik-choi commented 8 months ago

1602 적용 커밋 추가 후 머지해주시면 될 거 같습니다! @yangwooseong