-
Notifications
You must be signed in to change notification settings - Fork 39
[문주영] Sprint5 #182
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
[문주영] Sprint5 #182
The head ref may contain hidden characters: "React-\uBB38\uC8FC\uC601-Sprint5"
Conversation
베스트 상품에서 화면 사이즈가 변할 때 request를 다시 보내는 것을 방지
GANGYIKIM
left a comment
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.
주영님 5번째 미션 제출 고생하셨습니다!
기존에 하신 주차들도 react로 옮겨주시느라 더 오래걸리셨을 것 같아요.
추후 시간이 되시면 react 방식으로 더 수정해주시면 학습에 도움이 되실거에요~
다음 미션도 화이팅입니다!
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.
💬 여담
React Router를 사용하고 계신데 향후 라우트가 많아지면 Route들을 별도의 파일로 분리하면 App 컴포넌트가 더 깔끔해지고, 라우트 관리도 쉬워집니다.
| import './Index.css'; | ||
|
|
||
| function Index() { | ||
| document.title = "판다마켓"; |
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.
❗️ 수정요청
index.html에서 title 태그로 판다마켓을 설정해주셨기 때문에 필요없는 코드입니다~
| document.title = "판다마켓"; |
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.
다른 페이지에서 다시 index 페이지로 돌아올 경우 title이 판다마켓으로 돌아오지 않아서 필요했던 코드였습니다!
| import Pagenation from '../components/Pagenation'; | ||
|
|
||
| function Items() { | ||
| document.title = "중고마켓"; |
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.
❗️ 수정요청
사용하시는 라우트 라이브러리에서 메타태그를 정의하는 방법을 제공하니 아래의 문서를 읽고 변경하시는 것을 추천드립니다~
| <button | ||
| onClick={() => setPage( Math.ceil(page/5) === 1 ? 1 : Math.ceil(page/5 - 1)*5 )} | ||
| > |
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.
💊 제안
캐러셀과 페이지네이션과 같은 컴포넌트의 경우 prevButton, nextButton 이 특정 조건에서는 disabled 되는 것이 일반적입니다.
이를 추가해보시면 더 좋을 것 같아요.
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.
💊 제안
button type 이 reset이 아니라면 명시해주세요~
| <button | |
| onClick={() => setPage( Math.ceil(page/5) === 1 ? 1 : Math.ceil(page/5 - 1)*5 )} | |
| > | |
| <button | |
| type="button" | |
| onClick={() => setPage( Math.ceil(page/5) === 1 ? 1 : Math.ceil(page/5 - 1)*5 )} | |
| > |
| : <img src={icSort} /> } | ||
| { isOpen && | ||
| <ul className={styles.dropdownList}> | ||
| <li onClick={() => setState("recent")}>{option.recent}</li> |
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.
❗️ 수정요청
이런 클릭 가능한 요소의 경우 버튼을 사용해주세요. 더 적절한 태그가 있는데 이미지 태그에 onClick 을 주게되면 button에게 기대하는 동작을 제대로 수행하지 못합니다!
| <li onClick={() => setState("recent")}>{option.recent}</li> | |
| <li> | |
| <button type="button" value="recent" onClick={(e) => setState(e.target.value)}>{option.recent}</button> | |
| </li> |
위에처럼 button 태그로 바꾸시고 onClick 시 event 객체에서 button 태그의 value를 받아오는 방식으로 구현하시는 것을 추천드려요! 이렇게 하시면 option 객체를 기준으로 순회하셔서 작성하실수도 있습니다~
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.
nested button 문제로 인해 <input type="button" />으로 구현하겠습니다!
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.
스프린트 미션5 요구사항에 검색 기능 구현은 없었던 것 같은데 혹시 제가 놓쳤나요?
요구사항
기본
중고마켓
중고마켓 반응형
심화
주요 변경사항
스크린샷
멘토에게