-
Notifications
You must be signed in to change notification settings - Fork 3
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
[Feat/#72] 예매 조회 페이지 추가 #87
Conversation
…to feat/#72/Lookup
PR 작성하느라 고생 많았어요!! 라벨 잘 지정되었는지 확인 한 번 해 주기 🫶 |
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.
고생했또
코멘트만 확인하고 머지해줘 ~
|
||
import { LookupProps } from "../types/lookupType"; | ||
|
||
const LookupWrapper = ({ handleBtn, ...item }: LookupProps) => { |
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.
p4) 저번 코드에서는 ...rest였는데, 이번엔 왜 item 인가요? rest로 통일 되었으면 좋겠습니다!
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.
일반적으로 넘겨주는 props들은 ...rest로 넘기고 API를 받아오는 부분은 ...item으로 넘기고 있습니다!
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.
LGTM~ 코드 상 문제점 없어보여서 Approve 하도록 하겠습니다.
고생하셨어요 !
|
||
import { LookupProps } from "../types/lookupType"; | ||
|
||
const LookupWrapper = ({ handleBtn, ...item }: LookupProps) => { |
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.
이유가 다 있었네요 굿굿~
{lookUpList.length ? ( | ||
<> | ||
{lookUpList.map((item) => ( | ||
<React.Fragment key={item.bookingId}> |
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.
<></> 가 아닌 fragment를 직접 사용한 이유가 있나요?
<> 안에는 key prop을 줄 수 없어서인가요?
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.
맞습니다!! map을 돌려서 key prop를 주고 싶었는데 빈 태그에는 줄 수 없어서 이렇게 작성했어요!
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.
체고신지
Quality Gate passedIssues Measures |
📌 관련 이슈번호
🎟️ PR 유형
어떤 변경 사항이 있나요?
✅ Key Changes
📢 To Reviewers
Lookup.tsx에 import된 dummyData가 있고, 빈 배열의 dummyData가 있는데 이는 분기 처리를 테스트하기 위함입니다!
-> 예매한 공연 없는 경우 경우 빈 배열 주석 풀고 import 주석
-> 예매한 공연 있는 경우 import 주석 풀고 빈 배열 주석
-> 이 부분은 API 연결하면 삭제하도록 하겠습니다!
-> 현재 dummyData가 lookup 폴더 내에 있는데 API 연결하면 지울 예정이라 임시로 두었습니다.
📸 스크린샷
🔗 참고 자료