-
Notifications
You must be signed in to change notification settings - Fork 2
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] 디자인 시스템 기반 컴포넌트 이어서 작업 #11
Conversation
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.
고생하셨습니다!!
it('Icon은 정상적으로 렌더링 된다.', () => { | ||
render(<Icon />) | ||
|
||
expect(screen.getByText('Icon')).toBeInTheDocument() |
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.
P3: 이미지의 경우 getByAltText
쿼리를 통해 테스트 할 수 있습니다
render(<List>Text</List>) | ||
|
||
expect(screen.getByText('List')).toBeInTheDocument() |
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.
P2: 실제 children
과 테스트 쿼리가 달라서 테스트가 깨지는 것 같아요!
variant?: 'page' | 'search' | ||
kind?: 'table' | 'synonym' | 'description' | 'title' |
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.
P4: css.ts에서 만든 ListVariants
를 활용할 수도 있을 것 같아요!
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.
List Props으로 ListVariants를 가져오면 와 같이 사용할 수 있는데, 이 경우에는 사용자가 page와 search 두가지 경우 모두를 입력할 수 있게 되어서 구현에 어려움이 있었습니다.
keyof ListVariants으로 key 값을 가져오려고 해도 undefined가 붙어서 그런지 string은 never 형식에 사용할 수 없다는 type 에러가 발생합니다 ㅠㅠ
혹시 제가 놓치고 있는 부분이 있는 것이면 알려주세요!
PR Type
Overview
Additional Comment
Issue
#7
Check List
References