Conversation
This file contains 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
GANGYIKIM
approved these changes
Sep 9, 2024
There was a problem hiding this comment.
수영님 첫 react 스프린트 미션 고생하셨습니다!
JS로만 작업하시다가 react로 작업하시기 쉽지 않으셨을텐데
요구사항도 구현하시고, 일정도 지키시느라 고생하셨습니다.
스프린트 미션은 가능한 부분까지 학습하며 구현하시는 것이니 편하게 제출해주세요~
- 중복되는 사항에 대해서는 한번만 리뷰를 답니다. 원하실 때 비슷한 사항들을 찾아서 같이 바꿔보세요~
반응형은 구현을 시도했지만 완수하지 못했습니다(이부분은 구현이 어려워서라기보다는 시간을 많이 투자하지 못해 그런듯 합니다!): 반응형은 기존에 작업하시던 방식과 동일한 방식으로 하시면 되니, 시간이 잇으시면 능히 하실거라 저도 생각합니다.드롭다운을 select를 쓰지 않고 디자인과 똑같이 만드는게 꽤 까다롭게 느껴집니다 일단 구글링을 통해서 대충 구실은 하도록 만들었는데 추천해주시는 방법이 있을까요??: 실무에서 디자인을 구현하기 위해, input이나 select같은 요소들은 커스텀해서 사용하곤합니다. 수영님께서 작업해주신 방식과 비슷하게 작업을 하고 큰 차이점이라고 하면 dropdown에서 menu에 해당하는 ul이 보일때 다른 것들보다 위에서 나와야 하기 때문에 portal로 여는 것 정도일 것 같습니다.
There was a problem hiding this comment.
P3:
필요하다고 생각하는 것만 두시고, 글로벌 css도 추가하시고 좋네요 👍
Comment on lines
+41
to
+50
| useEffect(() => { | ||
| const handleResize = () => { | ||
| setPageSize(getPageSize()); | ||
| }; | ||
| window.addEventListener("resize",handleResize); | ||
| fetchSortedData(); | ||
| return () => { | ||
| window.removeEventListener("resize", handleResize); | ||
| }; | ||
| },[fetchSortedData]) |
There was a problem hiding this comment.
P3:
연관있는 것들끼리 묶여 있는 것이 가독성에 더 좋습니다.
(ex. 함수끼리, 변수끼리)
현 파일에서는 해당 useEffect 부분만 toggleDropdown 하단으로 옮겨주시면 더 좋을 것 같습니다.
| <img className="panda-logo" src={PandaLogo} alt="판다마켓로고" /> | ||
| </Link> | ||
| <div className="nav-container"> | ||
| <NavLink to="/community" activeClassName="active">자유게시판</NavLink> |
There was a problem hiding this comment.
P2:
해당 아이템이 active일 때 특정 classname을 주시려면 아래처럼 하시면 됩니다!
관련 문서도 확인해보시면 좋을 것 같아요.
https://reactrouter.com/en/main/components/nav-link
Suggested change
| <NavLink to="/community" activeClassName="active">자유게시판</NavLink> | |
| <NavLink to="/community" className={({ isActive }) => (isActive ? "active" : "")}>자유게시판</NavLink> |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
요구사항
기본
베스트 상품
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기
전체 상품
Desktop : 12개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기
심화
멘토에게