Feat/234 #259
Feat/234 #259
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| height: 185px; | ||
| height: 140px; |
There was a problem hiding this comment.
이 컴포넌트가 여기서만 쓰이는 게 아닌데 css를 이렇게 수정하면 기존에 사용하고 있던 부분의 스타일도 바뀌잖아요.
그래서 클래스를 하나 만드는 게 더 좋은 방법이라는 생각이 드네요.
src/layouts/Home/Thema/index.tsx
Outdated
| const TITLE = [ | ||
| '전하고픈 마음 가득 담아', | ||
| '센스 넘치는 선물', | ||
| '이런 선물 어때요 ?', | ||
| '남녀노소 모두 좋아하는 선물', | ||
| '반짝반짝 빛나는 마음', | ||
|
|
||
| '부담 없이 마음을 전해요', | ||
| '말없이 응원하고 싶을 때🍀', | ||
| '이런 선물도 좋을 것 같아요.', | ||
| '귀여운 게 최고야', | ||
| '달콤한 진심을 전하는 선물', | ||
|
|
||
| '고마운 사람들에게 전해요🎁', | ||
| '변치 않는 마음을 전해요', | ||
| '지금 안 사면 손해입니다 🚨', | ||
| '전하고픈 마음 가득 담아', | ||
| '활용도 높은 집들이 선물', | ||
|
|
||
| '마음을 전하는 빛나는 선물', | ||
| ]; |
There was a problem hiding this comment.
타이틀 사이사이에 들어간 엔터가 타이틀을 그룹핑하고 구분하는 느낌이 드는데, 실제로는 구분이 없으니 엔터를 빼도 될 것 같다고 생각합니다. 물론 줄바꿈이 있는 쪽이 더 가독성은 좋지만 타이틀 배열 같은 경우는 가독성이 크게 상관 없을 것 같아욥
| const CATEGORY_ID = [ | ||
| 170, 171, 172, 176, 177, 178, 179, 181, 182, 184, 185, 188, 192, 194, 195, | ||
| 196, 202, 203, 204, 205, | ||
| ]; |
There was a problem hiding this comment.
살짝 하드코딩의 느낌이 나지만 어쩔 도리가 없어보이네요 😅
There was a problem hiding this comment.
ㅠㅠ 개인적으로 프론트에서 카테고리 id를 랜덤으로 선택해서 보내는게 애매하다는 생각이 들지만 아예 다른부분의 api를 가져다 쓰고있기 때문에 어쩔수 없네요..
| const getRandomCategoryId = () => { | ||
| const categoryIdx = Math.floor(Math.random() * CATEGORY_ID.length); | ||
| return CATEGORY_ID[categoryIdx]; | ||
| }; | ||
|
|
||
| const getRandomTitle = () => { | ||
| const titleIdx = Math.floor(Math.random() * TITLE.length); | ||
| return TITLE[titleIdx]; | ||
| }; |
There was a problem hiding this comment.
utils/generate에 난수 생성하는 함수가 있는데 참고하셔두 좋을 것 같습니다 (굳이 바꿀 필요까지는 없어보이지만요)
src/services/api/v1/thema.ts
Outdated
| const productApi = axios.create({ | ||
| baseURL: import.meta.env.VITE_SERVER_URL, | ||
| }); |
There was a problem hiding this comment.
각 파일마다 axios 인스턴스를 만들어서 사용하고 있는데요.
각 인스턴스가 다 동일한 역할을 하고 있기 때문에, 하나를 만들어놓고 import해서 가져다 쓰는 게 좋을 것 같습니다.
| @@ -24,7 +24,7 @@ | |||
| } | |||
|
|
|||
| &.medium { | |||
| height: 140px; | |||
| height: 180px; | |||
There was a problem hiding this comment.
기존 값은 185 같은데 180으로 수정하신 이유가 있나요??
There was a problem hiding this comment.
헉 오로지 제 기억을 살려서 수정한거라 180인줄 알았어요 기존에 185였군요...
#️⃣연관된 이슈
close: #234 #233
📝작업 내용
🙏리뷰 요구사항(선택)
api가 무한스크롤을 구현하기에 적합하지 않다는 생각이 들었습니다.. 애초에 테마 아이템 데이터를 주는 api도 아니기 때문에 무한 스크롤 기능을 제외하고 7개가량의 컴포넌트만 사용자에게 보여질 수 있도록 구현했습니다. 추후 백엔드분들께 말씀을 드리고 새로운 api를 받아 리팩토링 하는 것이 좋을것 같아요