-
Notifications
You must be signed in to change notification settings - Fork 38
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
[1단계 - 상품 목록] 초코(강다빈) 미션 제출합니다. #32
Conversation
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
- handlers.ts : API 요청 처리 핸들러 - server.ts : msw 서버 객체 - vitest.config.ts : vitest 설정 파일 - vitest.setup.ts : msw 서버 관리 Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
Co-Authored-By: soosoo22 <[email protected]>
align-items: center; | ||
position: fixed; | ||
top: 0; | ||
z-index: 111; |
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.
111 값의 의미가 있을까요? 이 값도 상수로 관리되면 좋을 것 같아요 :)
|
||
const App = () => { |
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.
MSW를 사용하여 API 요청을 모킹하고 개발할 수 있다.
로컬 개발환경에서 모킹한 API를 활용할 수 있도록
setupWorker
로 worker를 구성하고 조건부로 worker.start
를 실행해주면 어떨까요?
src/App.tsx
Outdated
<h1>React Shopping Products</h1> | ||
<Header /> | ||
<ToastNotification /> | ||
<Product /> |
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.
ProductSection
처럼 그룹 성격을 나타낼 수 있는 이름도 추천드려보아요 section
태그의 활용도 같이 제안드려봅니당 :)
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.
초코🍫 안녕하세요
1단계 작업해주신 것 잘봤습니당 👍
미션과 관련된 상황도 잘 공유해주시고, 궁금한 부분도 상세하게 질문해주셔서 더 몰입해서 즐겁게 리뷰할 수 있었던 것 같습니당 :) 친절하게 공유해주셔서 감사해요 💙 (이미 리액트는 잘 쓰고 계신 것 같습니다 💯)
답변 포함해서 코멘트 남겨두었는데, 천천히 확인해보시고 반영 완료 되시면 재요청 부탁드릴게요!
수고많으셨습니다 👏👏👏👏👏
- <select>에 aria-label 추가 - <img>에 alt="" 추가 - 상품 제목에는 <h2> 추가 - z-dinex 값 상수로 관리 - <section> 사용하는 컴포넌트 ProductSection 으로 이름 수정
- 장바구니 수량이 아니라 장바구니 리스트를 기반으로 - 장바구니에 담겨있다면 빼기 버튼 활성화 - 장바구니에 없다면 담기 버튼 활성화 - 각 버튼에 맞는 api 요청 인데 400 에러 미해결 상태!
하루, 안녕하세요 초코🍫입니다. 🔗 배포 링크주요 변경 사항 바로가기추가 질문 사항
추가 반영하고 싶은 사항
|
src/api/cart.ts
Outdated
import { generateBasicToken } from "../utils/auth"; | ||
import { CART_ITEMS_ENDPOINT } from "./endpoints"; | ||
|
||
// const API_URL = import.meta.VITE_USER_API_URL; |
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.
오 혹시 필요한 주석일까용?
setCategory, | ||
resetPage, | ||
selectedCategory: category, | ||
selectedSort: sortOptionsMap[sortOption as "price,asc" | "price,desc"], |
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.
sortOptionsMap
을 구체적으로 타이핑했는데도, 여기에 타입 단언이 들어가 있어서 코멘트 남겨보아요.
타입 단언은 조용히 타입에러를 숨겨주기 때문에, 꼭 필요한 경우가 아니라면 사용하지 않는 것이 좋습니다.
이 경우는 꼭 필요한 경우가 아닌 것으로 보여요.
selectedSort: sortOptionsMap[sortOption as "price,asc" | "price,desc"], | |
const [sortOption, setSortOption] = useState<"price,asc" | "price,desc">("price,asc"); |
그리고 price,
이 반복되어 사용되는데, price,
는 필요한 곳에서 붙이고, 타입이나 상태의 값은 'asc' | 'desc' 로 관리하면 어떨까 하는 생각도 듭니당
천천히 보시고 2단계에서 이어서 고민해주셔도 좋을 것 같아요 :)
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.
2단계에서 반영할 수 있도록 해보겠습니당
- [x] 상품을 가격 순으로 정렬한다. | ||
- 낮은 가격 순 (asc) | ||
- 높은 가격 순 (desc) | ||
- [ ] 장바구니에 상품 담기 |
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.
장바구니에 담고 빼는 버튼의 동작은 장바구니에 이미 상품이 담겨있는지 확인하여 진행이 돼요. 그런데 이 장바구니에 있는지 확인하는 작업의 싱크가 맞지 않아서 그런 것 같아요! 수정했습니당
- 높은 가격 순 (desc) | ||
- [ ] 장바구니에 상품 담기 | ||
|
||
- [x] 장바구니에 담긴 아이템 종류의 갯수로 숫자를 표시한다. |
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.
수정 요청사항
1️⃣
사용자가 담기 버튼을 누르면, 장바구니에 추가된다. 이 때 장바구니에 담긴 아이템 '종류' 의 갯수로 숫자를 표시한다.
현재 장바구니에 성공적으로 담아도, 장바구니 숫자가 업데이트 되지 않고 있어요.
성공한 경우, 바로바로 장바구니 숫자가 올라가도록 변경이 필요해보입니다.
Screen.Recording.2024-06-05.at.10.08.54.PM.mov
2️⃣
API 요청 중 에러가 발생한 경우, 에러 메시지를 사용자에게 알려주는 UI를 표시한다.
현재 요청에 실패하더라도 아무런 UI가 표시되지 않고 있어요.
실패 시 해당 내용을 알려주는 UI가 노출되도록 변경이 필요해보입니다.
Screen.Recording.2024-06-05.at.10.09.51.PM.mov
3️⃣
현재 신규 진입 시 cart-items?page=0&size=20&sort=
로 불필요하게 많은 API 요청이 발생하고 있어요.
필요한 만큼만 호출할 수 있도록 변경이 필요해보입니다.
Screen.Recording.2024-06-05.at.10.06.57.PM.mov
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.
초코, 안녕하세요 :)
컨디션은 좀 회복되셨을까요 🙏💊
빠르게 재요청주셔서 감사합니다. 꼼꼼하게 학습하고 적용해주셔서 너무 좋네요 💯 고생많으셨어요 👍👍👍
2단계로 넘어가기 전에, 1단계에서 반영되면 좋을 것을 코멘트로 공유드려보아요.
1️⃣, 2️⃣ 는 꼭 1단계에서 마무리되면 좋을 것 같고,
3️⃣은 2단계에서 이어서 이야기 나누어보아도 좋을 것 같습니다 :)
- 장바구니 상품 동기화 로직 순서 수정 - 버튼 동작 코드 분리
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.
초코, 상품목록 1단계 너무너무 고생많으셨습니다 🍫
학습한 내용과 개선한 내용을 꼼꼼하게 정리하고 공유해주셔서 리뷰하는데 도움 많이 받을 수 있었어요 감사합니다 💝 2단계에 이어서 더 이야기 나누어보아요.
고생 많으셨습니다 👏👏👏👏
안녕하세요 하루, 초코🍫입니다.
그동안 크루들이 하루의 지극정성 리뷰에 감동받아 궁금했었는데, 드디어 만나뵙게 되어 반갑습니다!!
여러 가지 오류가 많은 프로그램이지만 리뷰 잘 부탁드립니다!
💻 로컬 실행
🔗 배포 링크
바로가기
🔑 키워드 및 학습 목표
😮💨 상황공유
저는 우아한테크코스를 시작하며 웹 프론트엔드에 입문했기에 리액트 숙련도가 낮다는 점 참고해주세요! 그래서 UI/UX를 기반으로 리뷰해주시고, 키워드 및 학습 목표에 맞게 조언과 리뷰해주시면 감사하겠습니다!
+) 추가로 페이지 확인할 때 chomre의 설정에서 안전하지 않은 콘텐츠를 허용으로 바꿔줘야 제대로 보인다고 합니다!
❓ 질문사항
styled component를 사용할 때 import 방식 중 어떤 것이 더 좋을까요.? 저는 1번 방식을 선호하는데 하루의 의견이 궁금해요
import { StyledBaseButton } from './baseButton.styled';
: 이는 임포트하는 것이 너무 많아지지만 리팩터링에 명확하게 대응할 수 있음import * as S from './baseButton.styled';
: 이는 임포트문이 짧아지지만 합성컴포넌트와 호출방식과 헷갈릴 수도 있음수업 자료인 react-basecamp를 기반으로 초반에는 TDD 방식을 고수했기에 시간이 많이 할애되었어요. 그런데, 후반에 기능 구현에 어려움을 겪으면서 테스트 코드를 챙기지 못했어요. 하루는 이렇게 시간이 촉박한 상황에서 TDD 방식과 일반 구현 후 TEST 방식 중 어느 것을 선호하시나요? 제가 앞으로 프로젝트할 때에도 시간이 촉박한 상황이 계속될텐데, 어떠한 방식을 고집해야 할지 고민이라서 질문드립니다!
MSW 를 이번에 처음 사용해 보았어요. 이 때 데이터의 형식은 swagger API에서 제공하는 데이터와 동일하게 가져가야 하는지, 아니면 기능 구현에 필요한 데이터만 뽑아서 구성하고 있으면 되는지 모르겠어요. 어느정도 디테일을 맞춰야 하는 걸까요?
이번에는 페어와 둘 다 오류 해결에 능숙하지 않아서 에러사항이 많았어요 😢 무한스크롤을 구현하여 다음 페이지의 상품을 불러오긴 하는데, 불러올 때마다 리렌더링이 일어나서 인지(?) 스크롤이 초기화되어 맨 위로 올라가는 문제가 있었어요. 몇 시간동안 시도해보았지만 해결하지 못한 채로 올려요.ㅠ 혹시 해결 방안에 대해 힌트라도 얻을 수 있을까요??
추가로 현재 장바구니에 담고 빼는 버튼 기능을 API 연결만 하면 된다고 생각하였는데요, 현재 장바구니 카운트가 바로 반영이 되지 않아요. 전역상태로 관리를 했어야 했는가? API 연결이 다가 아닌가? 추가로 무엇을 해야 하는가? 물음표만 남긴 채로 시간이 끝나버렸어요. 이것 또한 리팩터링 기간에 반영하고 싶어 힌트 요청합니다아,,ㅎ