[럼카] 2주차 2번째 PR #104
Merged
[럼카] 2주차 2번째 PR #104
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
수정전에는 코인의 Index로 확인하고 있었다.
- 상품이 없으면 품절 메세지 띄우고 비활성화 - 상품 클릭하면 재고 및 현재 충전된 돈 업데이트
- 상품 클릭시 로딩 컴포넌트 랜더링 closed #6
sungik-choi
approved these changes
May 22, 2022
| setInputMode(false); | ||
| setEventLog([...eventLog, { type: 'CHARGE', value: newMoney }]); | ||
|
|
||
| function chargeMoney(money) { |
There was a problem hiding this comment.
이 함수를 좀 더 잘게 나누고, reduce를 사용한 방식으로 리팩토링해보시면 좋을 거 같아요! 현재는 한 눈에 로직을 알아보기가 쉽지 않아보여요.
| </> | ||
| ); | ||
|
|
||
| function handleClickedInputBtn() { |
There was a problem hiding this comment.
전재산을 투입한 이후 아무것도 구매하지 않은 상태로 다시 투입할 금액을 입력하면 이전에 투입했던 금액이 그대로 증발해버리고 있어요. 1. 50000원을 투입, 2. 8000원을 추가 투입 한 상황이라면 자판기 투입 금액이 8000원 이 아니라 58000원 이 되어야할 거 같네요!
(+ 혹시 아직 구현하지 않으신 기능이라면 이 코멘트는 무시해주셔도 괜찮습니다!)
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable no-restricted-syntax */ | |||
| } | ||
|
|
||
| function getQuotient(parent, child) { | ||
| return Math.floor(parent, child); |
There was a problem hiding this comment.
Suggested change
| return Math.floor(parent, child); | |
| return Math.floor(child / parent); |
- 몫을 구하는 유틸 함수인데 이런 형태를 의도하신걸까요?
- 사용처가 없는데,
ModifiableInput컴포넌트에서 사용할 수 있을 거 같네요.
| return <Wrap>{eventLog.map(createEventLog)}</Wrap>; | ||
|
|
||
| function createEventLog({ type, value }, idx) { | ||
| if (type === 'CHARGE') { |
There was a problem hiding this comment.
이벤트 로그 텍스트만 조건문을 통해 가져오고, 마지막에 그 텍스트를 감싸는 컴포넌트를 리턴하면 중복을 줄일 수 있어 좋지 않을까요?
| import { LoadingContext } from 'components/App'; | ||
| import DELAY_MS from 'constants/delay'; | ||
|
|
||
| function Loading() { |
|
|
||
| function Loading() { | ||
| const { setLoading } = useContext(LoadingContext); | ||
| useEffect(handleLoading, []); |
There was a problem hiding this comment.
Suggested change
| useEffect(handleLoading, []); | |
| useEffect(function handleLoading() { | |
| setTimeout(() => setLoading(false), DELAY_MS.LOADING); | |
| }, []); |
취향일수도 있는 부분인데, 저는 이런 형태가 코드(콜백)가 하는 일을 아래 라인(L16)으로 갈 필요 없이 바로 확인할 수 있어서 가독성에 더 좋다고 생각해요.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
에드님 안녕하세요.
리뷰 해주셔서 감사합니다.
데모 페이지입니다.
진행 상황
금액 버튼을 클릭하면 자판기에 금액이 추가되고 돈의 갯수는 줄어든다.금액 버튼을 클릭했을 때 돈의 갯수가 0개 이하면 관련 에러 메세지를 띄운다.