-
Notifications
You must be signed in to change notification settings - Fork 1
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
멀티 스텝 온보딩 폼 구현 #18
멀티 스텝 온보딩 폼 구현 #18
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
💅 ## storybook-URL - https://66a939e10fecfdf69816c066-zvuupapuyl.chromatic.com/ |
src/app/onboarding/view.tsx
Outdated
const [currentStep, setCurrentStep] = useState<number>( | ||
ONBOARDING.INITIAL_STEP, | ||
); |
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.
P4
INTIAL_STEP이 number로 추론 가능하기 때문에 제네릭 <number>
는 생략 가능할 거예요
여기 뿐 아니라 다른 곳에서도 유용해요~
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.
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.
고생하셨습니다~~
const getLabel = (step: number) => { | ||
switch (step) { | ||
case 1: | ||
return `다음 (${fields.categories?.length ?? 0}/2)`; | ||
case 2: | ||
return `다음 (${fields.moods?.length ?? 0}/3)`; | ||
case 4: | ||
return `다음 (${fields.priorities?.length ?? 0}/3)`; | ||
default: | ||
return `다음`; | ||
} | ||
}; |
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.
저는 개인적으로 switch 문을 좋아하지 않습니다. 이유는 로직에 비해 불필요하게 장황해지는 코드, 하나의 옵션을 추가한다면 불필요한 case return break문같은 보일러 플레이트가 많아서 가독성을 해칠 수가 있습니다.
사실 대부분의 클린코드 관련 책에서도 switch 문에 대한 좋지 않은 얘기도 많습니다.
- 클린코드 책 내용 중 - https://saseungmin.github.io/summary_of_technical_books/docs/clean/clean-code/chapter-3#-switch-%EB%AC%B8
- 리팩터링 2판 chapter03 코드에서 나는 악취 중
- 중복된 switch문이 문제가 되는 이유는 조건절을 하나 추가할 때마다 다른 switch문들도 모두 찾아서 함께 수정해야 하기 때문이다. 이럴 때 다형성은 반복된 switch문이 내뿜는 사악한 기운을 제압하여 코드베이스를 최신 스타일로 바꿔주는 세련된 무기인 셈이다.
저는 switch 문 대신 if문을 사용하거나, 비교하는 값이 동일한 경우에는 object로 코드를 개선합니다.
const getLabel = (step: number) => {
const label = {
// 숫자를 object 키로 사용할 때에는 주의 필요.
// obj[1]과 obj['1'] 동일 동작, 대괄호 표기법(obj[1]) 사용해야함.
// Object.keys()로 키를 가져올 때, 모든 키는 문자열로 반환.
1: `다음 (${fields.categories?.length ?? 0}/2)`,
2: `다음 (${fields.moods?.length ?? 0}/3)`,
4: `다음 (${fields.priorities?.length ?? 0}/3)`,
};
return label[step] || '다음';
};
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.
그리고 undefined 반환으로 0으로 바꿔주는 로직이 중복적으로 발생하는 것 같아요!
export const checkNumber = (value?: number | null): number => {
if (typeof value === 'number') {
return value;
}
return 0;
};
이런 유틸함수를 만들어주고
const getLabel = (step: number) => {
const label = {
// 숫자를 object 키로 사용할 때에는 주의 필요.
// obj[1]과 obj['1'] 동일 동작, 대괄호 표기법(obj[1]) 사용해야함.
// Object.keys()로 키를 가져올 때, 모든 키는 문자열로 반환.
1: `다음 (${checkNumber(fields.categories?.length)}/2)`,
2: `다음 (${checkNumber(fields.moods?.length)}/3)`,
4: `다음 (${checkNumber(fields.priorities?.length)}/3)`,
};
return label[step] || '다음';
}
중복되는 분기처리를 개선해볼 수 있을 것 같아요!
💅 ## storybook-URL - https://66a939e10fecfdf69816c066-fwbemaolvb.chromatic.com/ |
@seokzin 리뷰 감사드립니다 ㅠ ㅠ 말씀해주신것들은 반영했구요. 클린코드적으로 다시 한번 생각해보게되는... 일단 머지하겠읍니다 🙇♂️ |
PR 타입 ( 하나 이상의 PR 타입을 선택해주세요 )
개요
저희 온보딩 UI에 따라서 일단 API가 안나온 상황에서 더미 데이터로 스텝 로직을 구현해봤습니다.
스탭 버튼 관련해서, 사실 현재 선택된 최소 갯수를 맞아야만 다음 스텝으로 갈 수 있어야하다보니까,
useForm
에mode
를onChange Validation
으로 바꿔서.. 컴포넌트 리렌더링은 어쩔 수 없었습니다..혹시 의문이 가는점이 있다면.. 서슴없이 말씀해주세ㅇ..
코드 리뷰시 참고 사항
preview에서 /onboarding으로 가시면 보실 수 있으실것 같습니다.https://fiesta-git-feature-onboarding-page-froggy1014s-projects.vercel.app/onboarding