Skip to content

Conversation

@hanheel
Copy link

@hanheel hanheel commented Apr 17, 2025

🎯 미션 소개

이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.

  • 재사용 가능한 Input Component를 개발한다.
  • Storybook을 사용하여 컴포넌트의 다양한 상태를 시각적으로 테스트한다.
  • 카드 정보를 효과적으로 렌더링 하기 위한 상태 관리를 경험한다.

🕵️ 셀프 리뷰(Self-Review)

제출 전 체크 리스트

리뷰 요청 & 논의하고 싶은 내용

✅ 기능 요구 사항

  • 카드 번호 입력 및 식별
    • 사용자가 입력하는 카드 번호를 실시간으로 파악하여, Visa나 MasterCard에 해당하면 해당 브랜드의 로고를 UI에 표시한다.
    • 입력은 숫자만 가능하며, 유효하지 않은 번호 입력 시 피드백을 제공한다.
  • 카드 유효기간 입력
    • 월과 년도를 범위 내에서만 입력할 수 있어야 하며, 입력 제한을 두어 사용자가 숫자만 입력할 수 있도록 한다.
    • 실시간 프리뷰 업데이트
  • 사용자의 카드 정보 입력에 따라 카드 프리뷰를 동시에 업데이트한다.

⚠️ 주의 사항

  • 사용자의 입력 input에 포커스를 자동으로 이동하는 부분을 1단계에서 고려하지 않는다. '기능' 자체에 집중해서 구현한다. 만약 리뷰어가 사용자 - 경험 측면에서 피드백을 준다면, 피드백을 반영하는 시점에서 사용자 경험도 함께 고려하여 개선한다.
  • 기본적인 기능 요구사항을 만족하지 못한 코드는 코드 리뷰 대상이 아니다.

그리고, 이번에 저희가 개발한 기능에 대한 간략한 구조는 다음과 같습니다.

flowchart TD
    App["App.tsx"] --> AddCard["AddCard.tsx"]
    AddCard --> Card["Card.tsx"]
    AddCard --> Description["Description.tsx"]
    AddCard --> InputGroup["InputGroup.tsx"]
    InputGroup --> Input["Input.tsx"]
    %% 핵심 데이터 흐름
    Input -- "사용자 입력 → 상태 업데이트" --> AddCard
    AddCard -- "카드 데이터 전달" --> Card
    style AddCard fill:#daf1ff,stroke:#333,stroke-width:2px
    style Card fill:#e1f7d5,stroke:#333,stroke-width:1px
    style Input fill:#e1f7d5,stroke:#333,stroke-width:1px
Loading

💬 의도

validate 함수가 throw new Error 대신 에러 문자열을 반환

이번 프로젝트에서는 throw new Error 로 에러를 발생시키는 대신, validate 함수가 에러 문자열을 반환하게끔 설정하였습니다.

  • throw : 비정상적인 흐름 제어 (ex. 서버 응답 없음, 값이 존재하지 않는 경우 프로그램이 돌아가지 않는 상황)

사용자 입력 실수는 exception(예외) 라기 보단 예상 가능한 범위 안에서 흔하게 발생할 수 있는 “정상적인 흐름”이라고 판단하여 아래의 예시와 같이 코드를 작성했습니다!

export const validateCardNumber = (cardNumber: string) => {
  if (cardNumber.length === 0) return;

  if (isNaN(Number(cardNumber))) {
    return ERROR_MESSAGE.NOT_A_NUMBER;
  }

  return "";
};

❓ 질문

1️⃣ validate와 setState의 순서

현재 Input 컴포넌트의 handleCardNumber 함수는 다음과 같습니다.

const handleCardNumber = (e: React.ChangeEvent<HTMLInputElement>) => {
    const value = e.target.value;
    const errorMessage = validate(value);
    if (errorMessage && errorMessage.length > 0) {
      handleErrorMessage(errorMessage);
      setIsError(true);
      return;
    }

    handleErrorMessage("");
    setIsError(false);

    setCardInput((prev: CardInputProps) => ({
      ...prev,
      [inputKey]: value,
    }));
  };


validate 함수를 통해 유효성 검사를 실시하고, 유효한 값들만 setCardInput 에 넣어 state에 반영하고 있습니다. 이렇게 구현 할 경우 유효성 검사를 통과하지 못한 값들은 카드 프리뷰에 반영되지 않게 됩니다.
스크린샷 2025-04-17 오후 5 16 43

대부분의 input에서는 유효하지 않은 값을 프리뷰에 반영하지 않는 것이 좋지만, 유효기간 부분에서 약간의 문제가 생깁니다.
위 사진처럼 사용자가 유효 기간의 월을 제대로 입력하지 않은 경우, 유효성을 통과한 “1” 은 프리뷰에 반영됩니다. 하지만 15 전체를 적었을 땐 유효하지 않은 값이기에, “5”는 프리뷰에 반영되지 않습니다. 따라서 프리뷰에는 1/44 가 표시됩니다. 실제로는 15를 입력했는데 1을 입력한 것처럼 보이는 효과가 생기게 됩니다.
반대로, validate와 setState를 바꿀 경우, 유효하지 않은 값들도 카드 프리뷰에 반영되게 됩니다. 카드 프리뷰의 목적은 사용자가 입력한 카드 정보를 실시간으로 시각적으로 반영하여, 올바르게 입력되었는지 확인할 수 있도록 돕는 것이라 생각하기에 이 방식으로 변경하기에는 무리가 있다 판단했습니다.
validate를 진행하고 state를 반영할 때, 프리뷰에서 사용자에게 오류에 대한 부분을 어떻게 피드백하면 좋을까요?

2️⃣ 스토리북과 코드의 로직 공유

코드를 제출하기 직전, 에러메세지가 중첩되는 오류를 해결하면서 InputGroup 이 받는 errorMessage props를 객체에서 string형태로 변환하였습니다.
메인 코드가 변경됐기 때문에 storybook 코드 또한 변경된 대로 반영해야 했지만, 시간 부족 이슈로 에러를 다 찾지 못해 이전에 배포했던 링크를 삽입하게 되었습니다 (😅).
컴포넌트의 비즈니스 로직이나 상태 관리 코드가 변경되었을 때, Storybook에서 그 변경 사항을 일일이 반영하지 않아도 되도록 하기 위해 코드와 스토리북 간 로직을 공유할 수 있는 좋은 패턴이나 방법론이 있을까요?


✅ 리뷰어 체크 포인트

1. 컴포넌트 설계

  • 재사용성확장성을 고려해 Input 컴포넌트를 잘 설계했는가?
  • 컴포넌트를 분리할 기준(역할, 책임, 관심사 등)을 고민한 흔적이 보이는가?
  • UI 단위가 지나치게 작거나 크지 않고 적절한 수준으로 구성되어 있는가?

2. 상태 관리

  • 카드 정보 렌더링에 필요한 상태를 React의 상태로 적절하게 관리하고 있는가?

3. Storybook 활용

  • 컴포넌트의 다양한 상태(정상 입력, 오류, 빈값 등)를 확인할 수 있는 스토리가 작성되어 있는가?

4. UI/UX

  • 유효성 검증, 에러 메시지 등에서 사용자 경험을 고려한 처리가 이루어졌는가?

hanheel and others added 30 commits April 15, 2025 15:19
- 카드 엠블럼 변경 로직 추가 필요
- 스타일 추가 필요
@hanheel hanheel changed the title Step1 [1단계 - 페이먼츠] 블루(이한희) 미션 제출합니다. Apr 17, 2025
@wonsss wonsss self-assigned this Apr 18, 2025
@wonsss wonsss self-requested a review April 18, 2025 09:35
Copy link

@wonsss wonsss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요! 블루 😄 반가워요.
컴포넌트 분리 고민을 잘 해주신 것 같아요. 스토리북 활용도 잘 해주셨구요.
사용자 경험도 많이 고민하신 게 느껴져서 좋았습니다. 👍
페이지 컴포넌트 등 분리가 필요한 부분, 유틸 타입, html 요소 적절한 사용, 네이밍, 폴더 구조 등 여러 관점으로 리뷰를 남겨드렸어요.

확인 후 재리뷰 요청주세요~! 감사합니다. 🙇

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storybook-static 폴더는 빌드된 정적 파일들을 포함하고 있기 때문에, 보통은 git에 포함시키지 않고 .gitignore 파일에 추가해서 버전 관리에서 제외하는 것이 좋아요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스토리북도 배포하면서 정적파일이 생긴다는 부분을 생각 못했네요 😅 감사합니다!

"react": "^19.1.0",
"react-dom": "^19.1.0"
"react-dom": "^19.1.0",
"styled-components": "^6.1.17"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스타일링 방식으로 CSS-in-JS 방식을 선택하시고, 그리고 그 방식의 라이브러리 중 styled-components 를 선택하신 이유가 궁금합니다~!

참고로 Styled-components 라이브러리는 25년 3월에 기능개발 중단하고 유지보수 모드에 들어갔어요.
학습용으로는 괜찮으나 프로덕션 레벨의 새로운 프로젝트에는 Styled-components 라이브러리를 사용하는 것은 권장되지 않고 있어요.

source
https://theinnovators.zone/archives/4218
https://opencollective.com/styled-components/updates/thank-you

Copy link
Author

@hanheel hanheel Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(styled-components 라이브러리가 유지보수 모드에 들어갔다는 건 몰랐네요! 찾아보겠습니다 감사합니다 ☺️)

이번에 페어랑 css 라이브러리를 선택하는 과정에서 몇 가지 고민을 해보았습니다.

페어 분은 css module 방식만 사용해보셨고 css-in-js 방식의 라이브러리는 사용해 본 적이 없다 하셔서, 이번 미션에선 학습 차원에서 css-in-js 라이브러리를 사용하는 것이 어떨까 싶었습니다.

emotion의 경우 styled-component와 기본적으로 문법은 유사하지만, 옵션이 많고 추가 기능이 존재합니다. "CSS를 컴포넌트화 한다"라는 개념에 일단 익숙해져야 했기에, 확장 기능 없이 styled-component를 통해 css-in-js의 가장 기본적인 문법을 학습하는 것이 좋겠다고 판단했습니다. emotion의 공식문서나 이를 사용한 블로그 예제 결과를 보면 emotion의 문법들을 사용한 예제가 자주 나왔기 때문입니다!
특히, 페어 분이 리액트에도 익숙하지 않은 상태였기 때문에, 스타일링 도구까지 복잡한 선택지를 열어두는 것은 학습에 부담이 될 수 있다고 판단했습니다.(향후 프로젝트에서는 리뷰어님의 말씀대로 emotion 등의 라이브러리를 적극 사용하려고 합니다!!)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니다! 상세한 답변 감사해요.
추가적으로 CSS-in-JS의 장단점에 대해서도 직접 한 번 정리해보시면 정말 좋아요! (대댓글로 공유해주시면 더 좋구요 😄 )
단점이 있다면 “왜 단점일까? 그걸 보완하려면 어떤 대안이 있을까?” 이런 고민을 해보는 것도 추천드려요.

프론트엔드는 워낙 변화가 빠른 분야라서, 단순히 트렌드를 좇기보다 이유, 장단점, 대안 등에 고민하고 이해하는 것이 훨씬 더 큰 자산이 되는 것 같아요.

src/App.tsx Outdated
Comment on lines 1 to 2
import "./style.css";
import "./reset.css";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스타일링 방식으로 글로벌 스타일링은 css를 쓰고 컴포넌트 스타일링은 styled-components 를 사용하셨네요.
이와 같이 두 가지 스타일링 방식을 유지하신 이유가 궁금해요. 장/단점이 무엇일까요?

Copy link
Author

@hanheel hanheel Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

디자인 토큰 (색상, 폰트) 는 프로젝트 전역에 일관적으로 적용되어야 하기 때문에 css 파일로 관리하고, 컴포넌트 스타일은 해당 컴포넌트 내에 국한되어 있기 때문에 코드와 스타일을 함께 관리할 수 있는 styled-component를 쓰면 유지보수가 간단해 집니다.

하지만 여러 가지 스타일 방식이 혼용되면 추후에 "어떤 스타일은 어디서 관리되는지"를 혼동할 수 있습니다. 또한, 두 방식의 우선순위 충돌이 발생할 가능성도 있다 생각합니다.

처음에는 ThemeProvider이라는 라이브러리를 사용해볼까? 싶었습니다. 하지만 [미션의 요구 사항 문제를 해결하기 위한 용도가 아닌, 욕심을 위한 라이브러리는 지양한다.] 라는 요구사항이 있었고, 디자인 토큰 설정에 대한 부분은 "주" 요구사항으로 보기가 어렵지 않을까 싶어 사용하지 않았습니다.

리뷰를 남겨주신 이후 최대한 스타일 방식의 혼용을 줄일 수 있는 방법에 대해 모색해봤는데, globalStyle (emotion에서는 Global 컴포넌트) 을 사용할 수 있다는 것을 알게 되었습니다. 따라서 스타일을 JS 내에서 관리할 수 있게끔 globalStyle을 적용하여 코드를 리팩토링해 보았습니다!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ThemeProvider이라는 라이브러리를 사용해볼까? 싶었습니다. 하지만 [미션의 요구 사항 문제를 해결하기 위한 용도가 아닌, 욕심을 위한 라이브러리는 지양한다.] 라는 요구사항이 있었고,

참고로 styled-components에서 사용하는 ThemeProvider는 styled-components 라이브러리에서 제공하는 컴포넌트예요. 별도의 외부 라이브러리를 설치할 필요 없이 styled-components만 설치되어 있으면 사용할 수 있어요.

다크모드/라이트모드를 지원해야 하거나, 테마 색상, 폰트 등 동적으로 바뀌는 전역 스타일이 필요할 때, CSS-in-JS를 사용하는 것이 유용해요.

Copy link

@wonsss wonsss Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storybook 파일들을 모두 stories 폴더에 모아두는 구조를 선택하셨군요!
어떤 이유로 이렇게 구성하셨는지 궁금했습니다 :)

저는 보통 컴포넌트와 관련된 파일들은 같은 폴더에 모아두는 colocation 방식을 선호하는 편이에요.
예를 들어 Card라는 컴포넌트가 있을 때, 이에 대한 스토리북, 테스트 코드 등을 아래처럼 구성하는 방식입니다:

/components
   /card
     Card.tsx
     Card.stories.tsx
     Card.test.tsx

이렇게 하면 연관된 파일들을 한눈에 파악할 수 있어서 응집도를 높이고, 유지보수 시에도 해당 컴포넌트를 중심으로 빠르게 작업할 수 있어서 유용하더라고요.

특히 스토리북, 테스트 코드 등이 외부에서 재사용되는 것이 아니라면, 해당 컴포넌트 내부에 같이 두는 colocation 구조가 더 자연스러울 수 있다는 점에서 참고해 보시면 좋을 것 같아요 😊

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 이전에 다른 미션을 진행하면서도 특정 컴포넌트에서 사용하는 css, 유틸 등을 가깝게 위치했을 때 수정하고 싶은 파일을 쉽게 찾을 수 있었던 경험이 있습니다!

storybook을 처음 사용해봐서 파일 구조까지는 미처 생각하지 못했는데, 스토리북도 결국 특정 컴포넌트의 시나리오를 보여주는 것이니 가깝게 위치하는 것이 효율적이겠네요!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다른 컴포넌트 파일들과 달리 Card.tsx 컴포넌트만 /component 폴더 밑에 바로 두지 않고 /component/card 처럼 자신만의 별도 폴더 하위에 두신 이유가 궁금해요.

Copy link
Author

@hanheel hanheel Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Card.tsx 컴포넌트에서만 사용하는 maskingNumber 라는 유틸 때문에 별도의 폴더를 두게 되었습니다. 해당 함수는 현재 프로젝트에서 Card.tsx 컴포넌트에서만 사용하고 있기 때문에 공통 util 폴더에 넣는건 적합하지 않다 판단하여 사용하는 컴포넌트 근처에 위치시키기로 결정했고, 그 때문에 Card 컴포넌트만 별도의 하위 폴더가 생성되게 되었습니다!

혹시 다른 컴포넌트 파일들도 별도 폴더 하위에 두어서 통일성을 주는 것이 더 일관적일까요?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 가능한 일관성을 유지하려고 하는 편이에요 😄
정답은 없고, 나중에 많은 사람이 오랫동안 함께 유지보수를 해야 하는 큰 프로젝트라고 생각했을 때, 어떻게 해야 괜찮을까 고민하여 상황에 적절한 실행을 해보시면 어떨까요?

Comment on lines 6 to 18
type InputKeyType =
| "first"
| "second"
| "third"
| "fourth"
| "MM"
| "YY"
| "CVC";

interface InputProps {
maxLength: number;
placeholder: string;
inputKey: InputKeyType;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타입 안정성을 강화하기 위해 InputKeyType을 별도로 선언하지 않고,
inputKey에 keyof CardInputProps를 직접 타입으로 사용하는 것이 더 좋을 것 같아요.

  inputKey: keyof CardInputProps;

이렇게 하면 CardInputProps와 항상 동기화된 상태로 키를 사용할 수 있어서,
추후 CardInputProps에 필드가 추가되거나 제거될 때 자동으로 감지됩니다.

결과적으로 타입 추적이 쉬워지고, 불필요한 타입 중복을 줄이면서,
추후 리팩토링이나 오타 방지에도 도움이 될 수 있을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InputType과 CardInputProps를 따로 두면 같은 값에 대해 두 타입을 같이 관리해 줘야 하는 군요! 하나로 통일하겠습니다

gap: 10px;
`;

const Label = styled.span`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

html 의 label 요소를 사용해주세요.

그리고 label과 input을 연결해보시길 제안드려요. label과 input을 연결하는 것은 접근성을 높이는 중요한 요소입니다. label과 input을 올바르게 연결하면, 사용자가 폼을 더 쉽게 사용할 수 있고, 스크린 리더와 같은 보조 기술에서도 더 잘 작동합니다.

https://developer.mozilla.org/ko/docs/Web/HTML/Reference/Elements/label

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

label과 input을 for과 id로 연결하면 스크린리더가 자동으로 연결하여 둘을 묶어주는 군요!! 알려주셔서 감사합니다

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 그리고 스크린리더 뿐만 아니라, 일반 사용자의 사용성 측면에서도 좋습니다.
라벨을 누르면 연결된 인풋으로 focus가 가게 돼요! 😄

| "YY"
| "CVC";

interface InputProps {
Copy link

@wonsss wonsss Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Input 컴포넌트 사용 시, html input 요소의 기본 속성 사용이 추가적으로 필요하게 될 수 있어요.
그럴 때마다 매번 InputProps를 수정해주기보다, 기본 HTML input 요소의 속성을 모두 지원할 수 있도록 할 수 있어요.
react의 내장 타입인 ComponentProps<'input'>를 사용하여 props를 확장할 수 있어요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input 컴포넌트의 props가 너무 많아서 이게 재사용성이 좋은게 맞나? 를 고민 중이었는데 ComponentProps<'input'> 이라는 게 있었군요!!! step2에서 버튼을 만들 때도 ComponentProps<'button'> 을 확장해 볼 수 있겠네요. 감사합니다 👍🏻

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스토리북 케이스를 꼼꼼히 작성해주셨네요. 👍
다만, InputGroup 스토리북이 꽤 기네요. 다른 코멘트에서 제안드린 것처럼 컴포넌트 분리를 하시게 된다면, 각각의 컴포넌트에 대한 스토리북도 분리하게 될 수 있을 것 같네요.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스토리북과 코드의 로직 공유
코드를 제출하기 직전, 에러메세지가 중첩되는 오류를 해결하면서 InputGroup 이 받는 errorMessage props를 객체에서 string형태로 변환하였습니다.
메인 코드가 변경됐기 때문에 storybook 코드 또한 변경된 대로 반영해야 했지만, 시간 부족 이슈로 에러를 다 찾지 못해 이전에 배포했던 링크를 삽입하게 되었습니다 (😅).
컴포넌트의 비즈니스 로직이나 상태 관리 코드가 변경되었을 때, Storybook에서 그 변경 사항을 일일이 반영하지 않아도 되도록 하기 위해 코드와 스토리북 간 로직을 공유할 수 있는 좋은 패턴이나 방법론이 있을까요?

일단 이 스토리북 파일이 너무 크다는 것이 문제의 원인 중 하나일 것 같아요.
적절하게 분리할 필요가 있어요.
그리고 상태 관리, 유효성 검사, 에러 처리 로직을 컴포넌트 바깥으로 분리해서, 스토리북에서도 같은 훅이나 유틸을 호출하여 동일한 로직을 적용하도록 해볼 수 있어요.

로직 재사용 가능한 구조를 처음부터 만드는 습관이 장기적으로 시간도 절약하고 실수도 줄여줘요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AddCard 의 컴포넌트를 분리하는 리뷰를 주셨었는데, 이를 반영하면서 스토리북도 자연스럽게 컴포넌트 별로 분리가 된 것 같습니다. 코드 길이도 줄게 되고 확실히 InputGroup 에 대해 스토리북을 작성했을 때보다 중복되는 로직도 자연스럽게 줄어들었네요!

Comment on lines 49 to 65
const handleCardNumber = (e: React.ChangeEvent<HTMLInputElement>) => {
const value = e.target.value;
const errorMessage = validate(value);
if (errorMessage && errorMessage.length > 0) {
handleErrorMessage(errorMessage);
setIsError(true);
return;
}

handleErrorMessage("");
setIsError(false);

setCardInput((prev: CardInputProps) => ({
...prev,
[inputKey]: value,
}));
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate와 setState의 순서
validate 함수를 통해 유효성 검사를 실시하고, 유효한 값들만 setCardInput 에 넣어 state에 반영하고 있습니다. 이렇게 구현 할 경우 유효성 검사를 통과하지 못한 값들은 카드 프리뷰에 반영되지 않게 됩니다.
대부분의 input에서는 유효하지 않은 값을 프리뷰에 반영하지 않는 것이 좋지만, 유효기간 부분에서 약간의 문제가 생깁니다.
위 사진처럼 사용자가 유효 기간의 월을 제대로 입력하지 않은 경우, 유효성을 통과한 “1” 은 프리뷰에 반영됩니다. 하지만 15 전체를 적었을 땐 유효하지 않은 값이기에, “5”는 프리뷰에 반영되지 않습니다. 따라서 프리뷰에는 1/44 가 표시됩니다. 실제로는 15를 입력했는데 1을 입력한 것처럼 보이는 효과가 생기게 됩니다.
반대로, validate와 setState를 바꿀 경우, 유효하지 않은 값들도 카드 프리뷰에 반영되게 됩니다. 카드 프리뷰의 목적은 사용자가 입력한 카드 정보를 실시간으로 시각적으로 반영하여, 올바르게 입력되었는지 확인할 수 있도록 돕는 것이라 생각하기에 이 방식으로 변경하기에는 무리가 있다 판단했습니다.
validate를 진행하고 state를 반영할 때, 프리뷰에서 사용자에게 오류에 대한 부분을 어떻게 피드백하면 좋을까요?

이처럼 사용자 경험을 고민해주시는 거 정말 좋네요.
다만, 이건 요구사항 정책적인 부분이라, 제 말이 정답이라고 하긴 어려울 것 같아요.
제 생각에는 에러가 발생한 필드는 preview card에서 보여주지 않도록 빈 값으로 초기화시키는 것도 방법일 것 같아요.

@hanheel
Copy link
Author

hanheel commented Apr 20, 2025

안녕하세요 마르코!! 두 번째 리뷰 요청 드립니다!

제가 지난 번 PR을 올릴 때 자기소개와 인사를 빼먹었더라구요..

노션에 적어두고 복붙하면서 실수했나봅니다 😅

각 코멘트에 새로운 질문을 달았는데, 아래 PR에 한 번에 정리해두었습니다!

이번 리뷰도 잘 부탁드립니다! 👍🏻

🛠️ 리팩토링 사항

1️⃣ 컴포넌트 분리 (7c47b45, ff4693c, fe569e9, 176042f)

  • AddCard에서 CardNumberInputs / ExpirationDateInput[s](https://github.com/woowacourse/react-payments/pull/441/commits/ff4693cbbe32e14f34b0dc8cd58fa5bad2ab9d18) / CVCInputs 분리
  • Card 에서 CardNumber 분리 (카드 전체 틀과 렌더링 / 포맷팅 기능 분리)
  • 분리된 컴포넌트는 inputSections 폴더에 위치

2️⃣ 추상화 (129bed0, d92f4b2, 8e63087)

  • 반복되는 CardInput 컴포넌트 추상화 (배열 + map 사용)
const expirationDateKeys: ("MM" | "YY")[] = ["MM", "YY"];

{expirationDateKeys.map((key) => (
  <CardInput key={key} />
))}
  • 반복되는 getFirstErrorMessage 추상화 (에러메세지 배열 중 첫 번째 반환)

3️⃣ html 태그 변경 (8f3a10b, 306e6c0)

  • AddCard 내부 Form 스타일 컴포넌트의 태그를 div에서 form으로 변경

  • InputGroup 내부 Label 스타일 컴포넌트의 태그를 span에서 label로 변경

    (+ for / id를 사용하여 input과 연결)

4️⃣ globalStyle의 적용 (43006f6)

  • styled-component와 .css 파일 혼용으로 인한 혼동을 방지하고자 globalStyle 로 디자인 토큰 설정
  • 스토리북 내에도 globalStyle 설정

5️⃣ 파일 구조 (422b971, 730d596)

  • util 파일들은 src 하단의 util 폴더로 옮김
  • 스토리북과 해당하는 컴포넌트를 가깝게 위치 시킴

5️⃣ 스토리북 파일 분리 (b11c48d)

  • InputGroup.stories.tsx 삭제
  • 컴포넌트 분리에 따라CardNumberInputs.stories.tsx / ExpirationDateInputs.stories.tsx / CVCInputs.stories.tsx 생성

6️⃣ 그 외

  • 스토리북 정적 파일 gitignore 설정 및 캐시 삭제 (45aee65, 0e9ed3c)
  • errorMessage, handleErrorMessage 훅으로 분리 및 스토리북에서 활용 ()
  • Input 컴포넌트 네이밍을 CardInput 으로 변경 (01efe57)
  • (styled-component) UI 컴포넌트 정의를 하단으로 이동 (13c468b)
  • CardInputInputPropsComponentProps<'input'> 을 확장하여 사용하도록 변경 (7f793f0)

❓ 질문

1️⃣ 어떤 기준을 갖고 커스텀 훅을 분리하면 좋을까요?

이번 미션은 로직이 복잡하지 않아 커스텀 훅으로 상태 관리를 분리했을 때, 오히려 복잡성보다는 UI와 도메인의 역할이 명확하게 구분된다는 느낌을 받을 수 있었습니다.

다만, 앞으로 더 큰 규모의 웹 애플리케이션을 만들게 되면, 상태가 많아지고 이를 커스텀 훅으로 각각 분리하게 될 경우 관리해야 할 훅이 오히려 많아지는 문제가 생기지 않을까 하는 고민이 들었습니다.

  const [cardInput, setCardInput] = useState<CardInputProps>({
    first: null,
    second: null,
    third: null,
    fourth: null,
    MM: null,
    YY: null,
    CVC: null,
  });

  const { errorMessages, handleErrorMessages } = useErrorMessages();

이번 코드에서는 아래와 같은 기준을 두고 훅으로 분리할지 안할지를 결정해 보았습니다.

cardInput은 단순히 값을 저장하는 역할만 하기에 useState로 충분했습니다. 반면 errorMessages는 특정 키를 기준으로 상태를 조작하는 로직(handleErrorMessage) 이 필요해 커스텀 훅으로 분리했습니다.

위와 같은 기준을 갖고 훅의 사용여부를 결정해도 괜찮을까요?

2️⃣ util 함수의 위치

maskingNumber는 매개변수로 받은 개수만큼 마스킹 문자()를 반복해 반환하는 간단한 유틸 함수입니다.

export const maskingNumber = (count: number) => {
  return "•".repeat(count);
};
  1. 이 함수는 UI 표시와 밀접하게 연관되어 있다고 판단되는데, 그렇다면 별도의 util 파일로 분리하기보다는, 사용하는 컴포넌트 내부에 함께 위치시키는 것이 더 적절한 선택일까요?
  2. 재활용되지 않고 특정 컴포넌트에 종속되어 있는 유틸들도 src 바로 하단의 util 폴더에 모아두는 것이 더 효율적일까요? 오히려 특정 컴포넌트에서만 사용되기 때문에 가깝게 위치 시키는 것이 함수를 찾는 데에 편하진 않을지 고민이 됩니다.

3️⃣ 스토리북을 염두에 둔 컴포넌트 설계를 해야하나요?

현재 CardNumberInputs.stories.tsxInputError 스토리는 play 함수를 활용하여 Canvas 탭에서 실제 에러 상황을 시뮬레이션하고 있습니다.

실제 코드에서 CardInput 컴포넌트는 “이벤트를 감지해서 그 안의 값이 에러인지 아닌지 판단” 하도록 설계돼 있기 때문에, 위와 같이 구현했습니다.

하지만 위와 같은 방식으로 구현하면 docs 탭에서는 에러 상황이 바로 반영되지 않습니다 (에러가 발생했을 때 border 색상의 변경을 반영할 수 없습니다).

  1. 단순히 스토리북 문서화를 위해 원래 코드를 불필요하게 확장시키고 싶지 않다

    input에 value값을 전달하도록 props를 수정하면 문제는 해결된다. 그러나 스토리북만을 위해 새로운 props를 삽입하는 것이 맞을까? CardNumberInputs 컴포넌트부터 CardInput 컴포넌트 까지 value를 전달하면 스토리북 문제는 해결되겠지만, 실제 코드에서는 어차피 활용하지 않는 props 아닌가?

  2. docs에 실제 시나리오가 제대로 반영되지 않는다

    에러가 발생했을 때 border의 색상이 변경되지만 docs에는 이 시나리오가 제대로 반영되지 않음. 스토리북을 협업 툴로 활용해야 한다면 이런 부분이 반영되지 않는 것은 큰 문제이지 않을까?

위 두 가지 의견 사이에서 고민 중입니다.

처음 컴포넌트 설계를 할 때 스토리북을 염두에 두고 설계했어야 했을까요?

이런 상황에선 어떤 선택을 하는게 더 나은 결과를 가져올 수 있을까요?

Copy link

@wonsss wonsss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요! 블루 🟦 😄
블루가 깨닫고 정리한 내용을 자세하게 답변 남겨주셔서 감사합니다!
이번 단계에서 블루가 앞으로 코드를 읽는 사람의 입장에서 좋은 코드를 작성해야겠다고 느끼셨다고 하셔서 정말 멋지다고 생각합니다! 👍
마찬가지로 "개발할 때, 사용자 입장에서 생각하기", "PR 리뷰 과정에서도 리뷰어나 나중에 1년 뒤에 이 PR을 읽을 수도 있는 사람 입장에서 생각하기" 등이 성장과 협업에 정말 중요한 요소라고 생각해요.
여기서 연장선에서 사소한 거지만 하나 부탁드리고 싶은 점이 있어요. 리뷰 반영한 내용을 한번에 모두 모아서 하나의 글로 남겨주시는 것도 너무 좋다고 생각해요. 다만, 컨텍스트가 왔다갔다 해야 할 수 있어서 리뷰 반영한 댓글을 남겨주실 때마다 관련하여 수정한 커밋이 있으면 해당 커밋링크를 같이 첨부해주시면 리뷰하는 데 더 도움이 많이 될 것 같아요. 🙇‍♂️

추가로 남겨드린 대댓글 참고해주시고, 1단계 목표에는 충분해진 것 같아 여기서 merge할게요. 커스텀 훅 등 리뷰는 2단계에서 더 이야기 나누어 봐요. 화이팅입니다!! 💪

@wonsss wonsss merged commit 47912b8 into woowacourse:hanheel Apr 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants