Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Enhance CheckableAvatar component to behave as an actual checkbox #1320

Merged
merged 15 commits into from
May 12, 2023

Conversation

sungik-choi
Copy link
Contributor

@sungik-choi sungik-choi commented May 4, 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

Fixes #1237

Summary

CheckableAvatar 컴포넌트가 체크박스로 동작할 수 있도록 개선(재구현)합니다.

Re-implement CheckableAvatar component.

  • CheckableAvatar is a checkbox component that looks like Avatar.
  • It now behaves as a checkbox. Keyboard control is available.
  • Add forwardRef.
const [checked, setChecked] = useState(false)
// Controlled
<CheckableAvatar
  name="John Doe"
  avatarUrl="..."
  checked={checked}
  onCheckedChange={setChecked}
/>
// Uncontrolled
<CheckableAvatar
  name="John Doe"
  avatarUrl="..."
  defaultChecked
/>

Details

  • 호버 트랜지션 스타일이 제거됩니다.
    • 내부적으로 AlphaSmoothCornersBox 컴포넌트를 사용하여 스타일링을 하도록 방식이 변경되었습니다.
    • 이 방식을 적용할 경우, 기존의 after 수도 엘리먼트에 smooth corners + opacity를 적용하여 트랜지션 효과를 적용하는 게 어렵습니다.
    • 별도의 div를 만들어 동일한 방식으로 트랜지션 스타일을 구현할 수 있습니다.
    • 하지만 채널 데스크 앱에서 굉장히 큰 테이블에 사용되는 CheckableAvatar 컴포넌트의 특성상, 내부에 트랜지션 스타일만을 위해 smooth corners 가 적용된 div를 추가로 두는 건 득보다 실이 크다고 판단했습니다.
    • 더해서, 이전에 구현했을 당시에도 정확한 디자인 스펙이 아니었기에 현재 트랜지션이 없는 Checkbox, Radio 와 우선 통일하기로 결정했습니다. 공식적으로 디자인 시스템 스펙에 트랜지션이 추가된다면 추후 추가할 예정입니다.
  • 인터페이스를 Checkbox 와 같은 방식으로 통일합니다. 컴포넌트의 사용 방식을 일관적으로 만들어 학습을 쉽게 하도록 하기 위한 변경입니다.
  • 몇 가지 사용하지 않는 속성을 제거했습니다.
    • isCheckable : 채널 데스크 앱에서 사용하지 않고, disabled 속성으로 대체가능합니다.
    • checkedBackgroundColor : 사용하지 않고, 해당 속성으로 오버라이드하기보다 추후 --*-checked 같은 디자인 토큰을 오버라이드하는 방식을 통해 스타일링하는 게 바람직하다고 판담ㄴ했습니다.
    • checkableWrapper* : 사용하지 않고, 불필요한 스타일 속성이라고 생각했습니다. 이 속성들까지 사용하여 스타일링하고 있다면, 컴포넌트를 잘못 사용하고 있을 확률이 매우 높습니다. 옵션을 제공하지 않는 편이 일관적인 사용을 강제할 수 있다고 판단했습니다.

Breaking change or not (Yes/No)

Yes

  • Change name of isChecked property to checked property.
  • Remove isCheckable property.
  • Remove checkedBackgroundColor property.
  • Remove checkableWrapperClassName property.
  • Remove checkableWrapperInterpolation property.

References

@sungik-choi sungik-choi added enhancement Issues or PR related to making existing features better a11y Issue or PR related to accessibility labels May 4, 2023
@sungik-choi sungik-choi self-assigned this May 4, 2023
@changeset-bot
Copy link

changeset-bot bot commented May 4, 2023

🦋 Changeset detected

Latest commit: ce0ce85

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 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
Copy link
Contributor

github-actions bot commented May 4, 2023

Chromatic Report

🚀 Congratulations! Your build was successful!

@sungik-choi sungik-choi force-pushed the enhance/checkable-avatar branch 3 times, most recently from 3ab97bd to a00991b Compare May 4, 2023 08:00
@sungik-choi sungik-choi force-pushed the enhance/checkable-avatar branch from a00991b to f3ba717 Compare May 4, 2023 09:44
@sungik-choi sungik-choi marked this pull request as ready for review May 4, 2023 09:44
@sungik-choi sungik-choi changed the title [WIP] Enhance CheckableAvatar component to behave as an actual checkbox Enhance CheckableAvatar component to behave as an actual checkbox May 8, 2023
@sungik-choi
Copy link
Contributor Author

TODO

  • AlphaSmoothCornersBox 안정화 이후 smoothCorners 믹스인 제거
  • SmoothCornersFeatureactivated 멤버 변수 private화

@sungik-choi
Copy link
Contributor Author

TypeError: symbol is not a function 에러 디버깅 필요합니다.

@sungik-choi sungik-choi requested review from Dogdriip and annie1229 May 8, 2023 04:51
@sungik-choi sungik-choi marked this pull request as draft May 8, 2023 04:52
@sungik-choi sungik-choi marked this pull request as ready for review May 8, 2023 04:52
@@ -12,8 +12,7 @@ module.exports = {
'src',
],
moduleNameMapper: {
'\\.(jpg|ico|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
'identity-obj-proxy',
'\\.(jpg|ico|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$': '<rootDir>/__mocks__/fileMock.js',
Copy link
Contributor Author

@sungik-choi sungik-choi May 8, 2023

Choose a reason for hiding this comment

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

정확한 이유는 모르지만, 테스트 환경에서 발생한 Symbol is not a function 에러가 다음 코드로 해결되었습니다.
cssUrl() 구문 해석 도중에 발생했는데, 왜 처음 cssUrl() 이 추가된 #1317 에선 테스트가 정상적으로 실행되었는지 모르겠습니다. 🤔

@codecov
Copy link

codecov bot commented May 8, 2023

Codecov Report

Patch coverage: 94.44% and project coverage change: +0.94 🎉

Comparison is base (b2629ec) 77.76% compared to head (ce0ce85) 78.71%.

Additional details and impacted files
@@             Coverage Diff             @@
##           next-v1    #1320      +/-   ##
===========================================
+ Coverage    77.76%   78.71%   +0.94%     
===========================================
  Files          302      302              
  Lines         3855     3838      -17     
  Branches       849      841       -8     
===========================================
+ Hits          2998     3021      +23     
+ Misses         577      538      -39     
+ Partials       280      279       -1     
Impacted Files Coverage Δ
...haSmoothCornersBox/AlphaSmoothCornersBox.styled.ts 100.00% <ø> (ø)
...eact/src/components/Avatars/Avatar/Avatar.types.ts 100.00% <ø> (ø)
...ct/src/components/Avatars/CheckableAvatar/index.ts 0.00% <0.00%> (ø)
...ier-react/src/components/Avatars/Avatar/Avatar.tsx 94.87% <100.00%> (+7.69%) ⬆️
.../Avatars/CheckableAvatar/CheckableAvatar.styled.ts 100.00% <100.00%> (+100.00%) ⬆️
...onents/Avatars/CheckableAvatar/CheckableAvatar.tsx 100.00% <100.00%> (+100.00%) ⬆️

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

@sungik-choi sungik-choi merged commit 5b6c2d5 into channel-io:next-v1 May 12, 2023
@sungik-choi sungik-choi deleted the enhance/checkable-avatar branch May 12, 2023 06:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
a11y Issue or PR related to accessibility enhancement Issues or PR related to making existing features better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance CheckableAvatar component to behave as an actual Checkbox
1 participant