[1단계 - 페이먼츠] - 버건디(전태헌) 미션 제출합니다. #343

brgndyy · 2024-04-18T07:56:37Z

스토리북 배포 링크 (재배포 완료)

안녕하세요 하루~ 잘지내셨나요 ?

레벨 1때 첫 미션을 하루가 리뷰해주셨었는데, 레벨 2의 첫 미션 또한 하루가 리뷰를 해주시네요!

좋은 리뷰어를 또 뵙게 되어서 감사하다는 감정이 듭니다.

하지만 동시에 후에 요구사항이 더 많은 미션에서 뵙게 됐다면 여쭤볼만한 부분이 훨씬 많지 않았을까..? 라는 아쉬움도 느꼈습니다 🥲 (뭔가 찬스를 벌써 사용한 기분이에요)

하지만 첫 미션이라고 하더라도 많이 까다롭다고 느껴졌고 고민해볼 부분이 많더라구요..!

여쭤보고 싶은 부분들 한번 코멘트로 남겨보았습니다.

많이 부족한 코드이지만, 잘 부탁드립니다 🙇

- 컴포넌트 분리 사진

Co-authored-by: Placeholder Author <placeholder@author.com>

Co-authored-by: brgndyy <brgndyy@gmail.com>

- 숫자 유효성 검증 - 비자 카드 유효성 검증 - 마스터 카드 유효성 검증 - 영어 유효성 검증 Co-authored-by: brgndyy <brgndyy@gmail.com>

Co-authored-by: brgndyy <brgndyy@gmail.com>

brgndyy · 2024-04-18T07:57:52Z

src/App.tsx

- useEffect에서의 유효성 검사, 어떻게 생각하실까요 ?

이번에 useInput이라는 커스텀 훅을 따로 만들어주어서

카드 유효 기간 달

카드 유효 기간 연도

카드 소유자

이렇게 3가지를 각각의 상태로 관리해주었습니다.

원래 이 useInput내에 유효성 검사 로직도 외부에서 인자로 주입해주어야하나 ? 라는 생각을 했었는데요.

만약 분기별로 나타내야할 에러 메세지가 많다면 ? 이라는 생각도 동시에 들었어요!

예를 들어 사용자 이름에 대해 에러메세지를 갖는다고 가정했을때

"등록자 이름이 너무 깁니다."

"등록자 이름은 영어 대문자여야만 합니다."

이렇게만 되더라도 동적으로 useInput내에서 에러메세지를 동적으로 매칭시켜줄수 없지 않을까? 라는 점이 걸렸었습니다.

그래서 useInput내에서는 단순히 인풋 값과, 그에 맞는 에러 상태값을 가져오고 App.tsx내에서 useEffect를 통해서 유효성 검사를 해주었는데요.

이렇게 하면 App.tsx내에서 useEffect가 너무 많아지는것 같다는 생각도 동시에 들었습니다.

useValidate라는 커스텀 훅으로 따로 뺀다고 하더라도, 결국 App.tsx 내에서 useValidate를 3번 선언하는건 마찬가지더라구요..

useEffect로 유효성 검사를 해준다는 판단, 하루는 어떻게 생각하실지, 혹시 제가 잘못 짚고 있는 부분이 있을지 여쭤보고싶어요!

App.tsx내에서 useEffect가 너무 많아지는것 같다

App.tsx 내에 useEffect가 많아지는 것에 대해 구체적으로 어떤 점이 우려가 되셨을까요?

우려되는 점이 있다면 App과 useInput 사이에 중간 레이어를 두는 방법도 있을 거 같아요.

useInput내에서 에러메세지를 동적으로 매칭시켜줄수 없지 않을까?

함수를 prop으로 받으면 말씀해주신 값에 따른 분기 처리도 가능해질까요?

더 중요하게는 언제 effect를 사용할지, 언제 event handling만으로 충분한지도 살펴보시면 좋겠습니다

⭐️추천⭐️: https://react.dev/learn/you-might-not-need-an-effect

When you’re not sure whether some code should be in an Effect or in an event handler, ask yourself why this code needs to run. Use Effects only for code that should run because the component was displayed to the user. In this example, the notification should appear because the user pressed the button, not because the page was displayed! Delete the Effect and put the shared logic into a function called from both event handlers

App.tsx 내에 useEffect가 많아지는 것에 대해 구체적으로 어떤 점이 우려가 되셨을까요?

�겉보기에도 useEffect가 많다보니, 나중에 에러가 발생했을때 디버깅에 용이하지 않을수 있다 라는 점이 있었습니다.
또한, useEffect내에서 사용 되는 로직이 유효성 검사라는 큰 틀에서 동일한 로직들이다 보니까, 비효율적이지 않나? 라는 생각도 들었던거 같아요!

함수를 prop으로 받으면 말씀해주신 값에 따른 분기 처리도 가능해질까요?

원래 말씀해주신 방식대로 구현을 하려했었는데요.

const { inputState: name, inputChangeHandler: nameChangeHandler, } = useInput([{fn : 유효성검사1 함수, message : "유효성 검사1에 맞는 에러메세지"}, {fn : 유효성검사2 함수, message : "유효성 검사2에 맞는 에러메세지"}]);

이런식의 그림을 생각했었는데, 안에서 로직이 좀 복잡해지지 않나? useInput에서는 상태 변화만 처리해주고, 그 상태변화에 띠른 유효성 검사는 외부에서 해주는게 더 깔끔한 분리이지 않을까? 라고 판단했습니다!

근데 지금 하루가 보내주신 공식문서 글을 읽어보고, 계속 생각해보고 있는데요!

setState를 통해 상태 변화를 하고, 그 후에 또 useEffect로 에러 감지를 해주어서 렌더링을 한번 더해준다는것 자체가 더 비효율적이라는 생각이 드네요..!

아예 안에 로직이 복잡해지더라도, setState 내부에서 유효성 검사를 해주는것이 더 맞겠다라는 생각이 듭니다!

짚어주셔서 감사합니다 🙇

아하, 그런 고민의 과정이 있으셨군요 👍

useEffect를 필요한 만큼 활용하는 것은 문제되지않으나, 너무 많아지면 말씀해주신 대로 디버깅이 어려워질 것 같아요. (서로 다른 관심사의 useEffect를 단지 useEffect 수를 줄이기 위해 하나의 useEffect로 합치는 것까지는 지양해야겠지만요)

setState를 통해 상태 변화를 하고, 그 후에 또 useEffect로 에러 감지를 해주어서 렌더링을 한번 더해준다는것 자체가 더 비효율적이라는 생각이 드네요..!

동의합니다. 이벤트 핸들링 동작의 일부를 useEffect로 넣게되면, 코드 흐름을 따라가기 어려워지고 따라서 디버깅 난이도가 더 올라간다고 생각해요 :)

brgndyy · 2024-04-18T07:58:10Z

src/stories/Input.stories.tsx

- 스토리북의 유용함, 스토리북의 책임

이번에 처음으로 스토리북을 만들어보고 배포를 해보았습니다!

제가 생각하는 스토리북의 필요성과 용이성은

협업을 할때 프론트엔드 분야가 아닌 협업자에게 본인이 만든 컴포넌트들을 문서화해서 직관적으로 보여줄 수 있다.

공통 컴포넌트들 같은 경우, 미리 스토리북을 작성해놓음으로써 일관성 있는 컴포넌트를 만들 수 있다.

정도 인데요..!

혹시 제가 놓친 부분이 있을까요 ? 더 추가로 생각해보아야할 점이 있다면 말씀주시면 감사하겠습니다.

그리고 스토리북을 잘짠다는건 무엇일까요 ?

스토리북 공식문서를 살펴보니, 테스트러너 같은 것으로 아예 테스트 코드를 작성할수 있는 것처럼 보이는데요..!

근데 리액트 테스팅 라이브러리랑 cypress랑 차이점이 있나..?

어차피 테스트 코드를 작성할거라면 테스팅 라이브러리로 작성을 해주는것이 낫지 않나? 라는 생각이 듭니다.

테스트는 rtl 이나 cypress 같은 테스팅 라이브러리로 작성한다.

스토리북에서는 ui, 즉 컴포넌트가 보여지는 부분에 조금 더 집중한다.

라고 역할 분리를 한번 해보았는데, 혹시 스토리북 내에서도 액션에 관한 테스트 코드까지 작성하는게 더 나은 방향성일까요?

현업에서는 스토리북의 역할을 어디까지 생각하고 활용하시는지 여쭈어보고싶습니다!

스토리북은 로딩/에러/데이터없음/정상 등 다양한 컴포넌트의 상태를 어떻게 표현할지도 시각적으로 확인할 때 매우 유용합니다. 따라서 말씀해주신 것처럼 디자이너-개발자, 개발자-개발자 등 커뮤니케이션을 돕는 문서가 됩니다. 컴포넌트를 개발하는 단계라면 스토리북에 렌더링된 모습을 확인하면서 마치 UI TDD를 하듯 개발하는데 활용할 수도 있지요.

아래 링크는 스토리북을 시각적회귀테스트의 테스트 베드로 선정한 내용을 담은 기술블로그 포스트인데, 최근 재밌게 읽은 글이라 추천드려봅니다.

참고: https://techblog.woowahan.com/17081/

그리고 스토리북을 잘짠다는건 무엇일까요 ?

제가 질문의 의도를 잘 이해했다면...

각 컴포넌트가 가질 수 있는 상태가 명확하게 구분되어, 각 상태별 UI를 시각적으로 확인하기 쉽고, 따라서 협업하는 동료들이 해당 컴포넌트를 보다 쉽게 이해하고 사용할 수 있도록 돕는 역할을 수행한다면 스토리를 잘 작성했다고 할 수 있을 것 같아요.

혹시 스토리북 내에서도 액션에 관한 테스트 코드까지 작성하는게 더 나은 방향성일까요? 현업에서는 스토리북의 역할을 어디까지 생각하고 활용하시는지 여쭈어보고싶습니다!

제 경우에 스토리북은 UI 테스트에 초점을 맞추어 사용하는 편이에요. 때때로 특정 핸들러 호출 정도의 간단한 액션을 테스트해보기도 합니다. 역시 말씀해주신 것처럼, 더 복잡한 테스트는 RTL이나 Cypress와 같은 도구를 사용하는 것이 더 효과적이겠지요.

와 글 링크까지..!

상세한 답변 감사합니다 🙇 한번 읽어보면서 더 공부해보도록 하겠습니다~

brgndyy · 2024-04-18T07:58:34Z

src/assets/images/mastercard.png

- png와 svg

이번에 이미지를 svg가 아니라 png로 설정해주었습니다.

svg를 사용하면

크기에 따라서 화질이 변화하지 않는다.

웹 환경에서 최적화가 되어있다

등의 장점이 있는거로 알고 있는데요!

근데 svg 파일 같은 경우 svgr 같은 라이브러리를 따로 설치를 해서 적용시켜주어야 하지 않나? 라는 생각 때문에 일단 png로 구현을 했습니다.

현업에서는 혹시 어떠한 파일을 더 사용할지, 어떠한 경우에서 svg, 또 어떠한 경우에선 png를 사용해야할지 여쭈어보고 싶어요!

현업에서는 혹시 어떠한 파일을 더 사용할지, 어떠한 경우에서 svg, 또 어떠한 경우에선 png를 사용해야할지 여쭈어보고 싶어요!

svg가 더 유리한 경우, png가 더 유리한 경우가 따로 있다고 생각해요.

제 경우에 로고나 아이콘은 svg로 작업합니다. 버건디가 말씀해주신 것처럼 크기가 변해도 항상 깔끔하게 렌더링 된다는 장점이 있지요.

반면에 사진, 일러스트레이션, 그라데이션이나 다양한 색상 요소를 사용한 배경이 포함된 경우 등 복잡한 디테일을 가진 이미지일 경우 png로 작업합니다. 이 경우에 svg로 다운로드 해보시면 용량 차이를 확인하실 수 있을 거예요.

말씀 감사합니다. 하루 말씀 참고하여 나중 미션에서도 비교해보고 반영해보도록 하겠습니다! 🙇

brgndyy · 2024-04-18T07:59:21Z

src/components/InputSection.tsx

컴포넌트의 재사용성에 대한 판단, 그리고 컴포넌트는 어디까지 분리하는게 맞을까요 ?

이번에 공용 컴포넌트를 Input 컴포넌트로 두고, 카드 입력 값에 대한 정보들은 InputInfo로 두었습니다.

그리고 그 안에 children 속성으로 각각 Input 컴포넌트들을 넣어주었는데요.

이것보다 아예 컴포넌트를 최대한 잘게잘게 더 분리하는게 더 나은 판단일지 확신이 서지 않습니다.

저 같은 경우는 지금까지 최대한 컴포넌트들을 나누는게 낫다라고 판단하고 공부해왔는데요..!

하지만 재사용성이라는 부분에 치우쳐져서 너무 과도하게 나누는거 아닐까? 라는 생각도 동시에 들었습니다.

지금까지 예를 들어서 Container 라는 컴포넌트를 만들었다고 한다면,

- Component.tsx

import { PropsWithChildren } from "react"; type ContainerProps = { className: string; }; export default function Container({ children, className, }: PropsWithChildren<ContainerProps>) { return <div className={className}>{children}</div>; } // 사용할때 export default function Component() { return ( <Container className={className}> <A /> <B /> <C /> <D /> </Container> ); }

아예 감싸는 컨테이너 컴포넌트도 이런식으로 만들어왔었어요!

다른 것들은 다 컴포넌트인데, 감싸는 wrapper 컴포넌트만 단일 태그로 되어있지 않다보니, 이쁘지 않다(?)라는 생각이 들어서 이렇게 해온것 같습니다.

근데 이런식으로 모든 부분을 컴포넌트화 해주면, 랩핑 그 이상의 의미가 있나? 라는 생각도 동시에 드는것 같습니다.

하루는 컴포넌트를 나눌때 어떤 기준으로 나누실까요 ?

단순히 재사용성을 넘어서서 코드 상의 일관성 (위의 모든 부분을 컴포넌트로 만들어주기 위해 Container라는 wrapper 컴포넌트를 만드는 경우)의 관점에서 컴포넌트 분리를 욕심내봐도 될까요?

한번 여쭈어 보고 싶습니다!

이쁘지 않다(?)라는 생각이 들어서 이렇게 해온것 같습니다.

제 경험으로 공유드리자면, 아래와 같은 코드를 많이 보기도, 또 작성하기도 했습니다.

<div> <A /> <B /> <C /> </div>

일관성 있게 맞추고 싶으신 그 느낌은 알 것 같지만, 단지 그것을 위해 랩핑하는 것은 제게는 조금 어색하게 느껴집니다.

랩핑이 안되어 있으면 안되어 있는 대로, '아하 여기서는 이 네이티브 요소를 사용하는구나' 하는 정보를 줄 수 있다고 생각해보면 어떨까요?

랩핑이 안되어 있으면 안되어 있는 대로, '아하 여기서는 이 네이티브 요소를 사용하는구나' 하는 정보를 줄 수 있다고 생각해보면 어떨까요?

맞는 말씀이에요! 짚어주셔서 감사해요 하루 ~!

brgndyy · 2024-04-18T08:00:29Z

src/components/composables/Input.tsx

+  padding: 8px;
+  font-size: 0.6875rem;
+  border-radius: 2px;
+  min-width: 71.25px;


피그마에서의 px

레벨1때 미션들에서도 그렇고, 이번 미션에서도 그렇고 모든 요소들의 사이즈가 px로 되어있는데요.

이런 피그마 디자인을 따라서 구현을 한다고 할때, 기재된 사이즈를 철저히 지키는게 맞을까요 ?

물론 디자이너와 의사소통을 통해, 원활한 협의가 된다면 문제는 없지 않나? 라는 생각도 동시에 드는데요.

저 같은 경우 혼자서 공부했을땐, body 같은 경우 100vw, 100vh를 준 다음에 그 안에서 속하는 부모 컨테이너(Wrapper 같은)들은 %로 주었거든요..!

- 예시

body { margin: '0', width: '100vw', minHeight: '100vh', height: 'auto', overflow: 'scroll', transition: 'all 0.3s ease', position: 'relative', } .wrapper { width: '100%', height: 'auto', minHeight: '100%', paddingTop: '5rem', paddingBottom: '4rem', } /* 후에 media query를 통해 반응형을 구현할때도 %로*/

그 후에 반응형을 구현할때도 대부분 %를 통해서 사이즈를 조절해주었는데, 근데 이러면 px이나 rem같이 구체적인 수치화를 해줄순 없겠구나 라고 생각이 들어요!

현업에서는 어떤식으로 사이징을 잡으실지 한번 여쭤보고 싶습니다!

그리고 모달 컨테이너 같이, 아예 구체적인 수치화를 해주어야 할때도, px이 아닌 rem을 사용했었는데 현업에서는 px과 rem 같은 경우도 커뮤니케이션을 통해서 결정되는 부분일까요 ?

글 링크

이 글은 px을 사용하면 사용자 설정값을 덮어씌우는 문제가 발생하여 rem을 사용하는 것이 낫다라는 내용을 담고 있는데, 하루는 어떻게 생각하실지 여쭈어보고싶어요!

음..

우선 제가 일하는 환경에서는 디자인 요구사항을 온전히 준수하는 것을 원칙으로 합니다. 모든 수치는 피그마와 같은 핸드오프 툴을 통해 결정해요. 미처 표현되지 않은 모호한 부분도 커뮤니케이션을 통해 명확하게 결정하고 디자이너의 의도를 코드로 구현합니다. 단 1px 오차라도 발생했다면 픽스 대상으로 간주하고 픽스합니다. 그래서 임의로 정해진 % 값으로 작업한다면 예상해주신 것처럼 추후 변경 시 어려움이 있을 것 같습니다.

말씀해주신 것처럼 디자이너와 협의로 변경하는 경우도 있는데, 우테코 환경에서는 함께 맞추어나갈 디자이너가 없는 환경이라는 점이 달라보이네요. 결국 중요한 것은 디자이너의 디자인 의도를 그대로 구현할 엔지니어링 역량이라고 생각합니다. (그런 의미에서 피그마 시안에 제시된 수치를 그대로 구현해보실 것을 권장드려요.)

모든 서비스에서 적용해야한다고 생각하는 것은 아니지만, rem단위를 적용했을 때 유리한 점에 대해서는 저도 공감합니다. 실무에서 rem을 사용할지 px을 사용할지는 각 팀의 기존 시스템과 니즈에 따라 결정될 것 같아요. (제 경우에는 px 위주로 작업하는 편이에요.)

제 경우에는 px 위주로 작업하는 편이에요.

오호 그렇다면 하나만 더 여쭈어보고싶어요!

그럼 현재 하루가 작업하실때의 경우에서, 반응형 같은 경우는 그럼 각각 디바이스 사이즈마다 구체적인 px이 기재가 되어있어서 그거에 맞춰서 작업을 한다 라고 이해해도 될까요 ?

즉, 현업에서는 디자이너가 현존하는 휴대폰과 웹, 테블릿 기기에 맞게 디자인을 전부 내려주는 걸까요?

레벨1 영화 미션에서 반응형을 구현했을때, 피그마 디자인에선 pc, 태블릿 휴대폰 이렇게 크게 3가지의 반응형 디자인이 있었는데요.

개발자 도구에서 임의로 반응형을 테스트 할때, 휴대폰도 기종에 따라서 컨테이너가 넘치거나 부족한 경우가 있었어서 px을 임의로 설정해주었습니다!

이런 경우에 피그마에선 따로 기재가 안되어있어서 임의로 설정 해주었거든요!

그렇다면 현업에서는 각각 기기에 맞는 반응형 사이즈들이 전부 정해져있는건지 여쭈어보고싶습니다!

현재 하루가 작업하실때의 경우에서, 반응형 같은 경우는 그럼 각각 디바이스 사이즈마다 구체적인 px이 기재가 되어있어서 그거에 맞춰서 작업을 한다 라고 이해해도 될까요 ? (...) 디자이너가 현존하는 휴대폰과 웹, 테블릿 기기에 맞게 디자인을 전부 내려주는 걸까요?

네 맞습니다 :) 프로덕트 전반에 사용할 브레이크포인트도 디자인 사이드에서 정해주신 값을 개발로 구현하고, 모든 지면의 반응형 디자인을 이에 기반하여 동작하도록 구현하고 있습니다.

휴대폰도 기종에 따라서 컨테이너가 넘치거나 부족한 경우가 있었어서 px을 임의로 설정해주었습니다!

디바이스 기종별로 수치를 설정하지는 않습니다. 예를 들면 desktop에서는 width: 480px, height: 56px 인 컴포넌트가, mobile에서는 width: 100%, hegith: 48px 정도로 구현하는 식이지요. :) 만약 특정 뷰포트에서 문제가 되는 경우가 있다면 논의를 통해 디자인 스펙을 변경할 수도 있습니다.

이것저것 질문해서 번거로우셨을텐데 상세한 답변 정말 감사드립니다 하루 ..! 🥲

brgndyy · 2024-04-18T08:04:13Z

src/components/composables/Label.tsx

- input과 label의 관계성

이번에 페어 덕분에 sr-only라는 걸 알게 되어서 공용 label 컴포넌트를 만들고 이를 감춰주었습니다!

근데 우리카드 홈페이지에선 따로 label속성은 없고 input 태그 자체에 title이라는 어트리뷰트로 첫번째 카드번호 섹션 이런식으로 값을 넣더라구요..!

(예시 사진을 찾아보려했는데, 못찾겠어서 설명만 드리네요 죄송합니다 🥲)

인풋태그를 사용할때, 접근성을 위해서 기본적으로 label을 같이 사용한다고 생각하면 되는 부분일까요 ?

input만 따로 사용할 때는 없을까요..!?

인풋태그를 사용할때, 접근성을 위해서 기본적으로 label을 같이 사용한다고 생각하면 되는 부분일까요 ?

네, 저는 적극 권장 드리는 방향입니다.

The main use of the title attribute is to label iframe elements for assistive technology.

말씀해주신 title도 주로 웹 접근성을 위한 목적으로 활용되는 어트리뷰트로 보여요. 제 환경에서 VoiceOver를 실행했을 때도 label을 지정했을 때와 동일하게 동작하는 것으로 확인했어요. (다른 환경에서는 동작이 다를 수 있습니다.)

<input title='타이틀'/> 및 label 타이틀 지정 시 VoiceOver 동작 스크린샷

그렇지만 MDN 문서에 따르면 권장하지 않는 방법이라고 합니다. iframe의 경우가 아니라면 title이 아니라 label을 활용해주시면 더 좋을 것 같습니다.

While title can be used to provide a programmatically associated label for an element, this is not good practice. Use a instead.

참고: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/title

헉 링크까지 상세한 답변 감사합니다!

(생각해보니 공식문서 검색을 놓쳤었네요. 여쭤보기 전에 충분히 페어와 결정할수 있었을거 같은데 죄송해요 🥲)

365kim · 2024-04-18T08:52:01Z

@brgndyy 버건디 안녕하세요 :)
혹시 스토리북 배포 링크 말고, 앱 배포 링크를 제가 못찾고 있는데 PR 본문 상단에 업데이트 부탁드려도 될까요?

365kim · 2024-04-18T17:02:06Z

package.json

@@ -13,7 +13,9 @@
  },
  "dependencies": {
    "react": "^18.2.0",
-    "react-dom": "^18.2.0"
+    "react-dom": "^18.2.0",
+    "styled-components": "^6.1.8",


스타일링에는 CSS Module, styled-components, emotion 중 한 가지를 선택하여 사용한다.

여러 후보 중 버건디가 styled-components 를 선택해주신 이유가 궁금합니다 :)

일단 저 같은 경우는 styled-components를 사용해본적이 없습니다!

지금까지 css modules나 vanilla extract를 사용해왔는데요!

배우는 과정에서 단순히 익숙하지 않다는 이유로 새로운 스택을 시도해보지 않는건 회피 밖에 안되겠다.라고 생각이 들었어요!

만약 얼른 배포를 해야하는 상황이고, 여러가지 현실적인 문제가 있다면 익숙한 기술을 채택해야겠지만..

이번에는 페어와 함께하다보니 페어를 믿고 진행해보았습니다!

페어 같은 경우는 styled-components를 사용해본 경험이 있다고 해서, 이번에 미션 진행해보면서 배울겸, 그리고 한번 겪어볼겸 시도해보았습니다!

근데 이번에 이 styled-components를 사용하면서, 장점보다는 단점이 더 많다고 저는 느껴졌어요.

1. 컴포넌트 레이아웃과 스타일링이 한곳에서 작성 된다.

이부분 같은 경우 따로 스타일 파일로 분리해서 관리를 해줄수도 있겠지만, 결국 스타일링을 적용한 컴포넌트 그 자체로 사용을 해주어야 하다보니 그냥 한 컴포넌트에서 작성하는 것이 최선이다라고 판단했습니다.

근데 전 스타일과 레이아웃이 아예 분리되는 걸 더 선호하는것 같아요.. 🥲 (css 파일로 따로 만들어서 className으로 주입해주는 것)

2. 높은 재사용성을 보장받을 수 없고, 가독성이 떨어진다.

이는 익숙하지 않아서 그런거일수도 있지만, 스타일 재사용성이 그렇게 높지 않다고 느꼈습니다.

가령 한 2~3개의 동일한 속성을 가진 스타일링 컴포넌트가 있다고 가정하고 이를 재사용해주고 싶다고 한다면,

재사용을 위해 스타일 컴포넌트 명이 애매모호 해질수 밖에 없겠다고 생각했습니다.

결국 명확한 컴포넌트 명을 위해 동일한 속성이더라도, 각각의 스타일 컴포넌트로 만들어주어야하지 않나? 라는 의구심이 들었어요!

또 결국 UI 구분을 위한 컴포넌트를 만드는데, 스타일링을 위한 컴포넌트를 또 만든다는것 자체가 아직 와닿지는 않네요..!

3. 런타임시 스타일 적용

styled-component 같은 경우 런타임시에 css를 만들어서 적용시켜주는 거로 알고 있는데요..!

이럴때 무거운 애니메이션 같은 작업이 있을 경우 성능 문제가 발생할수 있다고 알고 있습니다.

그래서 이러한 문제점을 해결 하기 위해서 나온것이 제로 런타임인 vanilla-extract나 panda-css로 알고 있어요!

그렇다면 자연스럽게 저런 제로 런타임 스타일링 기술들로 생태계 흐름이 변화하지 않을까? 라는 생각이 들어요!

하지만 이번에 페어 덕분에 그래도 수월하게 새로운 기술을 접해보았다는 것이 좋았던 점입니다.

하루의 styled-components에 대한 의견 한번 여쭤보고 싶어요!

컴포넌트 레이아웃과 스타일링이 한곳에서 작성 된다.

이것은 이번에 스타일 파일을 분리해주셔서 해결이 된 것 같군요 :)

높은 재사용성을 보장받을 수 없고, 가독성이 떨어진다.

FlexCenter Caption과 같은 유틸 성격의 컴포넌트를 작성해두고 재사용해볼 수는 있을 것 같아요.

런타임시 스타일 적용

저도 버건디의 의견에 동의합니다. panda-css 는 저도 처음들어보는데 엄청 귀여운 이름이네요 🐼 사실 저는 CSS-in-JS를 사용했을 경우 DX가 더 좋았다고 느꼈어요. 그렇지만 앞으로 새로운 프로젝트를 한다면 비슷한 이유로 vanilla-extract 나 sass 를 선택할 것 같습니다 :)

365kim · 2024-04-18T17:03:48Z

src/components/composables/Input.tsx

@@ -0,0 +1,23 @@
+import styled from 'styled-components';
+
+type InputProps = {


React.InputHTMLAttributes<HTMLInputElement>와 같이 제공되는 타입을 활용해서 타이핑하면 어떨까요? 혹은 의도해주신 걸까요?

오호 아뇨 생각해보지 못한 부분입니다!

짚어주셔서 감사합니다 하루 🙇

한번 적용시켜보겠습니다~!

365kim · 2024-04-18T17:04:13Z

src/components/composables/Input.tsx

+  id: string;
+};
+
+const StyledInput = styled.input`


Styled-*를 컨벤션으로 잡아주신거군용~!

앗 처음에 아마 이렇게 작성하고 컨벤션을 놓친 컴포넌트들이 많을거 같은데요..!

한번 검토해보면서 수정해보겠습니다!

짚어주셔서 감사해요 하루~!

365kim · 2024-04-18T18:03:09Z

src/stories/CreditCard.stories.tsx

+  ],
+  argTypes: {
+    cardNumbers: {
+      control: 'text',


cardNumbers의 control 타입은 'text'가 아닐 것 같아요. 실제로 조작해보니 에러가 나고 있는데 픽스해볼 수 있을까요?

Screen.Recording.2024-04-19.at.3.02.52.AM.mov

사실 지금 control 안의 속성 값을 정확히 언제 어떤걸 써야할지 감이 잘 안잡히는 상황이어서요..! (스토리북이 익숙치 않다보니 공식문서를 봐도 감이 딱 안오네요 🥲)

한번 더 공부해보고 알맞은 타입으로 수정하겠습니다! 짚어주셔서 감사합니다!

365kim · 2024-04-18T18:03:59Z

src/stories/CreditCard.stories.tsx

+      description: '카드 소유자 입력 값',
+    },
+    cardImageSrc: {
+      control: [MasterCard, VisaCard],


[MasterCard, VisaCard] 이게 control에 지정할 수 있는 값이 맞나용?

이 또한 한번 더 공부해보고 알맞은 값으로 바꿔보겠습니다!

짚어주셔서 감사해요~!

365kim · 2024-04-18T18:31:41Z

src/App.tsx

+    if (
+      month !== '' &&
+      (!validate.isNumberInRange({ min: 1, max: 12, compareNumber: Number(month) }) ||
+        !validate.isValidDigit(month))
+    ) {


if 안에 조건식이 길어보이는데 가독성을 위해 로컬 변수로 분리해보면 어떨까용?

맞는 말씀입니다!

한번 수정해보도록 하겠습니다.

짚어주셔서 감사해요 🙇

오 이 부분은 아예 인터페이스가 변경되었군용

365kim · 2024-04-18T18:32:09Z

src/constants/cardSection.ts

+const NUMBER_ERROR_MESSAGE = '숫자만 입력 가능합니다.';
+
+export const CARD_NUMBER = Object.freeze({
+  title: '결제할 카드 번호를 입력해 주세요',
+  description: '본인 명의의 카드만 결제 가능합니다.',
+  inputTitle: '카드 번호',
+  errorMessage: NUMBER_ERROR_MESSAGE,
+});
+
+export const EXPIRATION_PERIOD = Object.freeze({
+  title: '카드 유효기간을 입력해 주세요',
+  description: '월/년도(MMYY)를 순서대로 입력해 주세요.',
+  inputTitle: '유효기간',
+  monthErrorMessage: '1부터 12사이의 숫자만 입력 가능합니다.',
+  yearErrorMessage: NUMBER_ERROR_MESSAGE,
+});
+
+export const OWNER_NAME = Object.freeze({
+  title: '카드 소유자 이름을 입력해주세요',
+  inputTitle: '소유자 이름',
+  errorMessage: '영문 대문자만 입력 가능합니다.',
+});


365kim · 2024-04-18T18:32:36Z

src/App.tsx

+  useEffect(() => {
+    if (
+      month !== '' &&
+      (!validate.isNumberInRange({ min: 1, max: 12, compareNumber: Number(month) }) ||


1, 12 같은 매직넘버도 상수화해서 관리해도 좋겠습니당

에고 놓친 부분입니다..🥲

짚어주셔서 감사해요~! 수정하겠습니다.

365kim · 2024-04-18T18:36:14Z

src/App.tsx

+                  <>
+                    <Label htmlFor={uniqueId} />
+                    <Input
+                      key={uniqueId}
+                      id={uniqueId}
+                      placeholder="1234"
+                      type="text"
+                      maxLength={4}
+                      value={cardNumber.value}
+                      onChange={(e) => cardNumbersChangeHandler(e, index)}
+                      isError={cardNumber.isError}
+                    />
+                  </>


key는 맵 대상에 지정해야할 것 같습니다. key={'cardNumbers' + index} 와 key={index}에 어떤 차이가 있을까요?

Suggested change

<>

<Label htmlFor={uniqueId} />

<Input

key={uniqueId}

id={uniqueId}

placeholder="1234"

type="text"

maxLength={4}

value={cardNumber.value}

onChange={(e) => cardNumbersChangeHandler(e, index)}

isError={cardNumber.isError}

/>

</>

<React.Fragment key={index}>

<Label htmlFor={uniqueId} />

<Input

id={uniqueId}

placeholder="1234"

type="text"

maxLength={4}

value={cardNumber.value}

onChange={(e) => cardNumbersChangeHandler(e, index)}

isError={cardNumber.isError}

/>

</>

아하 맞는 말씀입니다..!

짚어주셔서 감사합니다 🙇

저 key값이 리액트 render phase 과정에서 업데이트를 시켜야할지 말지를 판단하는 값중에 하나라고 알고 있는데요..!

그래서 만약 단순히 index값으로 해주는 경우에, map함수로 뿌려준 요소들 중에서 삭제나 추가 같은 이벤트가 발생하면 index 들이 전부 수정이 되어버리기때문에 전부 리렌더링이 일어나는거로 알고 있습니다!

혹시 이러한 부분에 대해서 질문 주신것이 맞으실까요 ?

혹시나 잘못 생각하고 있거나, 추가로 말씀 주실 부분이 있다면 짚어주시면 감사하겠습니다!

버건디가 이해하신 의도로 질문드렸던 것이 맞습니다 :)

제가 덧붙여 설명 드려보자면, index를 key로 그대로 사용하는 것이 언제나 틀린 것이 아닙니다.

key의 사용목적은 key'는 해당 항목의 고유성을 보장해줘서, 리액트가 항목의 추가, 삭제 또는 변경하기 위해 해당 항목을 식별하기 위함입니다.

따라서, 리스트에 변경사항이 없다면 index를 그대로 사용해도 무방하지요.

또한 key는 sibling끼리만 고유하면 되므로, 여기 코드에서 key={'cardNumbers' + index} 와 key={index}에는 실질적인 차이가 없어 보입니다. (그래서 질문을 드리게 되었어요.)

단순히 index값으로 해주는 경우에, 삭제나 추가 같은 이벤트가 발생하면 index 들이 전부 수정이 되어버리기때문에 전부 리렌더링이 일어나는거로 알고 있습니다!

리렌더링이 전부 (의도한 대로) 일어나면 다행인데, 그렇지 않은 문제가 발생할 수 있어 주의해야합니다. 더 자세한 내용은 공식문서를 참고해주시면 좋겠습니다 👍

참고: https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key

또한 key는 sibling끼리만 고유하면 되므로, 여기 코드에서 key={'cardNumbers' + index} 와 key={index}에는 실질적인 차이가 없어 보입니다. (그래서 질문을 드리게 되었어요.)

헉 맞네요..🥲 step2때 반영하도록 하겠습니다! 짚어주셔서 감사해요 하루~!

365kim · 2024-04-18T18:45:24Z

src/components/CreditCard.tsx

+            <IcChip />
+          </CardHeaderContentWrapper>
+          <CardHeaderContentWrapper>
+            {cardImageSrc ? <CardBrand src={cardImageSrc} /> : null}


img 태그(CardBrand)에 alt 속성이 누락되어있네요 👀

헉 짚어주셔서 감사합니다! 반영하도록 하겠습니다 🙇

스크린리더로 실행한 모습입니다

혹시 보조기기 사용자에게 'cardBrandImage' 문자열 라고 전달하기보다 '비자카드', '마스터카드' 라고 전달하는 것은 어떨까요?

혹은 장식적 요소로 판단되신다면 아예 alt값을 빈문자열('')로 지정하는 방법도 있을 것 같습니다 :)

반영하도록 하겠습니다 짚어주셔서 감사합니다!

365kim

안녕하세요, 버건디! 또뵙네요 :)
작업해주신 코드 잘봤습니다. 👍

전반적으로 리액트 페이먼츠 미션의 기능 요구사항과 프로그래밍 요구사항을 충족하도록 잘 작성해주신 것으로 확인했어요. 💯

작업 내역을 읽기 좋은 커밋 단위로 잘 올려주셔서 리뷰하기 너무 수월했습니다. 컴포넌트 구조도 덕분에 재사용성을 고려해주신 흐름 따라가기도 좋았어요 감사합니다. 좋은 코드와 협업 방식에 대한 고민이 담긴 다양한 질문 남겨주셔서 저도 같이 즐겁게 고민해볼 수 있는 시간이었습니다.

코멘트 몇 가지 남겼는데, 천천히 보시고 확인 완료되시면 리뷰 재요청 부탁드릴게요~!
고생하셨습니다 👏👏👏

365kim · 2024-04-21T02:40:14Z

src/stories/CreditCard.stories.tsx

+    cardBrand: {
+      control: 'select',
+      options: ['none', 'MasterCard', 'Visa'],
+      description: '카드 브랜드 이미지',
+    },


오우 저보다 더 잘쓰시네용 😎👍

365kim · 2024-04-21T02:43:05Z

src/hooks/useCardBrandImage.ts

+import VisaCardImage from '../assets/images/visa.png';
+import { InitialCardNumberState } from '../App';
+
+const useCardBrandImage = (cardNumberStates: InitialCardNumberState[]) => {


커스텀훅을 잘 활용해서 관심사를 적절하게 분리해주신 것 같아 넘 좋습니당 👍

365kim · 2024-04-21T02:45:47Z

src/hooks/useCardBrandImage.ts

+    if (validate.isVisa(cardNumberString)) {
+      setCardImageSrc(VisaCardImage);
+    }
+
+    if (validate.isMasterCard(cardNumberString)) {
+      setCardImageSrc(MasterCardImage);
+    }


마스터카드 부분에서 else if로 전개되어도 좋을 것 같아요. 첫번째 if문제 해당하면 두번째 if문을 도달하지 않는다는 것을 코드 블록의 모양만으로도 명시적으로 보여주려는 목적입니다. 하지만 취향의 영역 또는 팀 내 컨벤션 정도의 영역으로 볼 수 있을 것 같아 지금도 좋습니다 :)

if (isVisaCard) { set VisaCard } else if (isMasterCard) { set MasterCard }

에고 프리코스 때부터 else는 지양한다. 라는 문구로 인해서 관성적으로 else를 안썼네요.. 🥲

짚어주셔서 감사합니다~!

365kim · 2024-04-21T03:10:09Z

.storybook/preview.tsx

+  },
+};
+
+export const decorators = [


365kim · 2024-04-21T03:19:20Z

src/hooks/useInput.ts

@@ -1,14 +1,24 @@
 import { useState } from 'react';

-const useInput = () => {
-  const [inputState, setInputState] = useState('');
+const useInput = (validators: { fn: (value: string) => boolean }[]) => {


함수 시그니처만 분리해서 type alias로 표현해보면 어떨까요?

type ValidatorFn = (value: string) => boolean const useInput = (validators: { fn: ValidatorFn }[]) => {

인터페이스를 아래처럼 바로 배열을 받지 않고 한겹더 설정해주신 이유가 궁금해졌어요, fn 말고 다른 프로퍼티도 추가해주실 예정일까용?

const useInput = (validators: ValidatorFn[]) => {

함수 시그니처만 분리해서 type alias로 표현해보면 어떨까요?

엇 맞는 말씀이에요 ! 짚어주셔서 감사합니다

인터페이스를 아래처럼 바로 배열을 받지 않고 한겹더 설정해주신 이유가 궁금해졌어요, fn 말고 다른 프로퍼티도 추가해주실 예정일까용?

다른 프로퍼티를 추가할 예정은 아니었는데, 혹시 유효성 검사 외에 다른 함수들이 추가 될까 싶어서 해보았습니다..!

2024/04/21 머지 받고 step2 피그마 시안을 살펴보는 현재,

입력값에 따른 분기처리를 해줄때 함수를 하나 더 인자로 넣어줄거 같습니다..!

혹시 제가 잘못 짚은건 없을지 step2 PR때 다시 한번 살펴봐주시면 정말 감사하겠습니다!

365kim

버건디, 페이먼츠 1단계 미션 완료 축하드립니다 🥳

제가 드린 피드백 하나하나 진지하게 고민하고 또 반영해주셔서 감사합니다 :) 배포 상태도 최신으로 잘 관리해주셔서 편하게 리뷰할 수 있었습니다. ("재배포 완료" 센스 💯)

리액트로 넘어와서 의견 나누니까 더 좋네요 😄
2단계 작업도 기대하고 있을게요 고생하셨습니다 👏
(p.s. 서비스 배포 링크도 함께 만나용)

Largopie and others added 25 commits April 16, 2024 16:22

style: prettier 설정

eac1107

Co-authored-by: Placeholder Author <placeholder@author.com>

docs: 기능 요구사항 작성

a2eaf72

Co-authored-by: brgndyy <brgndyy@gmail.com>

chore: styled-components, styled-reset 설치

bcd9c7d

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: 공용 인풋 컴포넌트 생성

cded571

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: sr-only 라벨 컴포넌트 생성

6dcf8e2

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: 글로벌 스타일 설정

b5f8195

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat(storybook): 인풋 공용 컴포넌트 스토리북 작성

7173e07

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: 인풋 섹션 컴포넌트 생성

4552764

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: useCardNumber 커스텀 훅 생성

2ea7df5

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: useInput 커스텀 훅 생성

3b17e49

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat(validate): 숫자 범위 유효성 검사 메서드 구현

7f0e7d1

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: CreditCard 컴포넌트 구현

152e9be

refactor(InputSection): description 분기 처리

f3b9ee0

docs: 카드 브랜드 조건 추가

ac06ece

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat(validate): 유효성 검사 추가

17fabd6

- 숫자 유효성 검증 - 비자 카드 유효성 검증 - 마스터 카드 유효성 검증 - 영어 유효성 검증 Co-authored-by: brgndyy <brgndyy@gmail.com>

refactor: useInput 로직 수정

3f86c2a

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: useCardNumber 유효성 검사 로직 추가

4c06f2f

Co-authored-by: brgndyy <brgndyy@gmail.com>

refactor: Input 컴포넌트 로직 수정

b323507

Co-authored-by: brgndyy <brgndyy@gmail.com>

chore: cardNumbers 타입 수정

11aeaf6

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat(App): 컴포넌트 및 상태 바인딩

2185961

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: 상수 분리

8bd4982

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: 인풋 컴포넌트 관련 스토리북 작성

c843b8a

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: CreditCard 관련 스토리북 작성

76a8058

Co-authored-by: brgndyy <brgndyy@gmail.com>

feat: InputSection 관련 스토리북 작성

fecb622

Co-authored-by: brgndyy <brgndyy@gmail.com>

chore: App 컴포넌트 스토리북 중앙 정렬

a9f5148

Co-authored-by: brgndyy <brgndyy@gmail.com>

brgndyy commented Apr 18, 2024

View reviewed changes

365kim reviewed Apr 18, 2024

View reviewed changes

365kim requested changes Apr 18, 2024

View reviewed changes

brgndyy added 15 commits April 19, 2024 14:30

feat: useCardImageBrand hook 선언

f60738d

refactor: 공용 컴포넌트 props 타입 변경

10a9308

refactor: useInput 커스텀훅 수정

420baad

refactor: InitialCardNumberState 타입 하위에서 정의로 변경

c1d5ea5

refactor: 변경된 useInput에 맞게 useEffect 선언들 삭제

208b14d

refactor: InputSection 컨테이너 스타일 수정

3f03f3a

refactor: 스토리북 layout centered 속성 전역 설정

422de20

refactor: 스토리북 전역 스타일로 설정

1c7a8f9

chore: .nvmrc 및 eslint 설정

394d204

fix: useInput 유효성검사 아예 못받게 설정, App import 변경

456e349

fix: CreditCard 스토리북 옵션 수정

edf45ea

refactor: 카드 브랜드 이미지 상태 관리 속성값 변경

a1ff303

refactor: 스타일 파일 분리

10f0b0c

refactor: useCardNumber 핸들러 함수 내 중복 제거

f9ae286

refactor: 카드 브랜드 이미지 로직 useCardNumber로 이동

b63c30a

365kim reviewed Apr 21, 2024

View reviewed changes

365kim approved these changes Apr 21, 2024

View reviewed changes

365kim merged commit 8d3d447 into woowacourse:brgndyy Apr 21, 2024

		@@ -0,0 +1,23 @@
		import styled from 'styled-components';

		type InputProps = {

[1단계 - 페이먼츠] - 버건디(전태헌) 미션 제출합니다. #343

[1단계 - 페이먼츠] - 버건디(전태헌) 미션 제출합니다. #343

Conversation

brgndyy commented Apr 18, 2024 • edited Loading

- 컴포넌트 분리 사진

brgndyy Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

- useEffect에서의 유효성 검사, 어떻게 생각하실까요 ?

365kim Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brgndyy Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

- 스토리북의 유용함, 스토리북의 책임

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brgndyy Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

- png와 svg

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brgndyy Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

컴포넌트의 재사용성에 대한 판단, 그리고 컴포넌트는 어디까지 분리하는게 맞을까요 ?

- Component.tsx

Choose a reason for hiding this comment

brgndyy Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

피그마에서의 px

- 예시

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brgndyy Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

- input과 label의 관계성

Choose a reason for hiding this comment

Choose a reason for hiding this comment

365kim commented Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1. 컴포넌트 레이아웃과 스타일링이 한곳에서 작성 된다.

2. 높은 재사용성을 보장받을 수 없고, 가독성이 떨어진다.

3. 런타임시 스타일 적용

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brgndyy Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

365kim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

365kim Apr 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brgndyy commented Apr 18, 2024 •

edited

Loading

brgndyy Apr 18, 2024 •

edited

Loading

365kim Apr 18, 2024 •

edited

Loading

brgndyy Apr 18, 2024 •

edited

Loading

brgndyy Apr 18, 2024 •

edited

Loading

brgndyy Apr 18, 2024 •

edited

Loading

brgndyy Apr 18, 2024 •

edited

Loading

brgndyy Apr 18, 2024 •

edited

Loading

365kim commented Apr 18, 2024 •

edited

Loading

brgndyy Apr 19, 2024 •

edited

Loading

365kim Apr 21, 2024 •

edited

Loading

brgndyy Apr 21, 2024 •

edited

Loading