-
Notifications
You must be signed in to change notification settings - Fork 0
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: 공통 컴포넌트 Chips, Tab, Badge, Filters, Tag 추가 #25
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.
PR 마스터 좋습니다 LGTM 👍
src/app/components/Badge/Badge.tsx
Outdated
<> | ||
<div className='rounded-full bg-var-gray-900 px-8 text-12 font-semibold text-white'> | ||
{children} | ||
</div> | ||
</> |
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) 빈 프래그먼트 안 감싸도 충분할 것 같은데 빼시는 건 어떠신가요?
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.
수정하겠습니다!
const stateClasses = { | ||
scheduled: 'bg-var-orange-100 text-var-orange-600', | ||
done: 'bg-var-gray-200 text-var-gray-500', | ||
confirmed: 'bg-white text-var-orange-500 border border-var-orange-100', | ||
pending: 'bg-white text-var-gray-500 border border-var-gray-200', | ||
}; |
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.
이거 볼 때마다 아이디어 좋네요 👏
const Tab = ({ type, isActive, onClick }: TabProps) => { | ||
const styles = isActive | ||
? 'text-var-gray-900 first-of-type:before:right-0 last-of-type:before:left-0' | ||
: 'text-var-gray-400 first-of-type:before:-right-full last-of-type:before:-left-full'; |
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) 진짜궁금
first-of-type:before:right-0 last-of-type:before:left-0
이거 설명 좀 해주세요
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.
Tab에서 클릭 시 active 상태가 바뀜에 따라 밑줄의 움직임 애니메이션을 하기 위함입니다!
before 상태로 border-bottom을 추가해 놓고,
isActive = false 시 첫번째 아이템은 -right-full -left-full 로, 마지막 아이템은 -left-full 로 밑줄이 안 보이게 되어 있습니다.
isActive가 true로 바뀌면 각각 왼쪽/오른쪽에서부터 0으로 밑줄이 이동합니다.
이에 애니메이션 상에서는 두개 tab을 번갈아 클릭 시 밑줄이 두 아이템 사이를 오가는 것처럼 보입니다.
설명을 잘 못하겠어요 😥
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.
어렵긴 하네요 제가 좀 더 뜯어보면서 이해해볼게요 !
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.
여기 쉽지 않네요
src/app/components/Tab/TopTab.tsx
Outdated
@@ -0,0 +1,16 @@ | |||
interface TopTabProps { | |||
isActive?: boolean; | |||
children: React.ReactNode; |
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) ReactNode 만 따로 임포트해서 쓰시는 거 어떠신가요?!
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.
수정하겠습니다!
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.
전반적으로 코드가 깔끔한 것 같습니다!
작업량이 많으셨을 텐데 고생 많으셨습니다...!
src/app/components/Badge/Badge.tsx
Outdated
<> | ||
<div className='rounded-full bg-var-gray-900 px-8 text-12 font-semibold text-white'> | ||
{children} | ||
</div> | ||
</> |
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) fragment 가 불필요하다면 삭제해도 될 것 같습니다!
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.
수정하겠습니다!
interface StateChipProps { | ||
state: 'scheduled' | 'done' | 'confirmed' | 'pending'; // 이용 예정 | 이용 완료 | 개설 확정 | 개설 대기 | ||
} | ||
|
||
const stateClasses = { | ||
scheduled: 'bg-var-orange-100 text-var-orange-600', | ||
done: 'bg-var-gray-200 text-var-gray-500', | ||
confirmed: 'bg-white text-var-orange-500 border border-var-orange-100', | ||
pending: 'bg-white text-var-gray-500 border border-var-gray-200', | ||
}; | ||
|
||
const stateContents = { | ||
scheduled: '이용 예정', | ||
done: '이용 완료', | ||
confirmed: '개설 확정', | ||
pending: '개설 대기', | ||
}; |
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.
👍
const styles = isActive | ||
? 'text-var-gray-900 first-of-type:before:right-0 last-of-type:before:left-0' | ||
: 'text-var-gray-400 first-of-type:before:-right-full last-of-type:before:-left-full'; |
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.
👍
src/app/components/Tag/Tag.tsx
Outdated
<> | ||
<div | ||
className={`absolute right-0 top-0 flex items-center gap-4 rounded-bl-[12px] bg-orange-600 py-4 pl-8 text-12 font-medium text-white ${sizeClasses[size]}`} | ||
> | ||
<IconAlarm width='24' height='24' /> | ||
<span className='shrink-0'>{children}</span> | ||
</div> | ||
</> |
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) 여기도 fragment 가 불필요하다면 삭제해도 될 것 같습니다!
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.
수정하겠습니다!
리뷰 주신 부분 반영하였습니다.
fragment 의 경우 습관이었어서, 지적 감사합니다. 고쳐보겠습니다. 😊 |
done: '이용 완료', | ||
confirmed: '개설 확정', | ||
pending: '개설 대기', | ||
}; |
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.
상태를 명확하게 나눠주셔서 사용할 때 편리할 것 같습니다👍👍
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.
이게 원래 사용할 top tap이었군요..! 제가 isActive만 props로 내려주도록 수정하면 사용할 수 있을 것 같습니다!
20: ['2.0rem', '2.8rem'], | ||
24: ['2.4rem', '3.2rem'], | ||
30: ['3rem', '3.6rem'], | ||
}, |
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.
👍
children: string; | ||
} | ||
|
||
const Badge = ({ children }: BadgeProps) => { |
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.
PropWithChilren type을 사용해볼까요?
✏️ 작업 내용
공통 컴포넌트 총 10개 파일 추가하였으며, 내역은 아래와 같습니다.
Badge
Tag
Tab
Filter
Chip
tailwind config 에 fontSize 추가
📷 스크린샷
(동영상) 기본 Chip, Filter 컴포넌트의 경우 모바일 뷰포트일 때 사이즈 변경이 있습니다
(동영상) Tab 컴포넌트의 경우 state 변경에 따른 애니메이션이 있습니다 (아래 사용법 참고)
tailwind.config.ts
추가! TopTab
✍️ 사용법
🎸 기타
ic_check 컴포넌트의 경우, 피그마 상 2가지 color state 를 가지고 있지만
사용처가 Card 컴포넌트에만, 오렌지컬러만 "개설확정" 이라는 라벨과 사용되고 있었습니다.
이에 너무 마이크로하게 컴포넌트를 분리하는 것 같아 추가하지 않았습니다.
의견 주시면 감사하겠습니다!
Filter 컴포넌트가 하나의 컴포넌트로 되어 있었으나, dropdown 형태와 sort 형태가 뷰포트 사이즈에 따른 스타일 변화가 커서 List, Sort 두 개의 컴포넌트로 분리하였습니다.
Top Tab 컴포넌트 빼먹었습니다... 얼른 작업하고 수정하겠습니다.추가 완료.close #17