Conversation
zereight
left a comment
zereight
left a comment
There was a problem hiding this comment.
오 안녕하세요 양하님!
네네 DM으로 페어프로그램 가능하신 날짜와 시간대 알려주시면 시간맞춰보시죠~!
은행모달 css 예쁘네요 ㅋㅋ 좋습니당
코드도 깔끔하게 작성해주시려한게 보여요 감동..
ㅎㅎ 감동끊고 제가 찾은(?) 미구현된 사항 확인해주시면 감사하겠습니다!
- 카드 소유자 이름 최대길이
- 보안코드 최대길이
- 카드별칭 최대길이
- 카드목록에서 카드 클릭시 수정페이지로 이동하지 않음
- 카드삭제
- 카드를 여러개 추가하다보면 사라지는 목록에서 카드들이 생김
- Map의 key값이 유니크한지 체크해볼 필요가 있겠네요
문의 주신사항
지금 저장될때 카드에 설정한 닉네임이 이전값을 가지는데, 이 부분 아직 수정 못한 채로 리뷰 올립니당..
RegistEnd 의 onClickButtonNextHandler 에서 문제가 있는거같아요.
- onClickButtonNextHandler 호출
- handleSetCardName 호출로 cardName 상태변경
- onNext 호출
- addMapElement 호출 이때 최신의 cardName을 가지고 수행할지는 신뢰할 수 없습니다.
2~3 사이가 동기적으로 동작할까요?!
addMapElement가 어떻게 최신의 cardName을 알 수 있게 할 수 있을까요!!!
| {[...cardList].map(([key, value]) => ( | ||
| <> | ||
| <CardBox | ||
| ownerName={value.ownerName} | ||
| expirationDate={value.expirationDate} | ||
| cardNumber={value.cardNumber} | ||
| theme={value.theme} | ||
| cardSelectionTypeName={value.cardSelectionTypeName} | ||
| /> | ||
| <div>{key}</div> | ||
| </> | ||
| ))} |
There was a problem hiding this comment.
음 key가 제대로 주어지지 않고 있는거같아요. 브라우저에서 console에러 확인해보시면 좋을거같습니다!
| handleSetCardName={setCardName} | ||
| registCardInfo={registeredData} | ||
| onNext={() => { | ||
| addMapElement() |
There was a problem hiding this comment.
함수이름을 바꿔보는건 어떨까요? cardList에 현재 카드정보를 추가하는 액션같은데요!
| const [cardName, setCardName] = useState<string>('') | ||
| const [registeredData, setRegisteredData] = useState<RegisteredDataType>(INITIAL_CARD_STATE) | ||
| const [cardName, setCardName] = useState<string>(registeredData.cardSelectionTypeName) | ||
| const [cardList, setCardList] = useState<Map<string, RegisteredDataType>>(new Map()) |
There was a problem hiding this comment.
음 이름이 list인데 자료형이 Map이군요.. 읽는이로 하여금 헷갈릴거같습니다!
| const CardBox = <T,>(props: CardProps<T>) => { | ||
| const { ownerName, expirationDate, cardNumber, onClick, theme, cardSelectionTypeName } = props | ||
|
|
There was a problem hiding this comment.
props로 받고 desctucting하는것보다 바로 descturct해서 가져오면 불필요한 line을 줄일 수 있을거같아요!
({ownerName, ex..} : CardProps<T>)
| export const Header = ({ children }: PropsWithChildren) => { | ||
| return <header>{children}</header> | ||
| } | ||
|
|
||
| export const Main = ({ className, children }: PropsWithChildren<MainProps>) => { | ||
| return <main className={className}>{children}</main> | ||
| } | ||
|
|
||
| export const Footer = ({ className, children }: PropsWithChildren<FooterProps>) => { |
| </button> | ||
| </BasicLayout.Header> | ||
| <BasicLayout.Main> | ||
| <CardBox<ThemeColorType> |
There was a problem hiding this comment.
오잉 이렇게 제네릭으로 컴포넌트를 쓰는건 처음보네요.
아마 넣은 prop으로 자동추론이 될거같습니다!
| } | ||
|
|
||
| return ( | ||
| <> |
| ) | ||
| } | ||
|
|
||
| export default RegistCard |
| type RegistEndProps = { | ||
| registCardInfo: RegisterDataType | ||
| registCardInfo: RegisteredDataType | ||
| handleSetCardName: (value: string) => void |
| const [cardSelectionInfo, setCardSelectionInfo] = useState<CardItemType<ThemeColorType>>({ | ||
| name: '', | ||
| theme: '', | ||
| }) |
There was a problem hiding this comment.
요거 초기값이랑 들어가는 값 보기 전까지, 어떤 값을 저장하는 상태인지 확 와닿지가 않았던거 같아요!
카드선택정보? -> 어떤정보지..?
결국에 타고타고 들어가서보니, 은행이었군요!
3 participants
안녕하세요. 정혁 리뷰어님. 👋🏻
페이먼츠 미션2 목록기능 내 몇 가지 기능적 요구사항이 미구현 된 상태이긴한데,
일단 리뷰를 받고싶어서 PR을 올립니다.
(merge 컨플릭트는 왜나는걸까요.. ?? pr 생성후에 컨플릭트 한 번 해결 하겠습니다.)_
=> 해결 완료(원인: 대소문자 구분 설정하면서 이전파일과의 변동사항이 conflict처럼 됨)
PR내용 + 리뷰어에게 드리고 싶은 말
지난 코드리뷰 피드백 반영 요약
추가 구현사항
추가 필요한 내용
++잡담
시간이 되신다면 끝나기전에 페어프로그래밍 시간을 한 번 더 갖고싶은데 가능하실까요 ~?
프리뷰
https://github.com/next-step/react-payments/assets/48500209/5961cfa7-e238-402c-b115-c1b198034281