-
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 : 북마크 페이지 #16
feat : 북마크 페이지 #16
Conversation
title={'포즈를 보관해 보세요!'} | ||
text={`북마크 버튼으로 포즈를 보관할 수 있어요.\n포즈피드에서 멋진 포즈를 찾아 보관해 보세요.`} | ||
button={'포즈피드 바로가기'} | ||
path={'/feed'} |
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)
중괄호 제거해주세용
import FilterSheet from './components/FilterSheet'; | ||
import PhotoList from './components/PhotoList'; | ||
|
||
export default function Feed() { | ||
return ( | ||
<> | ||
<Filter /> | ||
<div className="pt-72"> |
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)
pt-72보다, Spacing컴포넌트 사용하는 거 어떤가요 ??
} | ||
|
||
export default function EmptyCase(props: EmptyCase) { | ||
const { title, text, button, path } = props; |
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)
컨벤션을 통일해야 할 것 같아요.
PrimaryButton
등 다른 컴포넌트에서는 인자에서 구조분해하여 할당받았는데, 이 방식으로 통일하는 거 어떤가요?
export default function EmptyCase( { title, text, button, path } : EmptyCase) { }
이런 식으로요 !
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 👍
💡 왜 PR을 올렸나요?
💁 무엇이 어떻게 바뀌나요?
📆 작업 예정인 것이 있나요 ?
💬 리뷰어분들께
PR #15 에서 판 브랜치입니다. 해당 pr 코드리뷰 및 머지 먼저 부탁드려요☺️