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

[Feature] Switch 컴포넌트 구현 #22

Merged
merged 34 commits into from
May 28, 2024
Merged

Conversation

ghdtjgus76
Copy link
Collaborator

@ghdtjgus76 ghdtjgus76 commented May 22, 2024

🎉 변경 사항

스위치 컴포넌트 구현했습니다.

  • jsdoc props 설명 추가해뒀습니다.
  • as props 사용해서 polymorphic한 컴포넌트로 구성할 수 있도록 했습니다.
  • 웹 접근성 관련 속성들 추가해놨습니다.
  • 키보드 동작 추가했습니다. (토글 버튼에 포커스가 있는 경우, 스페이스 바나 엔터키 누르면 상태 토글되도록 해놨습니다.)

🚩 관련 이슈

🙏 여기는 꼭 봐주세요!

스토리북 문서화하면서 든 생각인데, 공통 템플릿 지정해서 스토리 파일 생성 자동화해보면 좀 더 좋을 거 같습니다.
plop js 사용해서 간단하게 활용할 수 있을 것 같습니다.
아래 레포지토리 참고하시면 될 거 같아요.
https://github.com/ghdtjgus76/warrrui-setting-test/tree/main/packages/codegen

세팅 변경한 부분이 있습니다.

  1. panda css에서 styled 사용하려고 jsxFramework 속성 추가해뒀습니다.
  2. jest 관련 세팅 변경했습니다.
    절대 경로 세팅 못 불러와서 jest.config.ts에 있는 moduleNameMapper 속성 변경했습니다.
    @testing-library/jest-dom을 setup 파일에서 잘 못 불러오는 문제가 있어 이 부분 tsconfig에서 추가로 설정해줬습니다.
    @testing-library/user-event 라이브러리도 설치했습니다.

Copy link
Collaborator

@eugene028 eugene028 left a comment

Choose a reason for hiding this comment

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

저희 styled-system 때문에 PR이 지나치게 커지는데, .gitignore파일에 세팅해두는거 어떻게 생각하시나욤?!
레퍼런스

Copy link
Collaborator

@eugene028 eugene028 left a comment

Choose a reason for hiding this comment

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

와웅~ Polymorphic한 컴포넌트 예제는 처음 보네요 👏
다만 Toggle은 비교적 button 아니면 input이라는 형식이 어느정도 결정된 컴포넌트라고 생각하여 Polymorphic하게 구현하지 않았다면 더욱 간단하게 만들 수 있지 않았을까??? 라는 궁금증도 이어서 다형성을 고려하여 만드신 이유가 궁금해여!

packages/wow-ui/src/components/Toggle/index.tsx Outdated Show resolved Hide resolved
packages/wow-ui/src/components/Toggle/index.tsx Outdated Show resolved Hide resolved
packages/wow-ui/src/components/Toggle/index.tsx Outdated Show resolved Hide resolved
packages/wow-ui/src/components/Toggle/index.tsx Outdated Show resolved Hide resolved
packages/wow-ui/src/components/Toggle/index.tsx Outdated Show resolved Hide resolved
@eugene028
Copy link
Collaborator

deploy-chromatic.yml << 요거 작동 안한거 같은데 혹시 중간에 작업하시다가 브랜치 바꾸거나..하신건가여? 🤔

@ghdtjgus76
Copy link
Collaborator Author

deploy-chromatic.yml << 요거 작동 안한거 같은데 혹시 중간에 작업하시다가 브랜치 바꾸거나..하신건가여? 🤔

작동은 했는데, 이전에 있던 PR 코멘트 기능 있는 부분을 없앴습니다
커밋할 때마다 PR이 크로마틱 코멘트로 가득해서,,,
혹시 있는 게 나을까요?

@ghdtjgus76
Copy link
Collaborator Author

저희 styled-system 때문에 PR이 지나치게 커지는데, .gitignore파일에 세팅해두는거 어떻게 생각하시나욤?! 레퍼런스

저도 그게 좋다고 생각합니다.
근데, 처음에 styled-system을 깃헙에 안 올리고 cicd 돌리니까 자꾸 빌드 에러가 나서,,,
styled-system을 찾을 수 없다는 에러가 발생하더라고요
그래서 일단 임시 방편으로 올려놨는데, 빠른 시일? 내에 해결해보겠습니다,,,

@ghdtjgus76
Copy link
Collaborator Author

와웅~ Polymorphic한 컴포넌트 예제는 처음 보네요 👏 다만 Toggle은 비교적 button 아니면 input이라는 형식이 어느정도 결정된 컴포넌트라고 생각하여 Polymorphic하게 구현하지 않았다면 더욱 간단하게 만들 수 있지 않았을까??? 라는 궁금증도 이어서 다형성을 고려하여 만드신 이유가 궁금해여!

유진님이 말씀하신 것처럼 기본적으론 button을, 그런데, input도 선택지가 될 수 있을 거 같다고 생각해서 일단 다형성을 고려해서 작업해뒀습니다!
그런데 하나로만 픽스해도 상관은 없겠다는 생각도 들긴 해서 다른 분들 의견 다 취합해서 많은 쪽으로 결정하는 건 어떨까요?

@ghdtjgus76
Copy link
Collaborator Author

@eugene028
혹시 스토리북 문서화 부분이나 테스트 코드도 다 괜찮나요?
한 번 확인 부탁드립니다!

@ghdtjgus76
Copy link
Collaborator Author

다들 크로마틱 링크는 필요하다고 생각하시는 거 같아서 다시 넣어뒀습니다!

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

@ghdtjgus76
Copy link
Collaborator Author

@eugene028 @SeieunYoo
일단 지금 반영할 수 있는 것들은 다 반영했습니다.
input, button 관련 리뷰는 일단 더 찾아보고 일요일에 다같이 정한 후에 반영할게요!

@ghdtjgus76
Copy link
Collaborator Author

@eugene028
찾았습니다...
지금 문제가 되는 부분이 네이밍 때문이기도 한 거 같은데, 아래 글 읽어보면 switch는 pressed, not pressed 두 가지로만 구분되는 걸 말하는 거 같고, toggle은 pressed, not pressed, partially pressed 등 딱 두 가지가 아닐 수도? 있는 경우를 말하는 거 같아요!
좀 더 찾아봤는데 switch는 대부분 input으로 구현하더라고요.
그래서 toggle이 아니라 switch로 이름을 변경하고 input으로 변경하는게 좀 더 저희 디자인과 기능에 맞는 거 같아요.

https://www.w3.org/WAI/ARIA/apg/patterns/switch/

One difference, however, is that switches can only be used for binary input while checkboxes and toggle buttons allow implementations the option of supporting a third middle state.

Copy link
Contributor

@ghdtjgus76 ghdtjgus76 changed the title [Feature] 토글 컴포넌트 구현 [Feature] 스위치 컴포넌트 구현 May 27, 2024
Copy link
Contributor

Copy link
Contributor

@eugene028
Copy link
Collaborator

@eugene028 찾았습니다... 지금 문제가 되는 부분이 네이밍 때문이기도 한 거 같은데, 아래 글 읽어보면 switch는 pressed, not pressed 두 가지로만 구분되는 걸 말하는 거 같고, toggle은 pressed, not pressed, partially pressed 등 딱 두 가지가 아닐 수도? 있는 경우를 말하는 거 같아요! 좀 더 찾아봤는데 switch는 대부분 input으로 구현하더라고요. 그래서 toggle이 아니라 switch로 이름을 변경하고 input으로 변경하는게 좀 더 저희 디자인과 기능에 맞는 거 같아요.

https://www.w3.org/WAI/ARIA/apg/patterns/switch/

One difference, however, is that switches can only be used for binary input while checkboxes and toggle buttons allow implementations the option of supporting a third middle state.

좋습니다....꼼꼼히 찾아주셔서 감사해욧

Copy link
Contributor

Copy link
Collaborator

@eugene028 eugene028 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!! 👏👏👏👏

Copy link
Collaborator

@SeieunYoo SeieunYoo left a comment

Choose a reason for hiding this comment

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

.gitignore 쪽에 컨플릭트 나는 부분만 해결되면 머지 고고 하시졍🚀

@ghdtjgus76 ghdtjgus76 changed the title [Feature] 스위치 컴포넌트 구현 [Feature] Switch 컴포넌트 구현 May 28, 2024
Copy link
Contributor

@ghdtjgus76 ghdtjgus76 merged commit acb62fd into main May 28, 2024
3 checks passed
@ghdtjgus76 ghdtjgus76 deleted the feature/toggle-component branch May 28, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants