-
Notifications
You must be signed in to change notification settings - Fork 4
kmin 과제 제출입니다. #1
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
base: main
Are you sure you want to change the base?
Conversation
add component -> card container + add 로 분리
path를 alias로 축약해서 "@components"등으로 사용하는데 storybook에서 이 alias를 정상적으로 해결하지 못함. 따라서 .storybook/main.js를 수정하였음.
input form을 위한 label추가
input에 원하는대로 focus를 적용하기 위해 dom을 직접 활용해야 함. 따라서 useRef를 사용했는데 브라우저에서 forwardRef를 사용하라고 함. forwardRef는 일반적으로 ref가 props로 사용될 수 없는데 사용될 수 있도록 만들어준다.
카드번호를 입력하기 위해 초기에 첫 번째 칸에 포커싱된다. 입력을 마칠 때 마다 다음 칸으로 포커싱이 이동된다.
add form의 ref위치를 cardNumberForm에서 cardForm으로 이동하여 카드 번호 컴포넌트와 만료일등 서로 다른 컴포넌트에서 포커싱이 이동될 수 있도록 함
flexCenter의 props interface에 children type 변경 flexColumn의 class-name style 변경
string splice 함수 생성
만료일과 카드 발급자의 위치를 조정하기 위해 생성
미작성된 항목 발견시 해당 돔으로 이동하기 위해 firstNumber ref를 생성하였음
font-family에 "Noto Sans KR"을 적용함
card의 색상을 변경하였음
card의 chip 디자인임
기존 React.ReactChildren에서 JSX.Element를 사용하는 것으로 변경
password에 props를 전달한다.
완료한 항목은 체크
todo에 작성한 내용들 구현
card, input들이 모바일화면으로 봤을 때 중앙에 위치할 수 있도록 수정
virtual keyboard의 확장성을 위해서 ref를 주입받아 사용하도록 변경
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.
추가적으로 세번째 카드 번호에 이미 4개 번호가 입력된 상태에서, 두번째 카드번호 입력 이후 세번쨰 카드 번호로 넘어갔을 때 시큐리티패드를 나갈 수 없는 오류 등 코드의 양이 많아서 모든 오류 사항을 리뷰드릴수 없었습니다 ㅠㅠ
다음번 코딩하실땐 최소한 프롭을 10개 이하 사용하시면서 처리하는게 좋을 것 같아요. 특히 폼 부분에서 언컨트롤드 작업으로 처리했으면 훨신 깔끔한 코드가 되었을 것 같네요.
const THIS_YEAR = 21; | ||
const ABSOLUTE = false; | ||
const NORMAL = true; |
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.
이런 변수들은 env로 따로 빼서 관리하면 좋을 것 같습니다!
Add, | ||
} | ||
|
||
const THIS_YEAR = 21; |
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.
이번 년도는 new Date()
로 처리 가능할 것 같습니다
const expireDateFormat = | ||
expireDate?.length === 4 ? strSplice(expireDate, '/', 0, 2) : expireDate; | ||
return ( | ||
<> |
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.
태그가 하나로 이루어진 컴포넌트의 리턴을 한번 더 태그로 감쌀 필요가 없습니다
|
||
const CardChip = () => { | ||
return ( | ||
<div className={'card-chip'} /> |
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.
<div className={'card-chip'} /> | |
<div className='card-chip' /> |
문자열로만 이루어진 prop은 중괄호로 감쌀 필요가 없습니다
import React from 'react'; | ||
import './cardChip.css'; | ||
|
||
const CardChip = () => { |
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.
CardTemplate에만 사용하는 CardChip은 굳이 컴포넌트로 나누지 않고 CardTemplate에 같이 넣어도 될 것 같습니다. 해당 내용은 styled-components 적용하시면서 연습해보시면 될것같아요
@@ -0,0 +1,4 @@ | |||
.cardList { |
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.
과제의 목표가 "모바일 UI/UX"이여서 PC로 접속할시 좀 안이쁘게 랜더링되는것 같은데, 컴포넌트에 max-width 정도를 설정해둔다면 어떨까 싶네요
firstNumbers={card.cardNumber[0]} | ||
secondNumbers={card.cardNumber[1]} | ||
thirdNumbers={card.cardNumber[2]} | ||
fourthNumbers={card.cardNumber[3]} |
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.
배열로 전달할 수 있는 방법이 없을까요...?
const [brandName, setBrandName] = useState<string>('purple-card'); | ||
const [firstNumbers, setFirstNumbers] = useState<string>(''); | ||
const [secondNumbers, setSecondNumbers] = useState<string>(''); | ||
const [thirdNumbers, setThirdNumbers] = useState<string>(''); | ||
const [fourthNumbers, setFourthNumbers] = useState<string>(''); | ||
const [expireDate, setExpireDate] = useState<string>(''); | ||
const [publisher, serPublisher] = useState<string>(''); | ||
const [cvc, setCVC] = useState<string>(''); | ||
const [password, setPassword] = useState<string>(''); |
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.
Card interface로 처리했다면 더 깔끔하지 않았을까요?
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.
context api로 card interface를 받아오면 좋았겠다는 말씀이신거죠??
setCardList: Dispatch<SetStateAction<Card[]>>; | ||
} | ||
|
||
const CardAddForm = ({ |
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.
애초에 입력 폼 자체를 contextAPI로 처리했다면 더욱 더 깔끔할 것 같네요
} | ||
|
||
.bottom-right { | ||
position: fixed; |
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.
카드 추가 폼
카드 확인