-
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: 상품 썸네일 및 판매시작 api 연동 #124
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.
콤엔트가 있어용
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.
코멘트들이 있습니다!
59196db
to
a9bd76a
Compare
어디서부터 봐야하는건가욤,,? |
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-1(2-3) 배포합시다 |
a9bd76a 이 커밋만 확인해주세욥 ! |
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/pages/CreateProduct/components/Footer/hooks/useSaveProduct.ts
Outdated
Show resolved
Hide resolved
9a80b3b
to
c268cc3
Compare
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.
saso한 코멘트가 있어욤 반영감삼다! 고생하셨어요
@@ -4,7 +4,7 @@ import styled from 'styled-components'; | |||
import { theme } from 'themes'; | |||
|
|||
export const ProductCard = styled(Link)` | |||
color: #000; | |||
color: ${theme.palette.common.black}; |
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.
Q: color: ${palette.text.secondary}; 이런거 안써주시고 새로 정의하신 이유가 궁금해욤
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.
A: 정의한게 아니라 palette
정의된거 보니까 기본으로 common
이라는 객체가 있고 그 안에 white, black 있습니당!
palette.ts
에 common 정의되어있는거 제가 한 것 같은데 ,, 왜 추가했는지 ,, 기억도 나지 않고 제거해도 동일하게 작동하네요 (33789a3 )
${palette.text.secondary}는 회색인 것 같슴다
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.
${palette.text.primary}도 있어용
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.
eb76c79 수정했어용
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.
반영 감사합니드아앙!!
PR 제안 사유
/product
api를 연동합니다.Resolves #114 #115
주요 변경 기록
Code review
Code review 에서 중점적으로 봐야하는 부분
Design review
Design review 에서 중점적으로 봐야하는 부분 / 스크린샷
기타 질문 및 특이 사항