[Feature] Chip 컴포넌트 테스트 작성 #88
Conversation
eugene028
requested review from
ghdtjgus76,
hamo-o,
SeieunYoo and
kongnayeon
as code owners
크악 원인이 그거였군용 |
호오오 자세히 알려주셔서 감사해요! 스토리북 에러 터지는 곳 문서화도 고쳐두었어용 |
오홍 알겟습니다 확인해보니까 Chip은 jest 테스트 PASS여서 a11y 부분 고쳐두겠습니당 |
|
리뷰 남겨주신 여러분께 감사의 말씀을 전하며 리뷰 요청 살포시 드립니다 ... 🥹 @ghdtjgus76 @SeieunYoo @hamo-o |
| disabled = true, | ||
| }: { | ||
| label: string; | ||
| isChecked: boolean; | ||
| ischecked: boolean; |
There was a problem hiding this comment.
ask;
ischecked 라는 네이밍으로 바꾼 이유가 있으실까요?! 카멜케이스로 유지하는 게 좋을 것 같아서요!
There was a problem hiding this comment.
return (
<Component
ischecked
aria-checked={clickable ? ischecked : undefined}
aria-disabled={disabled}
aria-label={`chip ${ischecked ? "activated" : "inactivated"}`}
data-selected={ischecked}
...aria 표준 지키려면 role이 checkbox일때 aria-checked를 쓰기보다는 ischecked라는 값을 컴포넌트에 그대로 전달해주어야 하더라구요~ 🥹 그동안 진행햇던 test에서 warning이 떠서 고냥 고쳐두었어용
There was a problem hiding this comment.
p5;
사소하지만 다른 스토리북 컴포넌트와의 확장자 tsx로 맞추면 좋을 것 같아요!
| disabled = true, | ||
| }: { | ||
| label: string; | ||
| isChecked: boolean; | ||
| ischecked: boolean; |
There was a problem hiding this comment.
ask;
ischecked 라는 네이밍으로 바꾼 이유가 있으실까요?! 카멜케이스로 유지하는 게 좋을 것 같아서요!
🎉 변경 사항
🙏 여기는 꼭 봐주세요!
@ghdtjgus76
Switch 컴포넌트와 Chip 컴포넌트가 유사한게 굉장히 많아서 코드를 많이 참고했는데요,
확인해보니 testing library의 fireEvent에 대한 사용을 많이 하셨더라구요,
어떤 아티클을 읽어보니까 ,
fireEvent에서userEvent로 마이그레이션을 하시길래 그 이유를 찾아보았습니다.fireEvent.click은 단순히 클릭 이벤트만 일으키지만,userEvent.click은 어떤element에서 발생한 이벤트인지 체크하여, 클릭이벤트가 호출될 때 브라우저에서 발생하는 모든 이벤트들을 함께 발생시켜준다.(DOM Event 형식으로 발생)