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] RadioGroup 컴포넌트 구현 #39

Merged
merged 58 commits into from
Jun 11, 2024
Merged

Conversation

hamo-o
Copy link
Member

@hamo-o hamo-o commented Jun 4, 2024

🎉 변경 사항

RadioGroup 컴포넌트 구현했습니다.

🚩 관련 이슈

🙏 여기는 꼭 봐주세요!

고민중인 지점은 RadioButton value의 중복값을 어떻게 체크할 것인가? 입니다. 아래 방법 중 어떤게 적절할지 의견 남겨주세요! 다른 방법들도 환영입니당,,

  1. value 뒤에 useId를 활용하여 고유한 값을 붙여준다. → 문제는 생기지 않겠지만, 사용자가 동일한 label을 마음대로 두 개 이상 적는 부적절한 활용을 막을 수 없음.
  2. RadioButton 코드 내부에 중복값 체크 로직을 추가한다. → 개발단에서 체킹하는 부분인데, RadioButton 내부에 적는 것은 부적절하다고 생각.
  3. eslint 규칙 추가 → eslint config 배포 후 사용자가 요걸 사용하게끔 해야 함.

@hamo-o hamo-o self-assigned this Jun 4, 2024
Copy link
Contributor

github-actions bot commented Jun 4, 2024

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.

수고 많으셨어용~~

RadioButton value의 중복값을 어떻게 체크할 것인가?

  • 컴포넌트 쪽에서 prop 으로 들어온 value 값 뒤에 고유한 id 를 붙여주게 되면 value 에 따라 select 상태가 제어 (같은 value 일 때 select) 되어야 하는데 사용자는 고유 id 를 모르기 때문에 value 를 저희가 수정하면 제대로 select 가 되지 않는 상황이 발생할 것 같아요.!
  • 저는 사용자가 인지하고 value 를 다르게 할 수 있도록 스토리북을 통해 충분히 안내하거나, 개발단에서 중복 체킹을 해서 중복이라면 에러를 발생시켜서 사용자가 고치도록 함 이렇게 두 가지 방식이 생각나는데 컴포넌트에서 이 부분까지 고려하는 게 좋을 지는 확신이 서지 않네용,, 🤔

다른 분들은 어떻게 생각하쉬나욥

Copy link
Collaborator

@ghdtjgus76 ghdtjgus76 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
Contributor

github-actions bot commented Jun 4, 2024

Copy link
Contributor

github-actions bot commented Jun 5, 2024

Copy link
Contributor

github-actions bot commented Jun 5, 2024

@hamo-o hamo-o force-pushed the feature/radio-group-component branch from 94bf5e3 to 1456ee3 Compare June 9, 2024 11:40
@hamo-o hamo-o force-pushed the feature/radio-group-component branch from 1456ee3 to fa807d7 Compare June 9, 2024 11:44
Copy link
Contributor

github-actions bot commented Jun 9, 2024

Copy link
Contributor

github-actions bot commented Jun 9, 2024

@hamo-o hamo-o requested review from ghdtjgus76 and SeieunYoo June 9, 2024 13:08
Copy link
Collaborator

@ghdtjgus76 ghdtjgus76 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.

어푸룹!!! 고생 많으셨어요~~

Copy link
Contributor

@hamo-o hamo-o merged commit 954f409 into main Jun 11, 2024
3 checks passed
@hamo-o hamo-o deleted the feature/radio-group-component branch June 11, 2024 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants