[2단계 - 페이먼츠] - 에프이(박철민) 미션 제출합니다. #382
Conversation
There was a problem hiding this comment.
안녕하세요 에프이~ 말씀 주신대로 커스텀 훅을 관심사 분리 측면에서 적용하려는 모습이 잘 보였습니다. 앱에서 테스트해보았을 때 UX도 깔끔하게 잘 만들어주셨네요 👍 세부 코멘트 확인 부탁드리고 질문에 대한 답변으로 마무리하도록 하겠습니다!
현재 카드 소유자 이름 입력 폼에서 Enter 키를 눌러야 다음 폼으로 넘어가는데, 모바일에서는 '다음' 버튼으로 보여 다음 폼으로 넘어가지지 않습니다.
제가 생각하는 바람직한 방법은, 실제 레퍼런스가 될 만한 다른 앱을 참고하여 그걸 바탕으로 구현 혹은 디자이너에 제안해볼 수 있을 듯해요. 인풋을 작성하는 UX는 자주 확인할 수 있으니 한번 찾아보시면 좋�겠습니다!
그리고 개인적으로 (가이드라인으로는 알고 있지만) 인풋 입력 후 엔터를 눌렀을 때 다음 인풋으로 넘어가는 것은 조금 우려되는 지점이 있을 것 같아요. 폼 내에 submit 버튼이 있을 경우 엔터 키를 눌렀을 때 폼 제출되는 동작이 native이기 때문에 이와 충돌되는 UX가 생길 수도 있을 듯해서요! 물론 이벤트가 상위로 전파되는 것을 막을 수는 있겠지만, 그보다는 다른 UX를 고려해봐도 괜찮지 않을까 싶습니다.
src/types/card.ts
Outdated
| cardPassword: string; | ||
| } | ||
|
|
||
| export interface CardFormValidity { |
There was a problem hiding this comment.
CardInfo와 강한 연관성을 띄는 파생 타입으로 보이는데, 필드를 일일이 직접 선언해주기보다 기존 타입에서 확장하는 방식으로 고민해보면 좋을 것 같아요!
There was a problem hiding this comment.
CardFormValidity는 커스텀 훅으로 로직을 분리하기 이전에 NewCardInputPage에서 각 form별 상태를 state 하나로 관리하기 위해 사용했던 인터페이스였네요...! 지금은 사용하지 않는다는 것을 뒤늦게 확인한 점 죄송합니다😅
사용 여부와는 별개로, 저도 기존 타입을 이용하는 것이 더 적절하다고 생각합니다!
CardInfo를 순회하면서 key들을 CardFormValidity 타입에 동적 생성하면 될 것 같아요.
| onFocus={onFocusChange} | ||
| onBlur={onFocusChange} |
There was a problem hiding this comment.
onFocus와 onBlur는 각기 다른 시점인데, 같은 핸들러로 처리하신 이유가 있을까요?
There was a problem hiding this comment.
각기 다른 시점이지만 두 이벤트 모두 카드 앞/뒷면 상태를 토글한다는 공통 action이 있기 때문에 하나의 핸들러로 처리했습니다.
{cardOwnerNameState.isNextVisible && (
<>
<Title content={CARD_META_INFO.cardCVCNumber.query} />
<CardCVCNumberInput
cardCVCNumber={cardCVCNumberState.value}
cardCVCNumberError={cardCVCNumberState.error}
onCardCVCNumberChange={handleCardCVCNumberChange}
onFocusChange={() => setIsCardFront((prev) => !prev)} // 카드 face 토글
/>
</>
)}There was a problem hiding this comment.
인터페이스를 제공하는 쪽이 사용하는 쪽의 상황에 맞추어 인터페이스를 수정하게 되면, 이는 컴포넌트 간 강결합을 불러일으키면서 재사용성을 점차 떨어트리게 됩니다. (이로 인해 제공하는 쪽은 어느정도 독립성을 가져야하는 이유이기도 합니다.) 예컨대 언젠가 focus, blur 시점에 각기 다른 이벤트를 처리해주고자하는 니즈가 생길 수 있는데, 현재와 같이 운용한다면 이를 대응하기가 아무래도 어려울 듯 해요. 때문에 인터페이스는 분리하고 사용하는 쪽에서 이 컨벤션을 맞추도록 하는 게 좋을 것 같습니다.
| <InputField> | ||
| <Label htmlFor='card-password'>{CARD_META_INFO.cardPassword.label}</Label> | ||
| <Input | ||
| id='card-password' |
There was a problem hiding this comment.
현재는 크게 문제 없겠지만 어떠한 컴포넌트에서 id를 직접 선언할 때는, 해당 컴포넌트가 페이지 내에 여러 개 렌더링 될 가능성을 고려하여 유일값을 부여하는 것이 좋을 듯 합니다!
There was a problem hiding this comment.
질문
{Array.from({ length: cardNumbers.length }, (_, index) => (
<Input
key={index}
id={`card-number-${index}`}
type='text'
placeholder='1234'
value={cardNumbers[index]}
maxLength={INPUT_RULES.maxCardNumberPartLength}
size='small'
isError={cardNumberErrors[index]}
onChange={(e) => onCardNumberChange(e, index)}
autoFocus={index === 0}
/>
))}4개의 input이 렌더링되는 CardNumberInput의 경우 length만큼 반복하면서 반환된 index 값을 사용해서 유일한 id를 설정했습니다.
하지만 map을 사용하지 않고 단순히 여러 개의 컴포넌트가 나열되는 경우를 생각해보았을 때, card-password-0과 같이 숫자를 하드코딩하는 방식은 좋지 않을 것 같습니다.
이런 이유로 uuid를 도입할 수 있을 것 같습니다.
혹시 카일이 의도하신 방법이 있는지, 그리고 uuid의 사용이 적절한지 궁금합니다!
There was a problem hiding this comment.
음 우선 id를 선언해야 하는 이유에 대해 먼저 생각해보면 좋을 것 같습니다.
- 스타일 목적이라면 -> styled-component 사용
- DOM 요소 조작 목적이라면 -> ref 사용
때문에 id가 따로 사용되어야 하는 이유가 있는지 먼저 자문해보면 어떨까 싶어요. (React 내에서 id를 사용하는 경우는 잘 없었던 것 같아서요!) 그럼에도 필요하다면, uuid 사용을 고려해볼 듯 합니다.
src/constants/card-app.ts
Outdated
| export const CARD_COMPANIES = [ | ||
| 'BC카드', | ||
| '신한카드', | ||
| '카카오뱅크', | ||
| '현대카드', | ||
| '우리카드', | ||
| '롯데카드', | ||
| '하나카드', | ||
| '국민카드', | ||
| ]; |
There was a problem hiding this comment.
카드 회사 정보는 리터럴 자체에 의미를 가지고 있기 때문에 enum 혹은 union type으로 관리하는 것이 더 적절할 듯 합니다
type CardCompany = 'BC카드' | '신한카드' | ...There was a problem hiding this comment.
위 CARD_COMPANIES는 드롭다운의 option prop으로 전달하는 배열입니다!
카드사 이름을 union type으로 관리한다고 해도 위와 같이 타입을 모두 담고 있는 배열은 따로 정의해줘야 할 것 같습니다!
질문
카드 회사 정보와 그 배열을 관리하는 두 가지 방법을 시도해보았습니다.
- union type을 먼저 정의하고
CardCompany[]타입의 옵션 배열을 만드는 방법입니다.
export type CardCompany =
| 'BC카드'
| '신한카드'
| '카카오뱅크'
| '현대카드'
| '우리카드'
| '롯데카드'
| '하나카드'
| '국민카드';
export const CARD_COMPANY_OPTIONS: CardCompany[] = [
'BC카드',
'신한카드',
'카카오뱅크',
'현대카드',
'우리카드',
'롯데카드',
'하나카드',
'국민카드',
];- 카드 회사 배열을 바탕으로 union type을 만드는 방법입니다.
export const CARD_COMPANIES = [
'BC카드',
'신한카드',
'카카오뱅크',
'현대카드',
'우리카드',
'롯데카드',
'하나카드',
'국민카드',
] as const;
export type CardCompany = (typeof CARD_COMPANIES)[number];
두 번째 방법은 코드가 더 간결하지만 배열에서 타입을 뽑아내는, 선후 관계가 뒤집힌 느낌이 듭니다.
이런 경우 어떤 방법이 더 적절할지 궁금합니다!
There was a problem hiding this comment.
위 CARD_COMPANIES는 드롭다운의 option prop으로 전달하는 배열입니다!
넵 이 부분은 인지하고 코멘트 드린 것입니다! 컴포넌트 내에서 데이터와 연관된 타입을 사용해야하는 경우가 있다면, 대부분 데이터에 기반한 원형 타입 -> 특정 UI를 위한 타입으로 파생되는 데요, 기존과 같이 유지하거나 질문 주신 두번째 케이스인 경우에는 파생 타입에서 원형 타입을 추출하는 형태이기에 선후 관계가 뒤집힌 게 맞습니다.
따라서 첫번째 방식이 좀 더 적절하다고 볼 수 있겠습니다.
|
안녕하세요 카일! 1차 피드백 반영을 완료했습니다. 🌐 배포 주소App: [바로 가기] Storybook: [바로 가기] 🔧수정 사항Github Pages 라우팅 문제 해결
카드 소유자 이름 입력 폼
공용 Button 컴포넌트 구현
기타 수정 사항
HTMLAttribute 속성 관련 문제 해결
// 공용 Label 컴포넌트
interface LabelProps extends React.HTMLAttributes<HTMLLabelElement> {
children: React.ReactNode;
}
const Label = ({ children, ...props }: LabelProps) => {
return <StyledLabel {...props}>{children}</StyledLabel>;
};Label 컴포넌트의 Props interface가
❓질문
|
안녕하세요 카일! step2의 구현을 마치고 돌아왔습니다. step1에 비해 구현할 것도 많아지고 코드도 길어지다 보니 로직을 분리하기 위해 커스텀 훅을 작성하는 데 집중했습니다.
🌐 배포 주소
App: 바로 가기
Storybook: 바로 가기
📚구조
파일 구조 트리 확인하기
📦src
┣ 📂assets
┃ ┣ 📜cardAdded.png
┃ ┣ 📜ellipse.png
┃ ┣ 📜masterCard.png
┃ ┗ 📜visaCard.png
┣ 📂components
┃ ┣ 📂CardConfirmButton
┃ ┃ ┗ 📜index.tsx
┃ ┣ 📂CardCVCNumberInput
┃ ┃ ┗ 📜index.tsx
┃ ┣ 📂CardNumberInput
┃ ┃ ┣ 📜index.stories.tsx
┃ ┃ ┗ 📜index.tsx
┃ ┣ 📂CardOwnerNameInput
┃ ┃ ┣ 📜index.stories.tsx
┃ ┃ ┗ 📜index.tsx
┃ ┣ 📂CardPasswordInput
┃ ┃ ┗ 📜index.tsx
┃ ┣ 📂CardPreview
┃ ┃ ┣ 📜CardCVCNumberDisplay.tsx
┃ ┃ ┣ 📜CardNumberDisplay.tsx
┃ ┃ ┣ 📜CardPreview.stories.tsx
┃ ┃ ┣ 📜CardPreview.tsx
┃ ┃ ┣ 📜CardText.tsx
┃ ┃ ┗ 📜ExpirationDateDisplay.tsx
┃ ┣ 📂CardSubmitButton
┃ ┃ ┗ 📜index.tsx
┃ ┣ 📂common
┃ ┃ ┣ 📜Arrows.tsx
┃ ┃ ┣ 📜Caption.stories.tsx
┃ ┃ ┣ 📜Caption.tsx
┃ ┃ ┣ 📜Dropdown.stories.tsx
┃ ┃ ┣ 📜Dropdown.tsx
┃ ┃ ┣ 📜Input.stories.tsx
┃ ┃ ┣ 📜Input.tsx
┃ ┃ ┣ 📜Label.tsx
┃ ┃ ┗ 📜Title.tsx
┃ ┣ 📂ExpirationDateInput
┃ ┃ ┣ 📜index.stories.tsx
┃ ┃ ┗ 📜index.tsx
┃ ┗ 📂layout
┃ ┃ ┣ 📜AppLayout.tsx
┃ ┃ ┣ 📜ErrorPageLayout.tsx
┃ ┃ ┣ 📜InputPageLayout.tsx
┃ ┃ ┗ 📜SubmitPageLayout.tsx
┣ 📂constants
┃ ┗ 📜card-app.ts
┣ 📂hooks
┃ ┣ 📜useCardCVCNumberInput.ts
┃ ┣ 📜useCardNumberInput.ts
┃ ┣ 📜useCardOwnerNameInput.ts
┃ ┣ 📜useCardPasswordInput.ts
┃ ┣ 📜useCardType.ts
┃ ┗ 📜useExpirationDateInput.ts
┣ 📂pages
┃ ┣ 📜CardSubmitPage.tsx
┃ ┣ 📜ErrorPage.tsx
┃ ┗ 📜NewCardInputPage.tsx
┣ 📂styles
┃ ┗ 📜global.ts
┣ 📂types
┃ ┣ 📜card.ts
┃ ┗ 📜index.d.ts
┣ 📂utils
┃ ┗ 📜errorCaption.tsx
┣ 📂validators
┃ ┗ 📜cardInputValidator.ts
┣ 📜App.css
┣ 📜App.tsx
┣ 📜index.css
┣ 📜main.tsx
┗ 📜vite-env.d.ts
커스텀 훅으로 도메인 로직과 UI 분리
useCardNumberInput,useExpirationDateInput,useCardOwnerNameInput,useCardCVCNumberInput,useCardPasswordInputisNextVisible값을 커스텀 훅 내부에서 계산하고 반환합니다.❗기능 구현 사항
카드 정보 입력 폼 동적 생성
이전 폼의 값을 변경해서 유효하지 않게 된 경우에도 다음 폼이 사라지지 않도록 구현했습니다.오토 포커싱
Enter키를 눌렀을 때 다음 폼으로 포커스가 이동합니다.유효성 검사
카드 제출
확인버튼이 렌더링됩니다.확인버튼을 클릭할 경우 제출 페이지로 이동하며,카드 시작 번호와카드사정보가 표시됩니다.카드 시작 번호와카드사정보는 navigate의state로 전달합니다.에러 페이지
에러 페이지 사진 보기
관심사분리 측면에서 접근해보면 좋을 것 같다’는 준의 의견을 참고했습니다. UI와 나머지 로직들을 분리하는 데 초점을 맞췄습니다😊❓ 질문
Enter키를 눌러야 다음 폼으로 넘어가는데, 모바일에서는 '다음' 버튼으로 보여 다음 폼으로 넘어가지지 않습니다.모바일 구동 화면 보기
컴퓨터에서의 Enter키 이벤트를 모바일까지 고려하면 어떻게 처리해주는 것이 자연스러울지 카일의 의견이 궁금합니다! 일정 글자 수가 되면 다음으로 넘어가게 하는 방법도 생각해보았는데, 적절한 방법은 아닌 것 같습니다.
이번 리뷰도 잘 부탁드려요!