-
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
[Style/#20] 즐겨찾기 컴포넌트 구현 #21
base: develop
Are you sure you want to change the base?
Conversation
…N-WEB-MCDONALD into style/#20/LikeButton
…N-WEB-MCDONALD into style/#20/LikeButton
…N-WEB-MCDONALD into style/#20/LikeButton
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.
좋습니다~!
그런데 버거 포스트 컴포넌트에 width: 100%
속성이 반영 안 되어 있는 것 같아서 추가해주시면 좋을 것 같습니다:)
그리고 즐겨찾기 버튼 눌렀을 때 효과를 setTimeout으로 구현하셨던데 css에 active 속성 사용하면 버튼을 누르는 순간부터 떼는 시점까지의 스타일을 적용시킬 수 있습니다! 그냥 이런 방식도 있다~ 하고 넘어가주세요. 고생하셨습니다!
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.
굿굿!!
@@ -17,6 +18,7 @@ const BurgerPost = ({ titleEn, titleKo }: BugerPostProps) => ( | |||
<div css={postStyleContainer}> |
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.
여기에 ThemeProvider를 넣어주신 이유가 무엇인가요??
App에 해둬서 지워두셔도 될 것 같습니다!
import { | ||
postStyleContainer, | ||
titleSection, | ||
imageSection, | ||
} from './BugerPost.style'; |
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.
스타일이 많다면 전체를 import해서 접근하는타일이 많다면 전체를 import해서 접근하는 것도 좋을 것 같습니다 ㅎㅎ
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.
컴포넌트가 원 색깔과, 하트 색깔말고는 다 같은데 하나의 컴포넌트로 2개의 컬러를 props로 관리하는것이 코드도 적어지고, 렌더링 측면에서도 좋을 것 같은데 어떻게 생각하시나요?
x={0.5} | ||
fill="#fff" | ||
fillOpacity={0.8} |
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.
이 값이랑 아래에 path에 있는 fill을 props로 관리하면 어떨까요?
background-color: ${theme.colors.gray000}; | ||
`; | ||
export const imageSection = css` |
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.
한줄 띄우고 작성하면 가독성이 더 좋을 것 같아요!
@@ -13,6 +13,7 @@ export const postStyleContainer = (theme: Theme) => css` | |||
background-color: ${theme.colors.gray000}; | |||
`; | |||
export const imageSection = css` | |||
position: relative; | |||
display: flex; | |||
height: 11rem; | |||
justify-content: center; |
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-shrink를 주신 이유가 있을까요?
📌 관련 이슈번호
🎟️ PR 유형
어떤 변경 사항이 있나요?
✅ Key Changes
즐겨찾기 하트 컴포넌트 퍼블리싱 및 기능구현
📢 To Reviewers
📸 스크린샷
20241125_185856.mp4
🔗 참고 자료