Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1단계 - 페이먼츠] 병민(윤병인) 미션 제출합니다. #87

Merged
merged 78 commits into from
May 6, 2022
Merged

[1단계 - 페이먼츠] 병민(윤병인) 미션 제출합니다. #87

merged 78 commits into from
May 6, 2022

Conversation

airman5573
Copy link

@airman5573 airman5573 commented Apr 28, 2022

데모페이지

안녕하세요 노스님! 이번 리뷰 잘 부탁드립니다 :D

Tech Stack

  • CRA
  • TS
  • Emotion, SCSS

어려웠던점 및 질문

  • 저희는 카드 번호입력 필드를 4개가 아닌 하나로 만들었습니다. 처음에는 이 방법이 더 편할거라 생각했지만 중간에서 입력하고 지우는 경우와 4글자 마다 하이픈을 넣어주는게 어려웠습니다. 그래도 다행히 input의 selectionStart속성으로 커서의 위치를 알 수 있었고, input의 setSelectionRange로 커서의 위치를 컨트롤 할 수 있었습니다.
  • CRA + Typesctip + Emotion + Storybook 이렇게 같이 쓸때 Emotion의 css props를 쓰기 어려웠습니다. @emotion/babel-preset-css-prop 이 바벨 플러그인을 사용 해야하는데 babel.config.js파일이 안보여서 애를 먹었습니다. 어찌저찌 craco를 사용해서 해당 플러그인을 추가해 주었으나 Storybook은 craco의 craco.config.js파일을 자동으로 인식해주지 않아서 결국 css props는 후반에 포기했습니다. 그런데 생각보다 styled.div이렇게 쓰는것도 괜찮았습니다. 2단계에서 이 부분을 조금 더 공부해 보겠습니다!
  • 저희는 Context API를 사용했는데, Storybook에서 Context API를 사용하는 컴포넌트를 표출하는게 어려웠습니다. 열심히 검색하서 decorators에 설정하는 방법을 찾았고 문제를 해결했습니다. 하지만 아직 다른 기본 옵션들에 대해서 모르는 부분이 많습니다. 이 부분도 2단계때 공부하겠습니다.
  • Atomic Design을 따라가는것이 어려웠습니다. 특히 Presentational Component와 Container Component둘다 Atomic인지 아니면 Container안에 Presentational이 있으니까 molecular인지 판단이 잘 서지 않았습니다. 우선 atoms에 폴더를 만들어서 둘다 같이 넣어 줬는데, 리뷰어님이 보시기에 이렇게 하는게 좋은 분리인지 알려주시면 감사하겠습니다!
  • Emotion을 오래전에 써보고 이번에 호기심에 다시 써보았습니다. JS안에서 CSS를 작성했기 때문에 둘이 같이 붙어다녀서 관리상에는 좋았으나, CSS코드를 작성하는건 조금 불편했습니다. 자동완성은 되지만 backgroundColor 이런식으로 작성해야 하고, 또 자식을 스타일링할때는 '& > .child' 이런식으로 작은 따움표를 매번 써주고 객체형태로 할당해야 하는것도 불편했습니다. 그래도 이름 충돌날 걱정이 없는 부분은 좋았습니다. 혹시 현업에서는 하나의 프로젝트에 SCSS와 Emotion(혹은 styled-component)를 같이 쓰나요? 저는 큰 레이아웃 잡을때는 한눈에 코드를 볼 수있어서 SCSS도 같이 쓰면 좋을것 같다는 생각이 들었습니다.
  • 저희는 이번에 비밀번호 입력 필드에만 Autofocus를 설정해 주었는데, 혹시 현업에서 input이 많은 페이지를 개발할때 autofocus를 많이 지원해 주시나요? 혹 자주 쓰시는 라이브러리가 있다면 추천 부탁드립니다!

1. cra with typescript
2. emotion
3. commitlint 추가
babel에 추가 설정 없이 emotion의 기능을 사용하려면
파일에 주석을 넣어줘야 하는데, 매 파일마다 이렇게 할 수 없어서
craco를 사용해 CRA의 바벨 설정을
override했다.
첫번째 필드를 입력하면 자동으로 두번째 입력 필드에 focus가 가도록 설정했다.
airman5573 added 26 commits May 1, 2022 03:59
babel의 다양한 플러그인을 사용하기 위함이다
CRA를 지우면서 build대신 dist를 사용하기로 했기 때문에 바꿔줘야한다
styled component를 아래로 내리고 main component에서 사용할때 lint에러가 나는걸 방지한다
정규식을 사용하니 처리가 단순해졌다
@airman5573
Copy link
Author

airman5573 commented May 3, 2022

안녕하세요 노스! 피드백 너무 감사드립니다.

  • CVC 코드 안보이게 만들기 : c7ed7b8
  • directory 구조를 기능 중심으로 변경 : aa048cb
  • 카드번호, 만료일 중간부터 수정시 제대로 반영 안되는 문제 해결 : 962b5e4 / b196eda
  • CRA없이 React + TS + Webpack + Babel + Stroybook + Emotion + SCSS 설정했습니다!

첨부해주신 링크 잘 읽었습니다. 캐시 메모리에 빗대어 어떤 방식으로 디렉토리 구조를 짜야하는지 설명하는 방식이 독특하고 또 글에서 말하는 취지에 많이 공감했습니다. 어떤 기능을 수정하기 위해서 접근하는 디렉토리에는 그 기능과 관련된 컴포넌트들을 같이 놓으면 우리 뇌가 덜 피곤해진다는걸 알게되었고 실제로 그렇게 될것 같습니다. 좋은글 공유해주셔서 감사합니다 :D

Head -> FieldsetHead , Body -> FieldsetContent
Copy link

@woowapark woowapark left a comment

Choose a reason for hiding this comment

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

병민 안녕하세요~! 마지막까지 꼼꼼히 피드백 반영 잘해주셨네요! 👏
추가 피드백이 있으시거나(cc. @hyoungnam) 추가로 반영하고 싶으신 부분이 있으시다면
2단계에서 이어서 의견 나눠주셔요 🙂
2단계 진행을 위해 요 PR은 머지하도록 할게요! 고생 많으셨습니다~

@woowapark woowapark merged commit 77b3cbd into woowacourse:airman5573 May 6, 2022
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