[배민지] refactor/sprint5 #192
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ithub-actions [Fix] delete merged branch github action
fix: 활성화된 네브 버튼 구현 + 주석 정리
addiescode-sj
approved these changes
Jun 19, 2025
Collaborator
addiescode-sj
left a comment
addiescode-sj
left a comment
There was a problem hiding this comment.
추가 PR도 수고하셨습니다! 👍
주요 리뷰 포인트
- 의미가 명확한 props만 남기기
- 재사용성을 위해 로직을 커스텀 훅으로 분리하기
- 리턴문 깔끔하게 유지하기
Comment on lines
+13
to
+24
| const MenuItem = styled(Menu.Item)` | ||
| display: block; | ||
| padding: 10px 16px; | ||
| font-size: 16px; | ||
| background-color: white; | ||
| color: #1f2937; | ||
| cursor: pointer; | ||
|
|
||
| &:hover { | ||
| background-color: #f3f4f6; | ||
| } | ||
| `; |
Collaborator
There was a problem hiding this comment.
Headless UI로 기본적인 접근성 및 기능을 붙이고 스타일링만 재정의하시는군요 :) 좋은 접근입니다! :)
Comment on lines
+162
to
+182
| useEffect(() => { | ||
| const updateVisibleCount = () => { | ||
| const width = window.innerWidth; | ||
|
|
||
| if (width >= 1200) { | ||
| // 데스크탑 | ||
| setVisibleCount(10); | ||
| } else if (width >= 768) { | ||
| // 태블릿 | ||
| setVisibleCount(6); | ||
| } else { | ||
| // 모바일 | ||
| setVisibleCount(4); | ||
| } | ||
| }; | ||
|
|
||
| updateVisibleCount(); // 처음 한 번 실행 | ||
| window.addEventListener("resize", updateVisibleCount); | ||
|
|
||
| return () => window.removeEventListener("resize", updateVisibleCount); | ||
| }, []); |
Collaborator
There was a problem hiding this comment.
이런 로직은 반응형 분기가 필요할때마다 재사용되지않을까요? 커스텀 훅으로 분리해봅시다!
Comment on lines
+242
to
+246
| onChange={(e) => { | ||
| setPage(1); | ||
| if (e.target.value === "recent") handleNewestClick(); | ||
| if (e.target.value === "favorite") handleFavoriteClick(); | ||
| }} |
Collaborator
There was a problem hiding this comment.
항상 리턴문은 깔끔하게 작성할수있도록 습관을 들여볼까요? 인라인 함수로 작성하기보단, object mapper를 활용하거나 내부 함수로 작성하는 편이 좋을것같네요.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
체크리스트 [기본]
중고마켓
중고마켓 페이지 주소는 “/items” 입니다.
페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.
상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.
'상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )
전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.
중고마켓 반응형
베스트 상품
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기
전체 상품
Desktop : 12개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기
체크리스트 [심화]
멘토에게
css 분리가 필요한데
styled-components로 작성하는 방식이랑
css파일로 따로 분리하는 방식을 같이 사용해도 괜찮을까요?
스타일은 일정한게 좋을 거같긴한데 css가 너무 짧을때는 styled-components가 가독성이 좋아보여서 고민스럽네요
기존에 주신 코드리뷰는 반영하기 전입니다
리팩토링 해보겠습니다!!