MISSON3 / 이규민 #24
MISSON3 / 이규민 #24
Conversation
Deacription: 이전에 계속해서 CardNumberInput의 마지막 DefaultInput에서 값을 4개 입력해도 3개만 출력되는 것을 수정
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
안녕하세요 이규민님 전체적으로 코드를 보는데 시간이 조금 걸렸지만 코드는 잘 짜신 것 같습니다. 다만 몇 가지 조언이 필요할 것 같아서 질문해 주신 부분과 조언을 드리도록 하겠습니다!
- input 상태를 관리하는데 있어서 값을 바꾸는 로직이 많은 위치에 혼재 되어 있는 것 같습니다. 이는 validation을 한 곳에서 하려해서 발생한 문제라고도 할 수 있을 것 같습니다 input 하나하나에게 기본적인 validation을 넘겨주고, submit에서 submit validation을 나누는게 좋을 것 같습니다
- 두 번째로 각각의 input이 자신의 입력값을 관리하고 있지 못한걸로 보입니다 value값이 내려가도록 하셨지만 내려가고 있지 않기때문에 값이 초기화 되지 않았던 것입니다.
- 코드 구조가 복잡하다고 느꼈는데, 복잡한 상태를 props 형태로 내려보냈기 때문인 것 같습니다. 많은 곳에서 필요한 복잡한 상태를 관리할때는 전역상태관리 라이브러리들을 사용해 봄직히 좋을 것 같습니다. Context API, Redux, Recoil, MobX 등이 있습니다! 한번 찾아보고 등록해보면 상태를 관리하는데 용이하다고 느끼실 것 같아요!
질의응답
이러한 경우에 ‘상태 끌어올리기’를 하여 CardInfo 데이터를 아예 MainPage에서 관리하고, MainLayout과 CardListModal 각각에 props로 전달해주는게 더 간단하고 알기쉬운 데이터 흐름 같아 보이는데, 제 생각이 올바른지 궁금합니다!
CardInfo는 입력상태값으로 보이는데요, CardInfo를 다루는 비즈니스 로직이 Input들과 가까운 MainLayout에서 관리하는게 더 좋아 보입니다. 과도한 Props 드릴링은 문제를 유발 시키기 때문입니다. 이 때 불편하셨던 내용이 상태를 가지고 올라오는건데, 사실 이미 함수의 인자를 통해 받고 계시기 때문에 이부분은 문제가 없어보입니다. 실무에서도 함수의 인자로 받는 방식으로 부모 컴포넌트에 상태를 전달합니다.
또, CardInfo라는 단어가 CardList라는 데이터에서 파생된건지 조금 헷갈렸었습니다. 이부분 한번 고여해주세요!
그럼 위의 카드번호를 관리하는 부분은 상태를 어디에서 관리하는게 좋을까요?
일관성 있게 하시면 될 것 같아요. 다른 input들도 결국에는 CardInfo라는 상태에 의존해야하는 것으로 보이는데 이 상태에 의존하는게 더 좋아보입니다. 그런데 value를 결국 전달하지 않고 내부에서 상태를 또 관리하던 것처럼 보입니다. 이 부분에서 내부에서 사용하는 상태 조차도 value로 넣지 않더라구요. 이부분 수정이 필요해 보입니다. 이런 부분 때문에 코드를 이해하는데 조금 어려웠던 것 같아요.
하지만, 의도대로 동작하지 않았는데, 이런 경우에 어떻게 해결해 줘야 하는지 궁금합니다. 해당 초기화를 MainLayout에서도 진행하였으나 동일하게 동작하지 않았습니다
위에서 말했던 것처럼 CardInfo의 data를 input들이 value로 사용하고 있지 않기때문에 초기화 되지 않은 것 같습니다!
커스텀 훅을 이용하여 현재 ‘삭제’하는 경우를 따로 hooks로 분리하였습니다. 제 생각엔 적절하게 진행했다고 생각하는데 어떻게 생각하시는지 궁금합니다
추가로 커스텀 훅을 통해 분리할만한 로직이 제 코드에 어떤 것들이 있는지 궁금합니다!
적절히 잘 사용하신 것 같고, 하나의 hook이나 함수가 상태와 관련된 많은 일을 한다면 하신 것처럼 커스텀 훅으로 분리해내는게 좋아보입니다.
혹시 유효성 감사 로직 이외에 utils에 들어가면 좋을 로직들이 어떤 것들이 있는지 궁금합니다
util이라고 하면 말 그대로 비즈니스로직이 아닌 순수로직을 의미하는 것이기 때문에, 특정 스트링을 조작한다거나, 배열을 조작하는 함수들을 모아 넣을 수 있을 것 같아요. 이 프로젝트에서 생각나는 부분은 없었습니다.
CardListModal → modal이 아니므로 파일명을 변환합니다 cardTab 정도가 적절해 보입니다.
현재 cardListModal을 그대로 map으로 돌려서 렌더링 중인데, 이 cardLisModal을 다른 곳에서도 더 쉽게 재사용하기 위해서는
MainLayout > cardTabList > cardTab 이렇게 한 단계를 추가하는게 더 좋아보입니다
MainPage에서 cardInfo 상태 전체를 관리합니다(위 상태관리 질문 관련)
CardNumberInput정도가 있을것 같습니다. 👍
전체적으로 상태 끌어올리기, props를 통한 상태 내리기 라는 어휘적인 표현 때문에 상태관리에 대한 개념이 전체적으로 꼬인 느낌입니다. ‘상태’에 대해서 조금 더 명확하게 나의 것으로 만들 수 있을만한 방법이 궁금한데, 멘토님께서는 어떤 방법으로 자신의 것으로 만드셨는지 궁금합니다!
저같은 경우에는 React 상에서는 부모의 데이터를 바꾸는 것은 callback, 자식의 데이터를 바꾸는건 Props로 이해하고 일을 해왔던 것 같습니다. 결국 React라는 프레임워크는 단방향 바인딩을 사용하고 있기때문에 이와 같은 개념을 조금 공부해보시면 좋지 않을 까 생각합니다.
추가
음, 질문에대한 답을 잘 해주지 못한 것 같아서 모호한 부분이 있다면 재차 질문을 통해 알려주세요!
마지막으로 리액트는 결국 상태를 전달하고 상태를 변경하는 부분만 잘 이해하게되면 큰 문제 없이 자기것으로 만들 수 있을 것입니다. 다만 우려되는 점은 javascript에 대해서 어디까지 이해하고 있는지가 조금은 우려됩니다. 제가 생각하는 javascript는 '이벤트'라는 것으로 돌아가는 언어라고 생각합니다. 사용자의 입력부터, 내부적으로 custom event도 만들 수 있으니 말이에요. 이런 부분과 js의 동작원리까지 이해하신다면 React라는 라이브러리는 그에 비해 그리 어려운 친구가 아니기 때문에 쉽게 정복하실 수 있으리라 생각이 됩니다.
이런 부분과 관련해서 하나의 스킬을 알려드리자면
자바스크립트에서 '&& 연산자는 앞의 조건이 False면 뒤 조건은 실행하지 않는다' 라는 걸 알면 조금 더 쉽게 짜시지 않을까 생각됩니다~!
한번 리팩토링 진행해주시고, 질문있으면 코멘트로 남겨주세요!
| const isCardNameValid = cardNameCheck(cardInfo.cardName); | ||
| const isCvcNumberValid = cvcNumberCheck(cardInfo.cvcNumber); | ||
| const isDateValid = dateCheck(cardInfo.expirationDate); | ||
| const isPassWordValid = passWordCheck(cardInfo.cardPassWord); | ||
| const isCardNumberValid = cardNumberCheck(cardInfo.cardNumber); | ||
|
|
||
| if ( | ||
| isCardNameValid && | ||
| isCvcNumberValid && | ||
| isDateValid && | ||
| isPassWordValid && | ||
| isCardNumberValid | ||
| ) { | ||
| const newCard = { ...cardInfo, id: nextId }; | ||
| setCardList((prevCardList) => [...prevCardList, newCard]); | ||
| setNextId(nextId + 1); | ||
| toast.success('카드를 성공적으로 등록하셨습니다!'); | ||
| } else if (!isCardNameValid) { | ||
| setCardNameValid(false); | ||
| toast.error( | ||
| '카드 이름을 올바르게 입력해주세요!\n10자 이하 공백, 특수문자 금지!' | ||
| ); | ||
| } else if (!isCvcNumberValid) { | ||
| setcvcNumberValid(false); | ||
| toast.error( | ||
| 'CVC 번호를 올바르게 입력해주세요! \n 공백을 제외한 숫자 3개!' | ||
| ); | ||
| } else if (!isDateValid) { | ||
| setDateValid(false); | ||
| toast.error( | ||
| '유효 기간을 올바르게 입력해주세요! \n 월 12 이하, 년도 24 이상! 공백을 제외한 숫자 4개' | ||
| ); | ||
| } else if (!isPassWordValid) { | ||
| setPasswordValid(false); | ||
| toast.error( | ||
| '비밀번호를 올바르게 입력해주세요! \n 공백을 제외한 숫자 2개!' | ||
| ); | ||
| } else if (!isCardNumberValid) { | ||
| setCardNumberValid(false); | ||
| toast.error( | ||
| '카드번호를 올바르게 입력해주세요! \n 각 공백을 제외한 숫자 4개!' | ||
| ); | ||
| } |
There was a problem hiding this comment.
하나의 함수가 카드를 등록하는 것 부터 에러핸들링에 토스트 까지 하는 것 같은데, 이경우 validation hook을 만들어 각각의 hook이 처리하도록 하는 건 어떨까요?
ex
const useValidateCardInfo = (validFn, setState,validText) => {
const checkCardInfo = (info) => {
if( validFn(info) ){
retrun true;
}else {
setState(false)
toast.error(validText)
return false
}
}
}
const handleButtonClick = (cardInfo) => {
if (
useValidateCardInfo(cardInfo.cardName)&&
useValidateCardInfo (cardInfo.cvcNumber);&&
useValidateCardInfo(cardInfo.expirationDate)&&
useValidateCardInfo(cardInfo.cardPassWord) &&
useValidateCardInfo(cardInfo.cardNumber)
) {
const newCard = { ...cardInfo, id: nextId };
setCardList((prevCardList) => [...prevCardList, newCard]);
setNextId(nextId + 1);
toast.success('카드를 성공적으로 등록하셨습니다!');
}
}
조금더 수정이 필요하겠지만 이런식으로 분리 할수도 있을 것 같습니다.
이런 부분은 이런 방식으로 valid하는 함수와 결과를 조작하는 부분으로 나눌수도 있고, step을 나누는 방법이 있을 수도 있습니다. 함수가 정말 거대해졌다면, 비즈니스 로직을 분리할 수 있을 가능성이 있어서 항상 면밀히 검토해 보면 좋을 것 같아요!
| const [cardList, setCardList] = useState([]); | ||
| const [nextId, setNextId] = useState(1); | ||
| const [cardNameValid, setCardNameValid] = useState(false); | ||
| const [cvcNumberValid, setcvcNumberValid] = useState(false); | ||
| const [dateValid, setDateValid] = useState(false); | ||
| const [passwordValid, setPasswordValid] = useState(false); | ||
| const [cardNnumberValid, setCardNumberValid] = useState(false); |
There was a problem hiding this comment.
(comment) cardList 라는 상태를 현재 이프로젝트 대부분에서 사용하는 것 같은데, ContextApi나 recoil, redux 같은 상태관리 라이브러리를 사용해 보면 좋을 것 같아요
| } | ||
|
|
||
| export const cardNumberCheck = (cardNumber) => { | ||
| const numbers = cardNumber.split('-'); |
There was a problem hiding this comment.
1111-2222-3333-4444 형태로 만들고 MainLayout에서는 이 값을 사용하게 됩니다.
추가로 이렇게 카드번호를 넘겨주기 때문에 utils에서 유효성 검사를 할때 ‘split’을 통해 배열로 분리하는 번거로움이 있었습니다. (이 또한 더 쉽게 하는 방법이 있는지 궁금합니다!)
카드 번호의 경우 16자리가 아닌 경우도 존재합니다. 때문에 -형태로 저장하지 않고 모두 붙여도 사실은 무방합니다. 각 국제 카드, 국내 카드별로 카드가 사용하는 번호의 패턴들이 다양하기 때문에 카드에 대한 정규식은 복잡할 수 밖에 없습니다! 지금 처럼 하셔도 무방합니다.
참고 사이트
https://locastic.com/blog/how-to-check-credit-card-type-with-javascript
| @@ -0,0 +1,99 @@ | |||
| import { cardNameCheck, cardNumberCheck, cvcNumberCheck, dateCheck, passWordCheck } from '../src/utils/validation'; | |||
|
|
|||
| import cardImage from '../../assets/Image/CardImage.png'; | ||
| import CardButton from '../atom/CardButton'; | ||
|
|
||
| const CardListModal = ({ info, onRemoveButtonClick }) => { |
There was a problem hiding this comment.
Modal의 경우 보통 아래 화면을 가리틑 컴포넌트에 사용하는 용어라서 구현상 적절한 것 같진 않습니다,
| return ( | ||
| <div className={styles.container}> | ||
| <div className={styles.imgBox}> | ||
| <img src={cardImage} alt='카드 이미지입니다' /> |
| import styles from '../../css/CardNumberInput.module.css'; | ||
| import DefaultInput from '../atom/DefaultInput'; | ||
|
|
||
| const CardNumberInput = ({ onChange, value, cardNumberValid }) => { |
| <div className={styles.container}> | ||
| <div className={styles.noticeBox}> | ||
| <p>카드 정보를 입력해주세요</p> | ||
| </div> | ||
| <CardNameInput onChange={handleNameChange} /> | ||
| <CardNumberInput | ||
| onChange={handleNumberChange} | ||
| cardNumberValid={cardNumberValid} | ||
| /> | ||
| <div className={styles.flexBox}> | ||
| <ExpirationDateInput onChange={handleDateChange} /> | ||
| <CvcInput onChange={handleCvcChange} /> | ||
| </div> | ||
| <CardPwInput onChange={handleCardPwChange} /> | ||
| <DefaultButton onClick={handleButtonClick} buttonText='등록하기' /> | ||
| </div> |
There was a problem hiding this comment.
vlaue 값들이 내려가지 않고 있습니다. value는 사용하지 않는 걸까요?
1. 구현 모습
용량이 커 동영상을 첨부합니다
2. 해결 과정
배포 링크
기능 요구사항
TEST
카드가 등록 되었는지카드 등록시 내부 내용이 초기화 되었는지플로우
파일 구조
파일 구조 보기
컴포넌트 분리의 기준
저번 1주차 미션 중 동완님의 미션에 기표님의 리뷰를 참고하였습니다.
개인적으로 CardListModal보다는 Card를 atom으로 만들고, CardList를 block으로 만들어서 각각 관리하는리팩터링이 필요하다고 생각합니다
결국, 아토믹 디자인 패턴도, 많은 사람들이 ‘쓰다 보니, 이렇게 하면 좋더라’라고 생각하였고,
이번 미션에 알맞게 컴포넌트를 분리하였습니다
스타일
상태 관리
상태 관련 질문점
처음에 무작정 MainPage는 ‘간단하게’ 라는 생각을 가지고, MainLayout에 CardInfo데이터를 전부 다 객체형태로 관리하였습니다
하지만, 결국 완성 전에 CardListModal에 CardInfo 데이터를 받아와주려다 보니 조금은 불편한 데이터 이동이 발생했습니다.
이러한 경우에 ‘상태 끌어올리기’를 하여 CardInfo 데이터를 아예 MainPage에서 관리하고, MainLayout과 CardListModal 각각에 props로 전달해주는게 더 간단하고 알기쉬운 데이터 흐름 같아 보이는데, 제 생각이 올바른지 궁금합니다!
추가로 제 생각대로 리팩터링을 진행하게 된다면 MainLayout 안에 CardNumberInput이라는 컴포넌트가 있는데, 카드번호를 입력하고 관리하는 곳입니다.
1stNumber, 2ndNumber, 3rdNumber, 4thNumber 이렇게 선언하여 객체로 관리하고 있습니다.
1111-2222-3333-4444 형태로 만들고 MainLayout에서는 이 값을 사용하게 됩니다.
그럼 위의 카드번호를 관리하는 부분은 상태를 어디에서 관리하는게 좋을까요?
마지막으로, MainPage에서 handleButtonClick에서 모든 값이 유효성을 통과하였을 때 상태를 초기화해주어 input에 값이 남아있지 않게 하려 아래와 같은 코드를 작성했습니다
하지만, 의도대로 동작하지 않았는데, 이런 경우에 어떻게 해결해 줘야 하는지 궁금합니다. 해당 초기화를 MainLayout에서도 진행하였으나 동일하게 동작하지 않았습니다
3. To 리뷰어에게
소중한 시간 내주셔서 정말 진심으로 감사합니다! 행복한 하루 되세요~
(현재 하고있는 행사가 있어 2주에 한번씩 판교에 가는데, 시간이 나면 한 번 뵙고 싶습니다!!)