[도비] VM 1주차 금 PR #51
[도비] VM 1주차 금 PR #51
Conversation
* Rename: Layout과 관련된 폴더 생성 및 파일 이름 변경 Related to: #9 * Design: props를 distructuring 하도록 수정 Related to: #9 * Style: ProductContainer 컴포넌트 내부의 변수명 변경 - 구체적인 이름으로 변경 Close: #9 Co-authored-by: kyle <[email protected]>
* Refactor: Router dom과 관련된 컴포넌트 분리 * Style: 변수명 변경 * Feat: 지갑의 금액버튼을 누르면 총금액이 감소하는 기능 구현 - context API와 useReducer 사용 - Wallet 컴포넌트에서 Money 컴포넌트 파일 분리 - 총금액을 구하는 유틸함수 생성 Related to: #11 * Feat: 자판기에 지갑에서 투입된 금액을 보여주도록 추가 - mock data 추가 - insertMoney 시 insertMoneyData가 업데이트 되도록 함 Related to: #11 * Refactor: Input과 관련된 컴포넌트 분리 * Refactor: moneyContext 개선 - insertMoneyData를 누적해서 더하는 방식으로 변경 - insertMoney의 변수명을 buttonInsertMoney으로 변경 Related to: #11 * Feat: 자판기 input에서 금액 투입 구현 - OrderContainer 컴포넌트 내부에 있던 useState를 context로 이동 - 투입 버튼 클릭시 자동보정되는 기능 추가 - 지갑의 돈이 큰 단위부터 소모되도록 구현 Related to: #11 * Feat: 투입된 금액 만족시 구매 버튼 활성화 기능 구현 - 품절된 상품이거나 투입된 금액 보다 높은 가격의 상품의 구매버튼 비활성화 Related to: #12 * Style: 중복되는 부분을 변수로 할당, 주석 추가 * Style: 변수명 변경 - count -> unit
There was a problem hiding this comment.
안녕하세요 2일이라는 짧은 시간에도 많이 구현하셨네요.
먼저 리뷰가 늦어져서 죄송합니다 ㅠㅠ
이번 PR에서 다양한 훅들을 활용하셨는데 너무 잘 활용해주신 것 같아요. 이미 훅을 잘 다루시는 것 같은데 이런 훅을 활용해서 정말 다양한 커스텀훅을 만들어서 사용할 수 있는데요. 커스텀훅은 재사용을 위해서만을 사용하지는 않는 것 같아요. 관심사를 분리하기 위해서만도 사용하곤해요. 도비도 학습하시는 단계시다보니 재사용성은 크게 신경쓰지 말고 다양한 커스텀훅을 만들어서 활용해보시면 많은 도움이 될 것 같습니다!
스토리 작성해주신 것도 봤는데 세세하게 정말 잘 작성해주신 것 같아요👍 저렇게 작성한 스토리를 테스트코드로도 연결지어서 구현할 수 있겠네요. 저렇게 사용자의 스토리를 테스트하는 것은 e2e테스트라고 부르고 요즘에는 cypress라는 라이브러리가 자주 사용되는 것 같습니다 :) 참고하셨다가 나중에 학습해봐도 좋을 것 같습니다.
한주 동안 고생많으셨습니다!
| import Wallet from 'pages/Wallet'; | ||
| import NotFound from 'pages/NotFound'; | ||
|
|
||
| function Router() { |
There was a problem hiding this comment.
Router라는 컴포넌트에서 라우팅을 한번에 관리하셨군요! 정말 좋네요 :)
| export const MoneyContext = createContext(initState); | ||
|
|
||
| export const MoneyProvider = ({ children }) => { | ||
| const [state, dispatch] = useReducer(moneyReducer, initState); |
There was a problem hiding this comment.
useReducer 훅까지 다양하게 훅을 활용하시는게 참 좋네요. useReducer 같이 action을 분리하는 패턴은 유용한 패턴이니 이번 기회에 익숙해지시면 좋겠네요
| case 'BUTTON_INSERT_MONEY': | ||
| const updateWalletMoney = state.walletMoneyData.map(money => { | ||
| return money.unit === action.payload | ||
| ? (money = { ...money, amount: --money.amount }) |
| root.render( | ||
| <React.StrictMode> | ||
| <App /> | ||
| </React.StrictMode> | ||
| ); | ||
| root.render(<App />); |
There was a problem hiding this comment.
지갑에서 돈 클릭시 두번 실행되는 문제가 있어서 StrictMode를 제거했습니다!
| const totalMoney = walletInfo.reduce((acc, cur) => acc + cur.count * cur.amount, 0); | ||
| const { walletMoneyData } = useContext(MoneyContext); | ||
|
|
||
| const setMoneyComponents = walletMoneyData.map((money, index) => { |
There was a problem hiding this comment.
setXXX 같은 함수명은 주로 setter에 많이 사용되는 변수명이에요. Money 컴포넌트 리스트에 맞는 변수명으로 변경하면 좋을 것 같네요!
개인적으로 동사로 시작하는 변수명은 주로 함수에만 사용하는 것 같아요 :)
| useEffect(() => { | ||
| if (totalMoney >= info.price) { | ||
| setIsAvailable(true); | ||
| } | ||
|
|
||
| if (totalMoney === 0) { | ||
| setIsAvailable(false); | ||
| } | ||
| }, [totalMoney, info.price, isAvailable]); |
There was a problem hiding this comment.
이팩트 훅도 잘 활용해주셨네요! 궁금한 부분이 있는데요. isAvailable를 dependency에 추가해주신 이유가 있을까요? (린트에서 추가하라고 했을 수도 있지만...) isAvailable만 변경되는 상황에서는 다시 이팩트를 호출해야되는 상황인가에 대해서는 의문인 것 같아서요.
오히려 totalMoney가 변경됐을 경우에 이팩트가 실행되어야 될 것 같아서 totalMoney를 추가하면 좋지 않을까요?
There was a problem hiding this comment.
이 경우는 제가 잘못 추가한것 같습니다! useEffect의 dependency에 대해 다시 공부하도록 하겠습니다 🥲
| -ms-overflow-style: none; /* IE and Edge */ | ||
| scrollbar-width: none; /* Firefox */ | ||
|
|
||
| &::-webkit-scrollbar { | ||
| display: none; /* Chrome , Safari , Opera */ | ||
| } |
| const 투입가능금액 = item.unit * item.amount; | ||
| const 투입가능횟수 = Math.floor(투입가능금액 / item.unit); | ||
| insertLog.push({ ...item, amount: 투입가능횟수 }); | ||
| money -= 투입가능금액; |
There was a problem hiding this comment.
한글 변수명이 새롭네요 ㅎㅎ 생각보다 코드를 읽는데 이해가 잘 되네요. 요런 다양한 시도도 좋은 것 같네요
| case 'INPUT_INSERT_MONEY': | ||
| const updateWalletMoney2 = state.walletMoneyData.map(money => { | ||
| action.payload.forEach(el => { | ||
| el.id === money.id && (money = { ...money, amount: money.amount - el.amount }); | ||
| }); | ||
|
|
||
| return money; | ||
| }); |
There was a problem hiding this comment.
StrictMode를 제거하신 이유를 말씀해주셔서 추가로 리뷰 남겨요!
StrictMode에서 2번 실행되는 이유를 생각해보면 Reducer들이 순수함수인지 확인해봐야 될 것 같아요.
이 부분에서도 map을 도는 money값을 직접 forEach 구문에서 변경하는 것은 위험해 보이네요.
안녕하세요 kyle 도비입니다!
이틀동안 구현한건 많이 없지만 잘부탁드립니다...🐢
진행 상황
2022-05-13.4.58.41.mov
스토리 작성
context API와 useReducer를 이용한 상태관리
할 일
궁금한 점
지금은 제가 정확히 뭘 모르는지 모르겠어서 궁금한 점은 학습을 더 하고 다음주에 질문드리겠습니다! @.@