-
Notifications
You must be signed in to change notification settings - Fork 2
[#44] ✨ 공통 컴포넌트 RadioInput 컴포넌트 구현 #94
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
Conversation
KingNono1030
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 ㅎㅎ
제가 예상한대로 깔끔하게 코드 작성해주셔서 인상 깊었어요
| className={clsx( | ||
| 'flex cursor-pointer items-center', | ||
| disabled && 'cursor-not-allowed opacity-50', | ||
| className | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clsx 같은 경우에는,
const labelClass = clsx(
'flex cursor-pointer items-center',
disabled && 'cursor-not-allowed opacity-50',
className
)
className={labelClass}이런 식으로 return 부는 가능한 깔끔하게 남기면 좋을거 같아요 !
| )} | ||
| > | ||
| <input type='radio' checked={checked} disabled={disabled} {...props} /> | ||
| <span className='custom-radio'></span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금 이 cusom-raio 가 라디오 버튼을 대신해주고 있으니, aria 태그는 이 친구에게
aria-label={'radio button'} 주면 좋을거 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<span role="radio" tabindex="0" aria-checked={checked}></span>추가로 줄 만한 속성이 role, tabindex, aira-checked 정도 있을 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://juhyejin.tistory.com/54
원래 input 의 display 를 none 으로 하면서, 접근성 면에서 손해를 보는 현상이 있는거 같아요. 해당 아티클 참고하면 좋을거 같습니다 !
yellowjang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!
📌 PR 템플릿
🏷️ PR 타입 (PR Type)
📝 요약 (Summary)
🔍 상세 내용 (Describe your changes)
RadioInput구현 (스토리북 코드 작성은 어려움이 있어 생략 -> 추후에 추가 예정)🔗 관련 이슈 또는 링크 (Issue Number or Link)
✅ 체크리스트 (Checklist)
📸 스크린샷 (선택 사항)
📝 기타 사항
빌드 시 이런 경고가 나오는데, 테일윈드 확장 (CSS InteliSense) 프로그램에서 자동으로 정리해주는 것과 맞지 않아서 나오는 것 같습니다.