Conversation
Yejiin21
left a comment
Yejiin21
left a comment
There was a problem hiding this comment.
퍼블리싱 하느라 수고 많으셨습니다:) 몇 가지 코멘트 남겼어요!
| <div className="flex gap-4"> | ||
| <img src={ticket} alt="ticket logo" className="w-7" /> | ||
| <div> | ||
| <p className="font-bold text-sm md:text-base">콘서트 티켓</p> |
There was a problem hiding this comment.
콘서트 티켓 텍스트 크기가 좀 더 커도 좋을 것 같아요!
콘서트 티켓과 티켓에 대한 설명 사이에 너비가 기본적으로 어느정도 있어서 mt-1도 없어도 괜찮을 것 같아요
| option.choices.map((choice) => ( | ||
| <label key={choice} className="block mt-2"> | ||
| <Checkbox | ||
| key={choice} |
There was a problem hiding this comment.
태크과 Checkbox 동시에 key={choice}를 사용하셨는데, Checkbox 컴포넌트가 이미 태그 내부에 있으므로 Checkbox 내에 설정한 key값을 삭제하셔도 됩니당
| //버튼 텍스트 | ||
| const isLastPage = currentPage === ticketAmount; | ||
| const buttonText = isLastPage ? "결제하기" : "다음 티켓 옵션 선택하기"; | ||
| const handleNextPage = () => { |
There was a problem hiding this comment.
이 함수에서 resetOptions(currentPage)를 호출하면 현재 페이지의 데이터가 모두 리셋하도록 설정하셨는데, 사용자가 페이지를 이동할 때 이전 데이터를 유지하는게 일반적인걸로 알고 있어요!
그래서 혹시 리셋 기능을 구현한 이유가 있을까요??
| const currentSelectedOptions = selectedOptions[currentPage] || {}; | ||
|
|
||
| //text | ||
| const handleInputChange = (optionName: string, value: string) => { |
There was a problem hiding this comment.
handleInputChange, handleRadioChange, handleCheckboxChange 핸들러 함수들에서 setOption(currentPage, optionName, value); 이 기능은 모두 공통적인 역할이네요!
유사한 기능이 있다면, 중복을 줄이고 하나의 handleChange 함수로 통합해서 타입에 따라 동작을 분기하는 방식으로 하면 더 좋을 것 같아요
There was a problem hiding this comment.
TicketOption.tsx는 사용자의 입력을 받아 저장하는 기능이므로 feature에 위치시킨 것 같은데 segments 분류가 이루어지지 않았네용
페이지에 보여져야하는 ui 컴포넌트여서 features/payment/ui/TicketOption.tsx 파일구조로 변경해주시면 좋을 것 같아요
hyeeuncho
force-pushed
the
feat/#70/ticket-option-response
branch
from
f87bf0d to
8a1ac01
Compare
2 participants
티켓이 1장일 경우 - 바로 결제하기 버튼
티켓이 2장 이상일 경우 - 다음 티켓 옵션 선택
마지막 페이지에서는 결제하기 버튼