-
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 : 필터 바텀 시트 #15
feat : 필터 바텀 시트 #15
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.
LGTM 👍
코멘트 부분 확인해주세용
{data.map((item, idx) => ( | ||
<div | ||
key={idx} |
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.
key를 index로 사용하며 재배열이 일어날 경우 에러가 발생할 수 있습니다.
key값은 고유한 값으로 설정하면 좋을 것 같습니다.
링크 참고 부탁해용 !
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.
아 그 부분을 생각해보긴 했는데,, 데이터값이 고유하다는 보장이 없고, selectionTag는 순서가 변경될 일이 없어서 고민끝에 idx를 사용했습니다..! 혹시 key값으로 사용할 값이 어떤게 있을까요 ㅠㅠ?
</div> | ||
<SelectionTag data={tagList} state={tagState} setState={setTagState} /> | ||
</section> | ||
<div className="flex gap-8 py-20 [&>*]:flex-1"> |
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.
[&>*]:flex-1
이 기호는 뭔가용 ??
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.
해당 div의 자식들에게 flex-1 을 주는 코드입니다!
primary button 을 flex로 관리하는데 페이지별로 버튼의 크기가 달라져서 부모 div에서 관리해주고 있어요 !
onClick={onClick} | ||
className={`flex items-center justify-center gap-8 rounded-12 px-24 py-14 ${style[type]}`} | ||
> | ||
{icon && <Image src={icon} alt={''} width={24} height={24} />} |
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)
alt
값 넣어주세용
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.
해당부분은 아이콘이 어떤 아이콘이 나올지 몰라서 비워뒀습니다..!
아이콘의 경우 오류로 표출되지 않아도 유저가 사용하는데 큰 문제는 없는데, 오히려 그 자리에 '아이콘' 같은 글씨가 있으면 오류라는게 명확히 보일 것 같아서요!
feat : 북마크 페이지
feat : 포즈피드 기능 완성 및 포즈픽 포즈톡 디테일
💡 왜 PR을 올렸나요?
💁 무엇이 어떻게 바뀌나요?
📆 작업 예정인 것이 있나요 ?
💬 리뷰어분들께
설정을 바꾼다고 바꿨는데,, 이모지 없이 커밋을 해도 자꾸 이모지가 자동으로 포함되어 커밋되네요..ㅠㅠ
본 PR 후 컴퓨터를 종료했다가 다시 시도해보도록 하겠습니다 죄송합니다 8ㅁ8
포즈피드의 배경을 검은색으로 둔건 레이아웃 영역을 확인하기 위해 일부러 뒀습니다! 포즈피드 API 연결 작업 하면서 수정 예정입니다 :)