-
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
[#67] 상품목록 페이지 UI , API 연동 #91
Conversation
- api를 component에서 직접 사용하지 않고 store를 거치게 수정
client/src/components/Header/HeaderMain/CategoryMenu/CategoryLayer.tsx
Outdated
Show resolved
Hide resolved
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.
고생 많으셨어요 ㅎㅎ
client/src/components/Header/HeaderMain/CategoryMenu/CategoryLayer.tsx
Outdated
Show resolved
Hide resolved
.filter((key) => query[key]) | ||
.map((key) => `${key}=${query[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.
음 query[key]
가 있는 부분만 사용한다는건 length가 0인 문자열은 사용하지 않는다는 의미이죠?
해당 부분도 length로 비교하는 편인데 짧아서 이점도 있는듯 하네요
다른 분들은 어떠신가요?
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.
QueryObject
의 value로 number도 올 수 있어서 지금 방법도 괜찮다고 생각합니다. 그런데 또 숫자 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.
흠 우선, 현재 상태로 남겨놓고 후에 한번 더 이야기 해보는게 좋겠네요 ..ㅎㅎ 사실 저는 filter만 추가하고 백앤드에서 복붙을 ..
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.
저의 레거시 코드들을 수정하신게 많이 보이는군요.. 😢 고생하셨습니다.
client/src/components/Header/HeaderMain/CategoryMenu/CategoryLayer.tsx
Outdated
Show resolved
Hide resolved
|
||
const AD_TITLE = '선물하기 딱 좋아요!'; | ||
|
||
const descendingDate = (a: MockProductItemType, b: MockProductItemType) => { | ||
return a.uploadDate.localeCompare(b.uploadDate); | ||
const descendingDate = (a: Product, b: Product) => { | ||
return b.createdAt.getTime() - a.createdAt.getTime(); | ||
}; | ||
|
||
const descendingDiscountRate = (a: MockProductItemType, b: MockProductItemType) => { | ||
const descendingDiscountRate = (a: Product, b: Product) => { | ||
return b.discountRate - a.discountRate; | ||
}; | ||
|
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.
이 부분은 게더에서 논의한대로 따로 API가 필요하긴 하겠군요..
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.
따로 api가 추가되면 고쳐야할 부분 같네요 .. ㅎㅎ
return { | ||
products: products.map((product: Product) => new Product(product)), | ||
totalPages, | ||
totalProductCount, | ||
}; | ||
} |
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.
runInAction
사용해서 상태로 관리하는걸로 안하신 이유가 있을까요? 🤔 보통 mobX에서 이런 식으로 store에서 직접 리턴을 하기도 하는지 잘 모르겠네요.
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.
mobx로 product를 상태관리 하지 않아서 이런 식으로 작성해봤습니다.
그런데 api를 직접 가져와서 컴포넌트에서 통신하는 것보다 ProductStore를 만들고 거기서 api를 호출하는게 더 맞다고 생각했습니다.
이 부분은 논의를 한번 해보면 좋겠네요 .. ㅎㅎ
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.
수고하셨습니당 ㅎㅎ
관련이슈
실제 소요시간
5h
체크리스트
base branch
를 확인했나요?설명
참고