-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat(Tab): 유닛 테스트 코드를 작성합니다. #156
Conversation
|
|
@jungwoo3490 사실 저도 translator가 구체적으로 어떤 상황에 필요했는지는 잘 모르겠어요...! 스토리에 |
이거 아마 이 경우 처럼 실제 서버와 통신해야하는 데이터와 UI로 보여줘야 하는 값이 다른 경우에 사용하려는 목적인 것 같아요. |
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.
translator 로 보여지는 값을 다르게 한 것은 작은 변화라 해당 PR에서는 넘어가도 괜찮을 것 같습니다.
테스트에 대한 부분은 문제 없어 보여요!
translator prop을 앞으로 남길지 없앨지 고민이 될 것 같은데, 남기게 된다면 translator의 값의 존재 유무에 따라 값이 잘 렌더링 되는지 확인하는 테스트를 추가하는 것이 좋을 것 같아요
Tab에 표시될 텍스트를 서버에서 받아오는 case가 있을지가 궁금하네요... 보통 클라 쪽에서 사전 정의하고 사용하는 경우가 많다고 생각했어요. |
흠 텍스트를 받아오기 보다는 서버에 데이터를 요청할 때 이런 경우가 있을 것 같아요. 탭을 클릭할 때 마다 api 요청을 통해서 데이터를 받아오도록 설계됐다면, 실제로 보여주어야하는 텍스트는
|
이전 어드민에서 말씀하신대로 실제 UI에 보여지는 값과 서버에 요청을 보내야 하는 값이 달라서 링크와 같은 translator를 여러 개 만들어줬거든요! 그런데 저도 두 분 의견에 동의하는게, 이런 변환은 시스템 레벨에서 제공할 기능은 아니라고 생각해요! 프로덕트에서 Item을 넣을때 사용자 재량으로 변환을 해서 넣는게 시스템의 복잡성을 줄이는 것이라고 생각이 들구요 |
bump는 patch로 올려주세요! @suwonthugger |
저도 이 부분에 동의하는 바입니다 :) |
변경사항
close #155
tab 컴포넌트 unit 테스트를 작성했습니다.
ui 관련된 부분은 크로마틱에서 확인 가능하여 제외했습니다.
tab1
이고, 화면에 보여주고 싶은 text가탭1
이라면translator = { tab1: '탭1'}
와 같은 방식으로 작성하게 끔 코드가 구성되어있습니다.링크
https://sopt-makers.slack.com/lists/T040QGZF77H/F07LCJ586TS?record_id=Rec07LCJ58NKS
시급한 정도
🏃♂️ 보통 : 최대한 빠르게 리뷰 부탁드립니다.
기타 사항