[최지은] sprint5#93
Hidden character warning
Conversation
[Fix] delete merged branch github action
…-sprint1 [최지은]sprint1
- form내의 id/for 설정과 submit을 버튼으로 수정
There was a problem hiding this comment.
안녕하세요, 지은님!
리액트로 넘어오면서 어려운 점들이 많으셨을텐데 고생 많으셨어요.
전반적으로 React의 기본기를 잘 다지고 계신 것 같아요. 다만, 이전 미션에서 사용한 파일과 샘플 폴더가 포함되어 있어요. 이런 부분은 따로 보관을 하고, 해당 브랜치에는 관련된 파일들만 관리 부탁드려요!
그리고 헤딩 태그의 계층 구조가 명확하지 않아요. 어떤 페이지는 h3 태그만 있고, h4만 있는 곳도 있어요. 각 헤딩 태그는 스타일을 위해 사용하기 보다는 목적에 맞게 사용해주세요. 😊
말씀하신 좋아요 기능은 현재 미션의 요구사항은 아니지만, 구현을 해보고 싶으시다면 로그인(인증) 기능부터 구현을 해야해요. 401 상태 코드는 "Unauthorized(권한 없음)"을 의미하죠. 인증 후에 응답으로 받은 accessToken을 좋아요 요청 헤더에 포함시키면 됩니다.
참고로, 403(Forbidden)은 인증은 되었지만 접근 권한이 부족한 경우를 뜻합니다. 🤓
Swagger를 통해 테스트 하는 방법을 알려드릴게요.
- Auth 섹션에서 /auth/signIn 클릭
- Try it out을 클릭
3. 그대로 Execute를 클릭하면 응답 바디에 있는 accessToken 복사
4. Authorize 버튼 클릭 후 Value 필드에 복사한 accessToken 붙여넣기
이후 Swagger에서 /products/{productId}/favorite 요청을 해보시면, 정상 동작하는 것을 확인하실 수 있어요. 👍
좋습니다! 이제 자세한 피드백은 아래에서 확인해 주세요. 하나씩 적용해 보시고, 궁금한 게 생기면 편하게 말씀해 주세요. 😀
There was a problem hiding this comment.
왜 node_modules 폴더는 원격 저장소에 올리지 않는 걸까요? 저는 용량 문제와 불필요한 변경 추적이 가장 큰 이유라고 생각해요. node_modules는 패키지 매니저(npm, yarn, pnpm 등)가 package.json과 package-lock.json(yarn.lock)을 기반으로 언제든 내 컴퓨터에서 재생성할 수 있는 폴더예요.
기술 과제를 볼 때도 node_modules 폴더는 꼭 제외한 후 제출하라고 주의를 주기도 하죠. 이 점 유의해 주세요! 🙂
There was a problem hiding this comment.
피그마에서 이미지를 추출할 때는 원본 크기의 2~3배 크기로 다운 받는 게 좋아요. PC 화면일 때 이미지의 고유 크기보다 더 크게 렌더링 되고 있어서 선명하지 않는 느낌을 받아, 사용자에게 전문적이지 않은 인상을 줄 수 있어요.
이미지가 너무 커서 디바이스 크기별 이미지 최적화를 하고 싶다면, img 태그의 srcset, sizes 속성을 한번 찾아보세요. 👍
There was a problem hiding this comment.
아이콘 이미지들은 svg 파일로 다운받는 건 어떨까요? svg 파일은 이미지의 크기에 상관없이 항상 선명하다는 장점이 있어요. 다만, 모든 곳에 사용하기에는 용량이 비교적 높은 편이라 이런 간단한 아이콘에 사용하기 좋아요.
There was a problem hiding this comment.
"commom"으로 오타가 있네요! 🙂
| @@ -0,0 +1,21 @@ | |||
| import { Link } from "react-router-dom"; | |||
|
|
|||
| import IntroImage from "@/assets/img_home_top.png"; | |||
There was a problem hiding this comment.
파일명은 Img_home_top.png인데 img_home_top.png으로 import 해오고 있어요. 환경에 따라 이미지가 깨질 수 있어서 대소문자 구분을 명확하게 해야해요.
| useEffect(() => { | ||
| fetchProducts(); | ||
| fetchBestProducts(); | ||
| }, []); | ||
|
|
||
| // 페이지이동, 검색, 정렬변경, 화면 크기 변경했을 때 렌더링 | ||
| useEffect(() => { | ||
| fetchProducts(); | ||
| fetchBestProducts(); | ||
| }, [page, searchInput, orderBy, device]); |
There was a problem hiding this comment.
useEffect에 의존성을 추가하더라도, 첫 렌더링 시 실행을 합니다. 따라서 현재 코드는 중복 코드예요. 2번 요청을 4번 요청하게 되는거죠. 🤓
| useEffect(() => { | |
| fetchProducts(); | |
| fetchBestProducts(); | |
| }, []); | |
| // 페이지이동, 검색, 정렬변경, 화면 크기 변경했을 때 렌더링 | |
| useEffect(() => { | |
| fetchProducts(); | |
| fetchBestProducts(); | |
| }, [page, searchInput, orderBy, device]); | |
| useEffect(() => { | |
| fetchProducts(); | |
| fetchBestProducts(); | |
| }, [page, searchInput, orderBy, device]); |
| const totalPages = Math.ceil(totalCount / getAllList()); | ||
|
|
||
| const groupStart = Math.floor((page - 1) / PAGE_GROUP) * PAGE_GROUP + 1; | ||
| const groupEnd = Math.min(groupStart + PAGE_GROUP - 1, totalPages); | ||
|
|
||
| const prevGroup = groupStart > 1; | ||
| const nextGroup = groupEnd < totalPages; |
There was a problem hiding this comment.
렌더링 중에 계산 가능한 값은 상태가 아닌 변수로 선언한 점이 좋네요. 👍
| <div className={`listBox ${isOpen ? "open" : ""}`}> | ||
| <div className="listLabel" onClick={() => setIsOpen((prev) => !prev)}> | ||
| <span>{orderBy === "recent" ? "최신순" : "인기순"}</span> | ||
| <img className="arrow_icon" src={arrowIcon} alt="arrowIcon" /> |
There was a problem hiding this comment.
접근성 개선을 해볼까요? 스크린 리더가 해당 이미지를 읽을 때, "화살표 아이콘"이라고 읽기 보다는 "정렬 옵션 토글 아이콘"으로 읽게 만드는 건 어떨까요? 참고로, 스크린 리더는 마지막에 "이미지"를 추가로 말해줍니다. (e.g. arrowIcon 이미지)
There was a problem hiding this comment.
HomeFooter에서 불러오는 snsData를 별도의 컴포넌트로 분리해 내부에서 관리하는 건 어떨까요? sns_wrapper 전용 컴포넌트를 만들면, UI 컴포넌트가 더 명확해지고 SNS 영역을 재사용하거나 유지보수하기도 더 쉬워집니다.
| <Route element={<NoLayout />}> | ||
| <Route path="/login" element={<LoginPage />} /> | ||
| <Route path="/signup" element={<SignupPage />} /> | ||
| </Route> |
There was a problem hiding this comment.
레이아웃이 필요하지 않다면 Route로 감쌀 필요는 없습니다. 불필요한 컴포넌트 계층은 지양하는 것이 좋아요. 🙂
| .nav_group a.active { | ||
| text-decoration: underline; | ||
| text-underline-offset: 4px; | ||
| text-decoration-thickness: 2px; | ||
| } |
There was a problem hiding this comment.
페이지 주소가 "/items" 일때 상단 네비게이션바의 "중고마켓" 버튼의 색상을 요구사항에서 제시한 색상으로 변경해 주세요. 😊
요구사항
기본
베스트 상품
Desktop : 4개 보이기
Tablet : 2개 보이기
Mobile : 1개 보이기
전체 상품
Desktop : 12개 보이기
Tablet : 6개 보이기
Mobile : 4개 보이기
심화
주요 변경사항
스크린샷
멘토에게