[정찬욱] Payment 미션 Step1 #3

seeyoujeong · 2024-07-23T06:42:17Z

배포 링크

미션을 수행하면서 어려웠던 점

카드 정보 추가에서 카드 별칭 추가로 넘어갈때 상태를 어떻게 가져갈 수 있을까하는 부분에서 고민을 많이 했네요. 처음에는 url에 id param의 유무로 했다가 퍼널이란걸 본 기억이 떠올라서 퍼널 패턴으로 구현했습니다.
카드명과 색상 고르는 모달의 트리거를 아직 제대로 못정해서 고민이 많네요.. (지금은 카드 추가 페이지에 들어가면 바로 보입니다.)
Input 컴포넌트 같은 공용 컴포넌트가 완전 깡통(?) 같은 상태인데 어떤식으로 스토리북을 작성해야할지 고민을 많이하는 중 입니다.

리뷰 받고 싶은 부분 & 궁금한 점

코드가 많이 지저분해서 리뷰를 받기는 좀 그렇지만 모달을 보여줄 타이밍은 언제가 가장 좋을까요?
한 페이지에 상태가 많게 되면 어떻게 관리하면 좋을까요?
어떤 기준으로 컴포넌트를 관리하면 좋을까요?
커스텀 훅에 관한 테스트 코드를 ai의 도움을 받아 작성해봤는데 '이게 맞나?'라는 생각이 들어서 의견을 주시면 감사합니다.

suyeon1218

안녕하세요 찬욱님...!!! 찬욱님 코드를 보면서 리뷰를 남기기보다는 인사이트를 많이 얻을 수 있었어요! Primitive UI 를 설계하신 부분이나, 여러곳에서 반복되는 핸들러 함수들을 hooks 으로 만드신 것들이나 인상 깊게 봤습니다. 코드를 깔끔하게, 그리고 이해하기 쉽게 짤 짰다는 느낌을 받을 수 있었어요...!!

리뷰한 부분

주로 변수명이 헷갈리거나 파일의 위치가 적절한지에 대해 리뷰를 남겼습니다! 폴더 구조는 저도 고민이 많은데... 준일 멘토님께서 참조하라고 주신 링크에서 atoms/molecules/organisms 순으로 컴포넌트를 레고처럼 합치는 분도 계시더라구요!! 이게 신기해서 저도 다음엔 요렇게 폴더를 분리해볼까 생각 중입니다

그외 기타

지금은 Input이 state로 관리되다 보니 하나의 input만 입력해도 전체 form이 렌더링되는 걸 확인했어요. 그리고 한글 입력을 할 때도 화면상에선 렌더링이 아무 변화가 없지만 렌더링이 일어나더라구요...! React.memo를 적용해보는 것도 좋을 것 같아요!
카드사 선택도 모달로 잘 분리하신 걸 보면 키패트도 잘 하실 수 있지 아늘까...?! 하고 생각이 듭니다. 찬욱님이 모달 관련으로 고민을 하셨는데 도대체 어떤 부분을 고민하신 거지? 싶었어요 (잘 작성하셔서..ㅋㅋㅋ)

suyeon1218 · 2024-08-02T06:10:07Z

src/contexts/cardInfo/cardInfoReducer.ts

+    }
+    case "UPDATE_NICKNAME": {
+      const { id, nickname } = action.payload;
+      const index = state.findIndex((cardInfo) => cardInfo.id === id)!;


그냥 index 보다는 card의 ID와 일치하는 index라는 느낌이 들면 좋겠어요...!

오 그러네요. cardId로 수정해볼게요!

suyeon1218 · 2024-08-02T06:12:24Z

src/contexts/cardInfo/cardInfoReducer.ts

+      return [
+        ...state.slice(0, index),
+        updatedCardInfo,
+        ...state.slice(index + 1),
+      ];
+    }


splice 메서드를 써서 다음과 같이 작성할 수도 있는데 혹시 slice를 사용하신 이유가 있으실까요?

return state.splice(index, 1, updateCardinfo);

splice는 원본값을 수정해버려서 저렇게 했습니다!

그러면 nextState를 만들어서 state를 복사해놓은 다음 splice를 쓰는 방법은 어떠실까요?! O(2N) 으로 성능면에서 좋을 것 같아요!

오 추천해주신 방법으로 코드를 수정해야겠네요! 감사합니다~

suyeon1218 · 2024-08-02T06:14:26Z

src/contexts/cardInfo/CardInfoContext.ts

찬욱님은 따로 store를 두는 대신 CardInfo를 Context로 관리하셨군요. CardInfo가 필요한 페이지에서 자주 변경될 일도 없고, CardInfo가 아예 사용되지 않는 페이지의 렌더링 걱정을 할 필요도 없으니 Context를 사용해도 좋은 것 같아요...! 거기다 useReducer는 잘 사용하지 않는 훅인데 이렇게 구현할 수도 있구나 하는 인사이트를 얻고 가요!

추가정보를 드리면 useState도 사실 useReducer로 만들어졌다고 합니다!

오우... 역시 이론에 빠삭하세요

suyeon1218 · 2024-08-02T06:38:22Z

src/styles/index.css

+  overflow-y: auto;
 }

 .page-title {


CardList 페이지에 있는 <Title /> 태그의 style이 어디 있나 찾아보려고 했는데 index.css 에 있어서 조금 헤맸어요...! 특정 컴포넌트의 style 파일만 따로 만들어두신 이유가 있으실까요?

아 특별한 이유는 없고 기존 스타일을 가져다 쓴...ㅎㅎ

suyeon1218 · 2024-08-02T06:46:59Z

src/pages/CardListPage.tsx

우앙... 이게 바로 Primitive UI를 사용해서 만든 페이지군요...! 제가 Primitive UI가 뭔지 확실히 몰라서 막 구현했는데 많은 걸 배우고 가요...!!

아직 미완성이긴하지만 도움이 됐다니 다행입니다. :)

suyeon1218 · 2024-08-02T07:20:04Z

src/hooks/useCardForm.ts

+  const handleExpiredYear = (value: string) => {
+    const filteredValue = filterDigits(value);
+
+    setExpiredYear(filteredValue);
+  };


찬욱님 ExpiredYear 를 보고 제가 요구사항을 년도가 아니라 일수로 알고 있다는 걸 발견했습니다..... 부끄럽군요

suyeon1218 · 2024-08-02T07:22:41Z

src/components/card/CardNicknameInput.tsx

Input 요소들은 Card와 관련이 있다기 보단 CardInfoEdit과 관련이 있어보인다는 생각이 들어요! 그럼에도 Card폴더에 넣으신 건 CardInfoEdit이 이런 Input보다 한단계 위에 있는 컴포넌트라서 그런 걸까요?

깊게 생각한 부분은 아니지만 결국 카드와 연관된 일이라고 생각해서 card폴더라고 했지만... 규모가 커지면 나누는게 좋을듯하네요!

suyeon1218 · 2024-08-02T07:24:14Z

src/components/card/CardNumberInput.tsx

+
+                focusNext(e.target, (value) => value.length === 4);
+              }}


옆으로 넘어가는 기능을 hook으로 분리했군요...! 잘 배워갑니당

suyeon1218 · 2024-08-02T07:26:02Z

src/components/common/ValidationMessage.tsx

+interface ValidationMessageProps {
+  isValid: () => boolean;
+  errorMessage: string;
+  showOnBlur: boolean;


왠지 Blur시에만 보여줄 수 있는 게 아니라 다른 상황에도 보여줄 수 있을 것 같아요...! 다른 프로퍼티 이름이 있을까요?

아니면 상단에서 isValid() && showOnBlur 를 함께 묶는 것도 좋을것 같아요~!

아 맞네요. blur에만 초점을 맞춰버렸군요. showMessage나 isShow()로 수정해보는게 괜찮겠죠?

좋은 방법이라고 생각해요!!

suyeon1218 · 2024-08-02T07:28:52Z

src/hooks/useFocusNext.ts

+import { useRef } from "react";
+
+const useFocusNext = (order: string[]) => {
+  const inputRefs = useRef<Record<string, HTMLInputElement | null>>({});


어떻게 관리하시나 했는데 ref를 객체로 관리하는 방법이 있군요... 👍

suyeon1218 · 2024-08-02T07:47:15Z

src/components/card/CardNumberInput.tsx

+              maxLength={cardValidator.number.maxLength}
+              type={index > 1 ? "password" : "text"}
+              value={value}
+              name={`num${index + 1}`}


input에 직접 name을 등록하는 방법이 있군요... 지짜 마니 알아가서 찬욱님 코드 보는 게 재밌어요

suyeon1218 · 2024-08-02T07:51:20Z

src/components/common/ValidationMessage.tsx

+interface ValidationMessageProps {
+  isValid: () => boolean;
+  errorMessage: string;
+  showOnBlur: boolean;


아니면 상단에서 isValid() && showOnBlur 를 함께 묶는 것도 좋을것 같아요~!

리팩터링

seeyoujeong added 30 commits July 17, 2024 16:03

chore: vitest/ui, vitest 버전 일치

1836a32

feat: 임시로 Card 컴포넌트 추가

68d6e4a

feat: 임시로 CardEdit, CardList 페이지 추가

4360648

design: 카드의 하단 수정

6fcba60

feat: 임시로 만든 컴포넌트, 페이지 추가

305b069

feat: 포멧팅 함수 추가

3ddecca

test: domain 관련 함수 테스트 추가 및 불필요한 파일 삭제

66793f9

feat: 타입, props 수정 및 포멧팅 함수 적용

10e691e

chore: react-router-dom 설치

8745148

feat: CardNicknameEdit 페이지 추가

e0c92c1

feat: 불필요한 태그 삭제

0aa6caf

feat: 라우터 설정

2b9567f

design: body태그 수정

d7d78b3

design: margin, cursor 관련 css 추가

d31428b

feat: Card 컴포넌트에 카드 별칭 추가 및 타입 수정

20b36a6

refactor: 타입 분리

e9a361b

design: css 수정

339c87c

feat: 더미데이터 임시 추가

0c03414

feat: 카드 추가 페이지 임시 구현

b2ae69c

design: 하단 버튼 스타일 수정

f0c09ba

feat: getRandomId 함수 추가

f97c2a8

test: getRandomId 함수 테스트 추가

2e38587

design: css 선택자 수정

02ac6da

feat: 카드 정보 컨텍스트 추가

71fa24a

feat: Card 컴포넌트에 카드 크기 관련 프로퍼티 추가

9f79a06

feat: 컨텍스트 및 크기 관련 프로퍼티 반영

bf67090

chore: pnpm으로 변경

f608867

design: ellipsis 추가

1043009

feat: 카드 정보 입력 임시 구현

20e38d0

chore: 확장자 수정

0ede0d4

seeyoujeong added 9 commits July 31, 2024 22:35

feat: Input 컴포넌트 스토리 임시 추가

66f70c7

refactor: 컴포넌트 폴더 구조 수정

06f9de0

refactor: 상수 폴더 생성하고 분리

d6583e7

fix: 잘못 import한 코드 수정

9fc1fa9

feat: Flex 컴포넌트 추가

bb3f563

feat: 닉네임 입력, 카드 이름 목록 부분 컴포넌트로 분리

3a0515d

refactor: 분리한 컴포넌트로 수정

9017944

fix: import 주소 수정

c1521ac

refactor: Flex 컴포넌트 적용

a24f250

suyeon1218 reviewed Aug 2, 2024

View reviewed changes

seeyoujeong and others added 19 commits August 16, 2024 14:24

refactor: Modal 컴포넌트 수정 및 스토리 추가

db80256

refactor: 불필요한 코드 삭제

a11335c

fix: import 오류 해결

e3b6e21

refactor: Input 컴포넌트 수정 및 스토리 수정

92ce803

refactor: Input 컴포넌트 수정에 따른 수정

c182f77

refactor: Button 컴포넌트 수정 및 스토리 수정

a7a9fa0

refactor: Button 컴포넌트 수정에 따른 수정

34b5aba

chore: 불필요한 파일 삭제

5bf4906

refactor: 테스트 파일 위치 수정

872b304

refactor: useTooltip 훅 수정

c6ddc32

test: useTooltip 훅 테스트 코드 추가

526435e

refactor: 화살표 함수로 수정

3a7bdd0

refactor: Tooltip 컴포넌트 수정 및 스토리 추가

182a9f1

refactor: 스타일 적용 방식 수정

25615f9

feat: 스타일 관련 타입 추가

b298ee7

feat: Flex 컴포넌트 수정 및 스토리 추가

c39cdef

refactor: Flex 컴포넌트 적용 및 디자인 수정

cb68ffe

Merge pull request #1 from seeyoujeong/refactor

ff9837f

리팩터링

test: act에 async, await 추가

3caca0d

[정찬욱] Payment 미션 Step1 #3

Are you sure you want to change the base?

[정찬욱] Payment 미션 Step1 #3

Uh oh!

Conversation

seeyoujeong commented Jul 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

배포 링크

미션을 수행하면서 어려웠던 점

리뷰 받고 싶은 부분 & 궁금한 점

Uh oh!

suyeon1218 left a comment

Choose a reason for hiding this comment

리뷰한 부분

그외 기타

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

seeyoujeong commented Jul 23, 2024 •

edited

Loading