[2단계 - 페이먼츠 모듈] 초코(강다빈) 미션 제출합니다. #75
Conversation
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
x 버튼 이미지 추가 Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: 00kang <[email protected]>
Co-Authored-By: river <[email protected]>
Co-Authored-By: 00kang <[email protected]>
There was a problem hiding this comment.
안녕하세요 초코! 2단계도 고생 많으셨습니다!
해결되지 않은 오류들도 피드백 반영과정에서 해결할 수 있으면 작업 부탁드립니다!
- 아니요! 합성 컴포넌트가 아니더라도 이번 미션에서는 아무 문제가 없었다고 생각합니다. 한 번 적용해보는 과정에서 왜 합성 컴포넌트라면 확장이 더 쉬웠을 지에 대해서 생각해보셨나요? 그런 옵션이 존재하는 것을 인지하고, 그 장단점에 대해서 안다면 충분한 학습이었다고 생각합니다.
- 더 나은 구조와 퀄리티를 위해서 패턴이나 라이브러리가 필요하지 않습니다. 고민과 경험이 필요한 것이죠! 정말 내가 고민하고 연구해서 만들어낸 구조가 이미 누군가가 만들어서 널리 쓰고 있는 것이라면 기분이 정말 좋지 않을까요? 내가 아무런 도움도 없이 이런 걸 만들어내다니! 물론 조금 더 현실적으로는 검색을 통해서 도움을 받으면 더욱 좋겠지만요.
저는 문제를 마주하고, 그 문제에 대해서 어떻게 해결하면 좋을지 가설을 세우고, 그대로 구현해서 잘 작동하는지 확인하면서 다른 사람들은 어떤식으로 접근하는지 비교해보는 방식으로 차근차근 학습해나가면 된다고 생각합니다. 오히려 섣부르게 패턴이나 라이브러리를 활용하기 위해서 왜 그 툴이 필요한 지에 대한 고민이 전혀 없다면 그 툴을 모르느니만 못하다고 생각합니다! - 스토리북의 환경과 지금 App.tsx를 보기 위한 환경의 차이는 어디에 있을까요? Storybook도 빌드가 되고 로컬 서버에서 제공될텐데, 이 구성 방식과 프로젝트의 실행 명령어를 통해서 제공되는 환경의 차이는 어떤 것일까요? 그 부분에서부터 조금씩 실마리를 풀어가면 좋을 것 같아요.
더불어서 저는 가장 중요하다고 생각하는 부분이 있는데요. 테스트를 잘 통과하나, 실제로 활용은 할 수 없는 코드는 의미가 없다고 생각합니다! 그러니 이 부분을 잘 해결해서 활용할 수 있도록 만들면 좋을 것 같아요!! - npm의 버저닝은 기본적으로 Semver라는 규칙을 통해서 정해지고 있는데요 major.minor.patch로 구성되어 있답니다. 자세한 내용은 링크를 참고하시면 될 것 같구요! 학습을 위한 라이브러리고 많은 사람이 사용하고 있지 않으니, 눈치보지 않고 원칙에 따라서 시원시원하게 버전을 올려도 되지 않을까요!!
그리고 훅에 관련해서 아주아주아주아주 중요한 코멘트를 남겨두었는데 꼭 이 부분은 답변을 해주시면 좋겠습니다!
그럼 다시 리뷰요청 주시는 것 기다리고 있겠습니다! 화이팅입니다!
| ${size === "small" ? "width: 320px;" : "width: 320px;"} | ||
| ${size === "medium" ? "width: 480px;" : "width: 320px;"} | ||
| ${size === "large" ? "width: 600px;" : "width: 320px;"} |
There was a problem hiding this comment.
지금이라면 size가 어떤 것이더라도 무조건 width가 세번 작성되게 되겠네요? 물론 css의 특성상 문제가 없겠지만 불필요한 삼항연산자도 줄이고 코드를 줄일 수 있는 방법은 없었을까요?
There was a problem hiding this comment.
const sizeMap = {
small: "320px",
medium: "480px",
large: "600px",
};
width: ${sizeMap[size] || "320px"};
이런 방식으로 수정했습니다!
| return ( | ||
| <ModalButtonGroup> | ||
| <LightButton children="취소" onClick={onClose} /> | ||
| <DarkButton children="확인" onClick={onClose} /> |
There was a problem hiding this comment.
확인 버튼에 대해서도 똑같이 onClose만 해서는 안되지 않을까요?
There was a problem hiding this comment.
alert 모달에서는 onclose, confirm 모달과 prompt 모달에서는 onConfirm이 동작하도록 수정했습니다!
There was a problem hiding this comment.
ModalType이라는 용어는 왠지 Alert, Confirm, Prompt등 모달의 종류를 나타낼 것 같은데, 파일은 types 파일이군요! 이미 Modal 디렉토리 하위에 있으니 types.ts정도의 네이밍은 어떨까요?
| export const ButtonWrapper = styled.button<Pick<ButtonProps, "backgroundColor" | "fontColor">>` | ||
| width: 100%; | ||
| height: 44px; | ||
| const ButtonWrapper = styled.button` |
There was a problem hiding this comment.
이 부분은 제가 지난번에 놓친 것 같은데요, button 요소인데 ButtonWrapper라는 네이밍은 버튼을 감싼 컨테이너일 것 같다는 느낌이 든는것 같아요! 다른 이름을 붙여주면 어떨까요?
There was a problem hiding this comment.
StyledButton 이라는 이름으로 수정했습니다.
저는 스탕일링 코드에서 주로 Wrapper, Container라는 이름을 많이 사용하는데 적절하지 않은 걸까요? 이것들은 보통 언제 사용하는걸까요?
There was a problem hiding this comment.
prompt 모달에 사용되는 input을 위한 스타일코드인데, 위치가 적절하지 않았던 것 같습니다!
/common/Input.style.ts 에서 /Modal/Modal.style.ts 로 수정했습니다.
| fontColor="rgba(139, 149, 161, 1)" | ||
| onClick={(e) => onClose(e)} | ||
| /> | ||
| <ModalContent>{children}</ModalContent> |
There was a problem hiding this comment.
modalType이 prompt일 때는 버튼이 특정 방식으로 표시 되는 것 외에는 인풋을 넣어준다거나 하는 변경점은 없나요?
There was a problem hiding this comment.
type을 prompt로 넘겨주게 되면 input이 1개 나타나게 되어 있습니다!
| args: { | ||
| modalPosition: "center", | ||
| title: "Prompt 모달", | ||
| children: "이것은 중앙에 위치한 Prompt 모달의 내용입니다.", |
There was a problem hiding this comment.
prompt의 예시에는 input이 포함되어 렌더링 되도록 해야하지 않을까요?
There was a problem hiding this comment.
위에서 말씀드렸다싶이 이렇게 보입니다! 현재 이렇게 보이지 않는 걸까요?
hooks/src/lib/hooks/useCVC.ts
Outdated
| }; | ||
|
|
||
| export default function useCVC(): UseCVCResult { | ||
| const [cardCVC, setCardCVC] = useState(""); | ||
| const [isTouched, setIsTouched] = useState(false); | ||
|
|
||
| function validatePassword(value: string): ValidationResult { |
There was a problem hiding this comment.
이 훅은 훅으로서 존재해야만 했을까요? 훅으로 존재하는 함수들은 왜 훅이어야 할까요?
훅이 일반함수와 구분되어 존재하는 이유는 무엇일까요?
다른 것은 다 괜찮으니 이번 미션에서는 이 부분에 대해서만 완벽하게 학습해보시면 많은 것을 얻어갈 수 있을 것 같아요!!! 리액트의 공식문서의 훅에 대한 부분부터 여러 자료들을 참고해서 찾아보면 좋겠습니다! 정말 꼭이요! 정말 중요합니다!!
There was a problem hiding this comment.
미션에서 다음과 같은 요구사항이 있었기에 자연스레 훅으로 만들었던 것 같습니다. 블링 덕분에 훅과 일반 함수의 차이점을 확인해 볼 수 있었습니다. 감사합니다 :)
페이먼츠 커스텀 훅
다양한 카드사 대응에 대한 요구사항이 들어왔다. (‘AMEX, Diners, UnionPay’ 카드 대응이 안돼요ㅠㅠ)
다양한 카드 브랜드 지원 확장
카드 번호 포맷팅 기능 추가
훅(Hook)
- React에서 상태와 라이프사이클 기능을 함수형 컴포넌트에서 사용할 수 있도록 만들어진 기능이다.
훅을 사용하는 이유
- 상태 관리와 부수 효과 관리의 간편함: 함수형 컴포넌트 내에서 상태와 부수 효과를 쉽게 관리할 수 있다.
- 코드 재사용성: 커스텀 훅을 통해 상태 관리 로직을 여러 컴포넌트에서 재사용할 수 있다.
- 명확한 구조: 훅을 사용하면 상태와 부수 효과 관리 코드가 명확하게 분리되어 코드의 가독성이 높아진다.
- 함수형 프로그래밍 장점: 순수 함수와 같은 함수형 프로그래밍 패러다임을 지원하여 예측 가능한 코드를 작성할 수 있다.
위의 코드가 훅으로 존재하지 않아도 되는 이유
- 상태 관리가 필요 없음: 이 함수는 단순히 입력 값을 받아 특정 패턴에 매칭되는지를 확인하고, 매칭되면 해당 카드 브랜드를 반환한다. 상태를 저장하거나 관리할 필요가 없다.
- 라이프사이클 관리가 필요 없음: 이 함수는 컴포넌트의 라이프사이클에 따라 동작할 필요가 없다. 한 번 호출되어 작업을 수행한 후 바로 결과를 반환하면 되기 때문이다.
- 부수 효과가 없음: 부수 효과가 없기 때문에 useEffect와 같은 훅을 사용할 필요가 없다.
| let startIndex = 0; | ||
| const formattedParts = formatPattern.map((length) => { | ||
| const endIndex = startIndex + length; | ||
| const part = value.slice(startIndex, endIndex); | ||
| startIndex = endIndex; | ||
| return part; | ||
| }); |
There was a problem hiding this comment.
map의 콜백함수에서 startIndex라는 외부에 변수를 변경시키고 이를 활용하는 것은 불변성 차원에서 옳지 않은 것 같아요! 혹시 이 로직을 수정해볼 수 있을까요?
|
블링, 재리뷰 요청이 너무 늦었죠 ㅠㅠ 페어 미션이 시작되면서 3일이 너무 정신없게 지나가서 리뷰요청할 시간이 없었네요. 이전 PR에 남긴 질문들 상세히 답해주셔서 감사합니다. 😊
추가로, 학습 키워드와 학습 목표에 초점을 맞춰 리팩터링할 부분이 있다면, 짚어주시면 감사하겠습니다! 🔗 배포 링크 |
안녕하세요 블링! 초코🍫입니다.
해결되지 않은 오류사항들이 있어서 로컬 실행하며 확인해 주시면 감사하겠습니다.
페이먼츠 모듈 2단계 미션도 리뷰 잘 부탁드립니다. !!
💻 로컬 실행
🔗 배포 링크
모달 컴포넌트 npm 라이브러리 바로가기
모달 컴포넌트 storybook 바로가기
훅 npm 라이브러리 바로가기
🔑 키워드 및 학습 목표
##❓ 질문사항
2단계 미션 시작 후 확장된 추가 요구 사항을 어떻게 다 충족시킬 수 있을지 고민이 많았어요. 그래서 크루들과 이야기를 해보니, 대부분의 크루들은 1단계 미션때부터 합성 컴포넌트를 사용했기에 기능을 확장시켜도 어려울 게 없었다고 하더라구요. 저도 사용해 보려고 했으나 처음 알게 된 개념이라 그런지 원활히 동작되지 않았어요. 그러자 한 크루는 합성 컴포넌트가 답은 아니니 원래 방식대로 해보는 건 어떻냐기에 1단계 구현 방식에 이어서 구현했어요. 블링은 이번 미션이 합성 컴포넌트를 사용했어야 됐다고 생각하시나요?
더 나은 코드 구조와 퀄리티를 가지 위해서 이러한 패턴이나 상태관리 라이브러리 등을 활용해야 하는 것 같아요. 그런데 저는 합성 컴포넌트도 처음 들었다고 했듯이 리액트의 아주 기본적인 문법 이외에 다른 개념들은 모르는 상태에요. 이런 상황에서 더 나은 코드를 위해 필요한 개념들은 어떻게 접근해야 소스를 얻을 수 있는걸까요? 경험을 통해 하나씩 쌓아가는 게 답이려나요?
지난 미션인 페이먼츠 프로젝트에 이번 미션의 결과물인 npm 라이브러리를 적용시키기엔 어려움이 많아 임의로 App.tsx에서 적용시켜보았어요. 그런데 스토리북 테스트에서는 잘 작동하는 것 같던 모달이 제대로 보여지지 않더라구요. (모달 컨텐츠가 모달을 벗어나는 문제, alert 등 모달타입 지정시 버튼이 나타나지 않는 문제) 어디가 잘못된 건지 찾을 수 없어 일단 리뷰요청 드리는 상태에요. 스토리북의 스토리로 잘 확인되던 것이 실제 사용시에 문제가 생기는 경우엔 어떻게 오류를 잡아야 내야 하는걸까요?
현재는 npm 라이브러리에 변경사항이 생기면
npm version patch를 통해서 버전을 관리하고 있습니다. 그런데 step1에 이어서 기능이 확장된 거라면 step2 시작전에 minor를 업데이트 시키고, step2 진행사항에 따라 patch를 업데이트했어야 했을까요? 이렇게 학습하는 용도의 라이브러리를 업데이트시킬 땐 버전 관리를 어떻게 해야 하는지 궁금합니다!