-
Notifications
You must be signed in to change notification settings - Fork 39
[윤진우] Sprint5 #175
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 #175
The head ref may contain hidden characters: "React-\uC724\uC9C4\uC6B0-sprint5"
Conversation
+ 절대경로를 상대경로로 수정
- 상품 설명을 이름으로 변경 - size 클래스 부모 요소로 변경
- 기존 source 태그 -> img 태그, display: none
- 이미지 파일 - 추가 질문
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번째 미션 작업 고생하셨습니다~
-
Header 컴포넌트에서 해당하는 페이지 글자에 파란색을 입히는 기능을 구현하기 위해 Header 컴포넌트에서 props로 location을 받아, 해당 값에 따라 active 클래스를 추가하도록 구현했습니다. 근데, 이후에 보니까 NavLink 라는 컴포넌트가 있더라구요. 제가 구현한 기능이랑 비슷한것 같은데, 제가 구현한걸 그대로 써도 될까요? 아님, 이걸 사용하는게 더 나을까요?:
NavLink는 내부적으로 현재 경로와 링크가 일치하는지를 판단해서, 자동으로 active 클래스를 넣어주는 기능이 내장돼 있어서 별도로 location을 내려받거나 비교할 필요가 없습니다. 따라서 NavLink를 사용하시는 것이 더 적절합니다. -
모바일용 화면에서 전체 상품 부분에 검색칸이 무조건 내려가게 만들기 위해서 어떻게 해야하나 고민을 많이 하다가 결국 title에 min-width를 줘서 내려가도록 했는데 이렇게 해도 괜찮은 부분인가요? 아니면 다른 방법이 존재할까요? 사실, min-width값도 어림잡아 설정한거라 확신이 안서서 질문드립니다!:
CSS는 원하는 결과를 만들 수 있고 너무 복잡하지 않다면 저는 괜찮다고 생각하기에 지금처럼 작업하셔도 괜찮다고 생각합니다! 다른 방법으로 작업하고 싶으시다면 display: grid를 추천드립니다. -
모바일용 화면에서 정렬 버튼을 누르면 최신순, 좋아요순을 선택하는 드롭다운을 보이도록 했습니다. 그런데, 두 가지 옵션 중 하나를 누르면 그 드롭다운이 다시 숨겨지게 하려고, show 클래스를 toggle 하도록 했는데, 작동이 안됩니다. 정렬 버튼을 누를 시에는 잘 숨겨지는데 왜 옵션 클릭시에는 토글이 안되는지 궁금합니다. 이벤트 버블링? 때문일까요??:
올려주신 구조상 div.orderby-mobile 클릭 시 show가 되고, 옵션 클릭시 hide가 되었다가 해당 요소의 부모인 div.orderby-mobile 에서도 이벤트를 감지하여 다시 show가 됩니다. 이런 경우 button 태그에 onclick 이벤트를 주시거나 내부 옵션 onclick 이벤트에 event.stopPropagation()을 추가하셔서 이벤트 전파를 막으시면 됩니다. -
상품 검색 시 keyword로 입력값을 넘겨줘서 api를 호출하도록 했습니다. 그런데, 테스트중 이상한 점을 발견했습니다. 검색어와 관련없는 상품들이 왜 나오는지 모르겠습니다..!:
이는 백엔드 로직이라 답변드리기 어렵습니다~ 프로젝트 QnA에 질문하시는 것을 추천드려요.
-작성하다보니, App.css와 index.css가 빈 파일로 남게 되었는데, 이부분은 딱히 작성할게 없으면 비워두는 경우도 있나요?? 그리고, 공통 스타일을 base.css로 묶어놨는데, 굳이 base.css 말고 App.css에 넣는게 나을까요??:
내용이 없는 경우 삭제하시는 것이 좋습니다~ 공통 스타일인 경우 App.css보다 base.css가 더 적절한 이름일 것 같습니다.
-
netlify로 배포 후 살펴보던 중, /items 에서 새로고침을 하면 Page not found 라는 오류가 나오는데, 왜 그런 걸까요? 메인 페이지를 입력하면 다시 잘 접속됩니다.:
React는 SPA 방식이기 때문에 /itmes에 해당하는 페이지가 없기 때문에 말씀하신 현상이 발생합니다. netfliy 문서를 확인해보세요. -
netlify로 배포 후 살펴보던 중, 이미지가 늦게 로딩되는 몇가지 상품들이 있는데, 왜 그런 것이며 어떻게 대처할 수 있을까요?: 이미지가 서버에서 다운로드 되는 것은 여러가지 요인이 있을 수 있습니다. 프론트에서 가능한 것은 이미지에 placeholder를 추가하거나 로딩 스피너 등을 추가해 유저에게 피드백을 주는 것입니다~
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.
👍 칭찬
notFound 페이지 추가해주신 것 좋습니다~
| @@ -0,0 +1,10 @@ | |||
| import { createRoot } from "react-dom/client"; | |||
| import "./index.css"; | |||
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.css에 유효한 내용이 없고 css들은 App.jsx에서 import 하고 있으니 삭제하시는 것이 적절할 것 같습니다~
| import "./index.css"; |
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.
💊 제안
3번째 미션에서 추가하셨던 메타 태그도 추가하시면 더 좋을 것 같아요!
| @@ -0,0 +1,19 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en"> | |||
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.
❗️ 수정요청
| <html lang="en"> | |
| <html lang="ko"> |
| <> | ||
| <header className="Header"> | ||
| <div className="header-left"> | ||
| <Link to={"/"}> |
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.
💊 제안
처럼 중괄호로 감싸지 않아도 되는 문자열 경로는 직접 작성하는 것이 더 간결하고 일반적인 컨벤션에 부합합니다.| <Link to={"/"}> | |
| <Link to="/"> |
| /> | ||
| <div className="text-section"> | ||
| <p className="item-name">{name}</p> | ||
| <p className="item-price">{price}원</p> |
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.
💊 제안
금액의 경우 (,)를 통해 단위가 구분되도록 아래처럼 작성해주세요!
| <p className="item-price">{price}원</p> | |
| <p className="item-price">{price.toLocalString()}원</p> |
| const getItemSize = () => { | ||
| if (deviceType === "mobile") return "best-one"; | ||
| if (deviceType === "tablet") return "best-two"; | ||
| return "best-four"; | ||
| }; |
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.
💊 제안
화면 크기에 따라 클래스명을 생성해 Item 컴포넌트에 props로 전달하기 보다 div.item-list에 클래스를 주어 반응형 처리를 하시는게 구조상 더 깔끔할 것 같습니다~
| onClick={() => { | ||
| setOrderBy("recent"); | ||
| setCurrentPage(1); | ||
| document | ||
| .querySelector(".orderby-dropdown") | ||
| .classList.toggle("show"); | ||
| }} | ||
| > |
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 핸들러가 길어질 경우, 가독성을 위해 별도 함수로 분리하는 것이 좋습니다.
| const changePage = (page) => { | ||
| if (page < 1 || page > totalPages) return; | ||
| setCurrentPage(page); | ||
| }; |
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.
💊 제안
이런 경우 함수 내부에서 불가능한 경우 return하시기보다 버튼에 disabled를 추가하시는 것이 유저 피드백 측면에서 더 좋습니다~
<button disabled={page < 1 || page > totalPages} onClick={changePage}>
<img src={arrow_left} alt="처음으로" />
</button>| <select | ||
| className="orderby" | ||
| onChange={(e) => { | ||
| setOrderBy(e.target.value); | ||
| setCurrentPage(1); | ||
| }} | ||
| > | ||
| <option value="recent">최신순</option> | ||
| <option value="favorite">좋아요순</option> | ||
| </select> |
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.

요구사항
기본
🐼 중고마켓
🖼️ 중고마켓 반응형
베스트 상품
전체 상품
심화
주요 변경사항
스크린샷
🖥️ 데스크탑 이미지
📲 태블릿 이미지
📱 모바일 이미지
https://jinwoo-sprint5.netlify.app/
멘토에게
사실, min-width값도 어림잡아 설정한거라 확신이 안서서 질문드립니다!
상품 검색 시 keyword로 입력값을 넘겨줘서 api를 호출하도록 했습니다. 그런데, 테스트중 이상한 점을 발견했습니다.
이렇게 해야지 한번만 찍,aaa,에스파 윈터가 나오고판다마켓...,Ferris the crab이 나옵니다.검색어와 관련없는 상품들이 왜 나오는지 모르겠습니다..!
작성하다보니, App.css와 index.css가 빈 파일로 남게 되었는데, 이부분은 딱히 작성할게 없으면 비워두는 경우도 있나요??
그리고, 공통 스타일을 base.css로 묶어놨는데, 굳이 base.css 말고 App.css에 넣는게 나을까요??
netlify로 배포 후 살펴보던 중, /items 에서 새로고침을 하면 Page not found 라는 오류가 나오는데, 왜 그런 걸까요? 메인 페이지를 입력하면 다시 잘 접속됩니다.

netlify로 배포 후 살펴보던 중, 이미지가 늦게 로딩되는 몇가지 상품들이 있는데, 왜 그런 것이며 어떻게 대처할 수 있을까요?