-
Notifications
You must be signed in to change notification settings - Fork 165
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단계 - 페이먼츠] - 버건디(전태헌) 미션 제출합니다. #343
Changes from all commits
eac1107
a2eaf72
bcd9c7d
cded571
6dcf8e2
b5f8195
7173e07
4552764
2ea7df5
3b17e49
7f0e7d1
152e9be
f3b9ee0
ac06ece
17fabd6
3f86c2a
4c06f2f
b323507
11aeaf6
2185961
8bd4982
c843b8a
76a8058
fecb622
a9f5148
f60738d
10a9308
420baad
c1d5ea5
208b14d
3f03f3a
422de20
1c7a8f9
394d204
456e349
edf45ea
a1ff303
10f0b0c
f9ae286
b63c30a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
v20.0.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"singleQuote": true, | ||
"semi": true, | ||
"useTabs": false, | ||
"tabWidth": 2, | ||
"trailingComma": "all", | ||
"printWidth": 100, | ||
"bracketSpacing": true, | ||
"arrowParens": "always", | ||
"endOfLine": "auto" | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import type { Preview } from '@storybook/react'; | ||
import GlobalStyles from '../src/GlobalStyles'; | ||
import React from 'react'; | ||
|
||
const preview: Preview = { | ||
parameters: { | ||
controls: { | ||
matchers: { | ||
color: /(background|color)$/i, | ||
date: /Date$/i, | ||
}, | ||
}, | ||
layout: 'centered', | ||
}, | ||
}; | ||
|
||
export const decorators = [ | ||
(Story) => ( | ||
<> | ||
<GlobalStyles /> | ||
<Story /> | ||
</> | ||
), | ||
]; | ||
|
||
export default preview; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,27 @@ | ||
# react-payments | ||
# 💳 페이먼츠 | ||
|
||
페이먼츠 사용자의 카드 정보를 입력하고 해당 카드를 어플리케이션에 등록한다. | ||
|
||
### 카드 번호 입력 및 식별 | ||
|
||
- 사용자가 입력하는 카드 번호를 실시간으로 파악하여, Visa나 MasterCard에 해당하면 해당 브랜드의 로고를 UI에 표시한다. | ||
- Visa : 4로 시작하는 16자리 | ||
- MasterCard: 51~55로 시작하는 16자리 숫자 | ||
- 입력은 숫자만 가능하며, 유효하지 않은 번호 입력 시 피드백을 제공한다. | ||
|
||
### 카드 유효기간 입력 | ||
|
||
- 월과 년도를 범위 내에서만 입력할 수 있어야 하며, 입력 제한을 두어 사용자가 숫자만 입력할 수 있도록 한다. | ||
|
||
### 카드 소유자 이름 입력 | ||
|
||
- 영문자 대문자만 입력 가능한 폼을 구현한다. | ||
|
||
### 실시간 프리뷰 업데이트 | ||
|
||
- 사용자의 카드 정보 입력에 따라 카드 프리뷰를 동시에 업데이트한다. | ||
|
||
### ⚠️ 주의 사항 | ||
|
||
- 사용자의 입력 input에 포커스를 자동으로 이동하는 부분을 1단계에서 고려하지 않는다. **'기능' 자체에 집중해서 구현**한다. 만약 리뷰어가 사용자 경험 측면에서 피드백을 준다면, 피드백을 반영하는 시점에서 사용자 경험도 함께 고려하여 개선한다. | ||
- 기본적인 기능 요구사항을 만족하지 못한 코드는 코드 리뷰 대상이 아니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,11 +9,14 @@ | |
"lint": "eslint . --ext ts,tsx --report-unused-disable-directives --max-warnings 0", | ||
"preview": "vite preview", | ||
"storybook": "storybook dev -p 6006", | ||
"build-storybook": "storybook build" | ||
"build-storybook": "storybook build", | ||
"chromatic": "npx chromatic --project-token=chpt_a2a6e1e59fdce40" | ||
}, | ||
"dependencies": { | ||
"react": "^18.2.0", | ||
"react-dom": "^18.2.0" | ||
"react-dom": "^18.2.0", | ||
"styled-components": "^6.1.8", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
여러 후보 중 버건디가 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 일단 저 같은 경우는 지금까지
만약 얼른 배포를 해야하는 상황이고, 여러가지 현실적인 문제가 있다면 익숙한 기술을 채택해야겠지만.. 이번에는 페어와 함께하다보니 페어를 믿고 진행해보았습니다! 페어 같은 경우는 근데 이번에 이 1. 컴포넌트 레이아웃과 스타일링이 한곳에서 작성 된다.이부분 같은 경우 따로 스타일 파일로 분리해서 관리를 해줄수도 있겠지만, 결국 근데 전 스타일과 레이아웃이 아예 분리되는 걸 더 선호하는것 같아요.. 🥲 (css 파일로 따로 만들어서 className으로 주입해주는 것) 2. 높은 재사용성을 보장받을 수 없고, 가독성이 떨어진다.이는 익숙하지 않아서 그런거일수도 있지만, 스타일 재사용성이 그렇게 높지 않다고 느꼈습니다. 가령 한 2~3개의 동일한 속성을 가진 스타일링 컴포넌트가 있다고 가정하고 이를 재사용해주고 싶다고 한다면, 재사용을 위해 스타일 컴포넌트 명이 애매모호 해질수 밖에 없겠다고 생각했습니다. 결국 명확한 컴포넌트 명을 위해 동일한 속성이더라도, 각각의 스타일 컴포넌트로 만들어주어야하지 않나? 라는 의구심이 들었어요! 또 결국 UI 구분을 위한 컴포넌트를 만드는데, 스타일링을 위한 컴포넌트를 또 만든다는것 자체가 아직 와닿지는 않네요..! 3. 런타임시 스타일 적용
이럴때 무거운 애니메이션 같은 작업이 있을 경우 성능 문제가 발생할수 있다고 알고 있습니다. 그래서 이러한 문제점을 해결 하기 위해서 나온것이 제로 런타임인 그렇다면 자연스럽게 저런 제로 런타임 스타일링 기술들로 생태계 흐름이 변화하지 않을까? 라는 생각이 들어요! 하지만 이번에 페어 덕분에 그래도 수월하게 새로운 기술을 접해보았다는 것이 좋았던 점입니다. 하루의 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
이것은 이번에 스타일 파일을 분리해주셔서 해결이 된 것 같군요 :)
저도 버건디의 의견에 동의합니다. panda-css 는 저도 처음들어보는데 엄청 귀여운 이름이네요 🐼 사실 저는 |
||
"styled-reset": "^4.5.2" | ||
}, | ||
"devDependencies": { | ||
"@chromatic-com/storybook": "^1.3.3", | ||
|
@@ -30,10 +33,16 @@ | |
"@typescript-eslint/eslint-plugin": "^7.2.0", | ||
"@typescript-eslint/parser": "^7.2.0", | ||
"@vitejs/plugin-react-swc": "^3.5.0", | ||
"chromatic": "^11.3.0", | ||
"eslint": "^8.57.0", | ||
"eslint-config-prettier": "^9.1.0", | ||
"eslint-plugin-import": "^2.29.1", | ||
"eslint-plugin-jsx-a11y": "^6.8.0", | ||
"eslint-plugin-react": "^7.34.1", | ||
"eslint-plugin-react-hooks": "^4.6.0", | ||
"eslint-plugin-react-refresh": "^0.4.6", | ||
"eslint-plugin-storybook": "^0.8.0", | ||
"prettier": "^3.2.5", | ||
"storybook": "^8.0.8", | ||
"typescript": "^5.2.2", | ||
"vite": "^5.2.0" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. - useEffect에서의 유효성 검사, 어떻게 생각하실까요 ?이번에
이렇게 3가지를 각각의 상태로 관리해주었습니다. 원래 이 만약 분기별로 나타내야할 에러 메세지가 많다면 ? 이라는 생각도 동시에 들었어요! 예를 들어 사용자 이름에 대해 에러메세지를 갖는다고 가정했을때
이렇게만 되더라도 동적으로 그래서 이렇게 하면
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
App.tsx 내에 useEffect가 많아지는 것에 대해 구체적으로 어떤 점이 우려가 되셨을까요? 우려되는 점이 있다면 App과 useInput 사이에 중간 레이어를 두는 방법도 있을 거 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
함수를 prop으로 받으면 말씀해주신 값에 따른 분기 처리도 가능해질까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 더 중요하게는 언제 effect를 사용할지, 언제 event handling만으로 충분한지도 살펴보시면 좋겠습니다
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
�겉보기에도
원래 말씀해주신 방식대로 구현을 하려했었는데요. const {
inputState: name,
inputChangeHandler: nameChangeHandler,
} = useInput([{fn : 유효성검사1 함수, message : "유효성 검사1에 맞는 에러메세지"}, {fn : 유효성검사2 함수, message : "유효성 검사2에 맞는 에러메세지"}]); 이런식의 그림을 생각했었는데, 안에서 로직이 좀 복잡해지지 않나? 근데 지금 하루가 보내주신 공식문서 글을 읽어보고, 계속 생각해보고 있는데요! setState를 통해 상태 변화를 하고, 그 후에 또 useEffect로 에러 감지를 해주어서 렌더링을 한번 더해준다는것 자체가 더 비효율적이라는 생각이 드네요..! 아예 안에 로직이 복잡해지더라도, setState 내부에서 유효성 검사를 해주는것이 더 맞겠다라는 생각이 듭니다! 짚어주셔서 감사합니다 🙇 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아하, 그런 고민의 과정이 있으셨군요 👍 useEffect를 필요한 만큼 활용하는 것은 문제되지않으나, 너무 많아지면 말씀해주신 대로 디버깅이 어려워질 것 같아요. (서로 다른 관심사의 useEffect를 단지 useEffect 수를 줄이기 위해 하나의 useEffect로 합치는 것까지는 지양해야겠지만요)
동의합니다. 이벤트 핸들링 동작의 일부를 useEffect로 넣게되면, 코드 흐름을 따라가기 어려워지고 따라서 디버깅 난이도가 더 올라간다고 생각해요 :) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import { createGlobalStyle } from 'styled-components'; | ||
import reset from 'styled-reset'; | ||
|
||
const GlobalStyles = createGlobalStyle` | ||
${reset} | ||
|
||
a{ | ||
text-decoration: none; | ||
color: inherit; | ||
} | ||
|
||
*{ | ||
box-sizing: border-box; | ||
} | ||
Comment on lines
+7
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 자동 포매팅 적용이 안되고 있는 것 같아요 설정 확인해주시면 감사하겠습니다 :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아이고 확인해보겠습니다..! 짚어주셔서 감사합니다! |
||
|
||
#root { | ||
display: flex; | ||
justify-content: center; | ||
align-items: center; | ||
|
||
width: 100vw; | ||
height: auto; | ||
min-height: 100vh; | ||
} | ||
|
||
input, textarea { | ||
-moz-user-select: auto; | ||
-webkit-user-select: auto; | ||
-ms-user-select: auto; | ||
user-select: auto; | ||
} | ||
|
||
input:focus { | ||
outline: none; | ||
} | ||
|
||
button { | ||
border: none; | ||
background: none; | ||
padding: 0; | ||
cursor: pointer; | ||
} | ||
`; | ||
|
||
export default GlobalStyles; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍