Jinho1011/week8 mission #129
Conversation
|
이거 리뷰만 부탁드리고 머지는 나중에 제가 할게요! |
| switch (step.name) { | ||
| case 'INIT': | ||
| return ( | ||
| <PasswordInput | ||
| key={step.name} | ||
| title={'비밀번호를 입력해주세요'} | ||
| length={PASSWORD_LENGTH} | ||
| onComplete={handleComplete} | ||
| /> | ||
| ); | ||
|
|
||
| case 'CONFIRM': | ||
| return ( | ||
| <PasswordInput | ||
| key={step.name} | ||
| title={'비밀번호를 다시 한 번 입력해주세요'} | ||
| length={PASSWORD_LENGTH} | ||
| onComplete={handleComplete} | ||
| /> | ||
| ); | ||
|
|
||
| case 'ERROR': | ||
| return ( | ||
| <PasswordInput | ||
| key={step.name} | ||
| title={'비밀번호를 다시 입력해주세요'} | ||
| subtitle={step.payload.message} | ||
| length={PASSWORD_LENGTH} | ||
| onComplete={handleComplete} | ||
| /> | ||
| ); | ||
|
|
||
| case 'COMPLETE': | ||
| return <Header title="비밀번호 입력을 완료했어요!" />; | ||
|
|
||
| default: | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
이렇게 하나의 component에서 상태에 따른 switch로 화면 변경하는 것이 퍼널 관리하는 방법인가요?
There was a problem hiding this comment.
넹 페이지 내부에서 명령형의 방식으로 INIT -> CONFIRM -> COMPLETE 등의 일련의 플로우를 구현하는게 아니라 각각의 상태를 선언적으로 정의해서 마치 state machine처럼 동작하도록 많이 하는거 같아요
참고. XState
| useEffect(() => { | ||
| if (isComplete) { | ||
| onComplete(password); | ||
| resetPassword(); | ||
| } | ||
| }, [isComplete]); |
There was a problem hiding this comment.
기존 state를 이용해 isComplete를 따로 선언하고 이를 의존성 배열에 담은 이유가 있나요?
password를 바로 의존성 배열에 담는 것보다 useEffect가 덜 실행되어서 그런 건가요?
There was a problem hiding this comment.
네! password가 변경될 때마다 effect 실행되는걸 방지하고 딱 해당 조건을 만족했을 때만 effect를 일으킬 수 있어서 이렇게 작성했어요
| const shuffledNumbers = useMemo( | ||
| () => shuffle(Array.from({ length: 10 }).map((_, index) => index)), | ||
| [], | ||
| ); |
There was a problem hiding this comment.
이 useMemo를 통해 처음에 받아온 숫자 화면 배치가 비밀번호 4개 다 입력할 때까지 유지되도록 하는 건가 보네요!
혹시 shuffle된 숫자 키패드 배열을 useState로 관리하지 않은 이유가 있으신가요?
There was a problem hiding this comment.
값이 변경되지 않고 초기 렌더링 시에만 계산하면 되어서 상태로 안만들었어요!
| if (index < value.length) { | ||
| return <div key={index} className="password-indicator-dot filled" />; | ||
| } | ||
| return <div key={index} className="password-indicator-dot empty" />; | ||
| })} |
There was a problem hiding this comment.
삼항연산자로 filled랑 empty 구분하는 것도 괜찮아 보이는데 if 문으로 구분한 것은 가독성 때문인가요?
There was a problem hiding this comment.
넹 가독성 측면에서 이게 더 낫다고 생각했어요
| return ( | ||
| <div className="password-indicator-container"> | ||
| {Array.from({ length }).map((_, index) => { | ||
| if (index < value.length) { |
There was a problem hiding this comment.
length가 PASSWORD_LENGTH 상수인데 props로 전달만 되고 있는 것 같은데,
혹시 상수를 export 해서 불러와서 쓰는 것보다 props 전달이 더 나은 것인지 궁금해요!
There was a problem hiding this comment.
비밀번호의 길이가 4가 기본값이라는 지식을 최대한 상위 계층(PasswordInput)에서 드러내고 싶었어요
| onClick={() => { | ||
| onPress(number); | ||
| }} |
There was a problem hiding this comment.
| onClick={() => { | |
| onPress(number); | |
| }} | |
| onClick={() => onPress(number)} |
There was a problem hiding this comment.
👍 👍
참고로 이렇게도 쓸 수 있습니다
export default function NumberKeypads({ numbers, onPress }) {
const handleKeyPress = () => {
return (number) => {
onPress(number);
};
};
return (
<div className="number-keypad">
<div className="number-keypad-grid">
{numbers.map((number) => (
<button
key={number}
className="number-keypad-button"
onClick={handleKeyPress(number)}
>
{number}
</button>
))}
</div>
</div>
);
}There was a problem hiding this comment.
👍 💯
함수를 return하는 함수라니.. 윽
뭔가 가독성이 더 안 좋은 것 같은데 내부 변수 같은 거 써서 클로저로 저장해야 할 필요가 있을 때 쓰면 좋으려나요..?
There was a problem hiding this comment.
onClick={handleKeyPress(number)}
onClick 부분에 함수 할당하는 부분을 깔끔하게 적고 싶을 때 저렇게 많이 쓰시더라고요
AB180에서 요렇게 많이 쓰는걸 봤숨다
📚 워크북 링크
📝 변경/추가 사항 요약
✅ 리뷰 요구사항
📸 확인 방법 (스크린샷)
2024-11-15.10.47.55.mov