Skip to content
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주차 1번째 PR #26

Merged
merged 12 commits into from
May 11, 2022

Conversation

yongseongjeon
Copy link
Collaborator

@yongseongjeon yongseongjeon commented May 11, 2022

에드님 안녕하세요!

리뷰 해주셔서 감사합니다!

데모 페이지입니다.

진행 상황

컴포넌트 구조

<App /> 컴포넌트에서 라우팅 처리 및 Context를 생성합니다.

image

@yongseongjeon yongseongjeon added the review-FE New feature or request label May 11, 2022
@sungik-choi
Copy link

전반적으로 코드 퀄리티가 좋다고 느꼈습니다 👍
태스크도 깔끔하게 잘 분리하신 거 같아요. (혹시 도식은 어떤 툴을 사용하신건가요? 좋아보이네요 🤩)

이 정도의 코드 퀄리티를 유지하시면서 쭉 구현해나가시면 좋은 결과물이 될 거 같아요. 수고 많으셨습니다!

</Wrap>
);
function handleChargeMoney() {
const hasCoins = moneyCnt >= 1;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변수가 잘 분리되어서 읽기 좋네요 👍


export default Product;

const Wrap = styled.div({});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 스타일 컴포넌트는 그냥 div로 사용해줘도 무방할 거 같아요. 덜 작성되었나 오해할 수도 있겠다는 생각이 들어요.

@@ -0,0 +1,9 @@
const MESSAGES = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 분리해주셨습니다! 💯 조금만 더해보자면, constants/ 같은 디렉토리 안에 위치했으면 더 좋았겠다는 생각이 들어요! 지금 모듈명만으로는 상수 모듈이라는게 잘 유추되지 않는 거 같습니다.

@@ -0,0 +1,17 @@
import { getRandomNumber } from 'utils';

const COINS = [

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array.prototype.map() 메서드를 활용해보면 더 간결하게 표현할 수 있을 거 같아요.

return (
<ProductWrap>
{PRODUCTS.map(({ name, price }) => (
<Product name={name} price={price} />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

key prop을 추가해주세요!

@@ -0,0 +1,11 @@
function getRandomNumber({ min, max }) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 유틸함수 분리 좋네요!

const [errorMsg, setErrorMsg] = useState(defaultErrorMsg);

return (
<MoneyContext.Provider value={{ curMoney, setMoney, showErrorMsg }}>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 구조에선 아니지만, 만약 setMoney 혹은 showErrorMsg만 사용하는 컴포넌트가 있다고 가정해봅시다. value에 curMoney가 포함되어있으므로, curMoney가 변하면 객체가 새로 생성되면서 레퍼런스가 변하게 되어 curMoney를 사용하지 않음에도 리렌더링이 일어나게 됩니다. 어떻게 하면 불필요한 리렌더링을 방지할 수 있을까요? 공부해보시면 큰 도움이 될 거라 생각합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함께 변하지 않는 상태들에 대해서 Context를 나누면 될 것 같습니다.
혹시 말씀해주신 부분이 다음 이슈와 관련이 있는지 여쭤봐도 될까요?
facebook/react#15156 (comment)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네 맞습니다. 자주 변하는 상태와 아닌 상태를 분리하고 useMemo, useCallback을 사용해서 동일한 참조를 유지하는 방식으로 불필요한 리렌더링을 막을 수 있어요!

@@ -0,0 +1,3 @@
# https://www.robotstxt.org/robotstxt.html

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

의도해서 소스에 포함하신건지 궁금하네요 👀

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRA로 생성되었는데 의도해서 포함하진 않았습니다. 😅

@sungik-choi sungik-choi merged commit 9bce1ba into codesquad-members-2022:rumka May 11, 2022
@yongseongjeon
Copy link
Collaborator Author

정성껏 리뷰해주셔서 감사합니다!!
활용했던 툴 링크는 다음과 같습니다!
https://excalidraw.com/

kowoohyuk pushed a commit that referenced this pull request May 21, 2022
* chore: 프로젝트 초기 셋팅 (#2)

* feat:VendingMachine, Wallet 라우팅 (#5)

* Wallet Component - Markup (#6)

* update: money data

* design: Coin Component

* design: Balance Component

* design: Wallet Component

* VendingMachine Component - Markup (#9)

* update: product data

* design: VendProduct Component

* design: VendController Component

* design: HistoryBox Component

* design: VendingMachine Component

* design: ChangeOutlet Component

* design: InsertCoin Component

* refactor: VendController Component의 하위컴포넌트 분리

* chore: update yarn.lock

* Common Components 분리(1) (#10)

* feat: Button Component 구현

* feat: InsertCoin, BrandLabel, Unit에 Button Component 적용

* chore: polished 라이브러리 설치

* refactor: Nav를 NavLink 적용하여 리팩토링

* chore: gh-pages 라이브러리 설치

* Display Mode(dark/light) (#13)

* [DOTORI] 1주차 첫번째 PR (#25)

* chore: 프로젝트 초기 셋팅 (#2)

* feat:VendingMachine, Wallet 라우팅 (#5)

* Wallet Component - Markup (#6)

* update: money data

* design: Coin Component

* design: Balance Component

* design: Wallet Component

* VendingMachine Component - Markup (#9)

* update: product data

* design: VendProduct Component

* design: VendController Component

* design: HistoryBox Component

* design: VendingMachine Component

* design: ChangeOutlet Component

* design: InsertCoin Component

* refactor: VendController Component의 하위컴포넌트 분리

* chore: update yarn.lock

* Common Components 분리(1) (#10)

* feat: Button Component 구현

* feat: InsertCoin, BrandLabel, Unit에 Button Component 적용

* chore: polished 라이브러리 설치

* refactor: Nav를 NavLink 적용하여 리팩토링

* chore: gh-pages 라이브러리 설치

* feat: useDisplay 커스텀 훅 생성

* feat: AppLayout, ToggleDisplay Component 구현

* feat: DisplayProvider 구현

displayMode를 전역 상태로 관리한다.

* design: title font 변경, change outlet title 대문자로 변경

* PR1 Refactoring (#16)

* refactor: components 디렉토리 구조 변경

* feat: common components - Nav 구현

* refactor: 라우터 모듈 분리

* refactor: 사용하지 않는 styled-components 제거, div->span, strong 태그 변경

* refactor: CoinContainer, VendProductContainer Component 분리

* Wallet Component - 동전 투입 기능 (#17)

* feat: 금액 버튼 클릭시 해당 금액 개수 감소 렌더링

useCoin 커스텀 훅 사용

* feat: 금액 버튼 클릭시 잔고 금액 감소 렌더링

useBalance 커스텀 훅 사용

* feat: CoinProvider 생성

outlet 상위 컴포넌트에 생성하여 라우팅에 따른 상태 초기화 방지

* feat: InsertCoinProvider 구현

지갑에서 선택한 금액을 자판기에 투입된 금액으로 렌더링

* rename: components 디렉토리 구조 수정

* remove: merge로 생성된 중복 파일 제거

* rename: 컴포넌트 디렉토리명 대문자로 변경

* refactor: balance state 제거, useBalance삭제

coin state로 연산할 수 있는 balance state를 제거

* PR2 Refactoring (#21)

* refactor: setCoin 로직 변경

배열의 카피본에서 같은 값을 갖는 인덱스 요소의 객체를 변경 -> map 사용

* refactor: grid repeat 함수 적용

* refactor: Coin Component useMemo 적용, setProvider 분리

Coin Component 클릭시 금액의 변동사항이 있는 컴포넌트만 리렌더링 된다.

* feat: 지갑에서 금액을 투입하면 알림 문구가 뜬다. (#23)

useReducer 적용

* 자판기 - 금액 투입, 상품 선택기능  (#25)

* feat: 지갑에서 금액을 투입하면 알림 문구가 뜬다.

useReducer 적용

* feat: 자판기에 투입된 금액 자동보정 기능 구현

* design: historyBox list margin, 자판기 margin-top

* feat: 자판기에서 금액을 투입하면 알림 문구가 뜬다.

* refactor: 투입 금액 보정 함수 수정

재귀함수로 리팩토링

* feat: 자판기 투입 최소, 최대 금액 설정

0원이면 early return, 잔고보다 큰 금액이면 잔고를 return

* feat: 최대금액 입력시 지갑 잔고 관리

잔고와 동전의 개수는 0으로 초기화된다.

* feat: history provider 분리로 렌더링 최적화

* feat: SelectButton 구현

inputCoin과 stocked에 따라 선택 가능한 상품에 초록불을 렌더한다.

* feat: 선택 가능한 상품을 누를 때 상품명, 잔돈 알림 문구 구현

* feat: 돈을 투입하고 5초간 입력이 없으면 잔돈 자동반환

* feat: 투입금액이 추가 발생하면 타이머 초기화

디바운싱을 이용하여 타이머를 초기화한다.

* rename: setTimer -> setDebounce

* 자판기 - 상품 선택 기능(2) (#26)

* feat: 지갑에서 금액을 투입하면 알림 문구가 뜬다.

useReducer 적용

* feat: 자판기에 투입된 금액 자동보정 기능 구현

* design: historyBox list margin, 자판기 margin-top

* feat: 자판기에서 금액을 투입하면 알림 문구가 뜬다.

* refactor: 투입 금액 보정 함수 수정

재귀함수로 리팩토링

* feat: 자판기 투입 최소, 최대 금액 설정

0원이면 early return, 잔고보다 큰 금액이면 잔고를 return

* feat: 최대금액 입력시 지갑 잔고 관리

잔고와 동전의 개수는 0으로 초기화된다.

* feat: history provider 분리로 렌더링 최적화

* feat: SelectButton 구현

inputCoin과 stocked에 따라 선택 가능한 상품에 초록불을 렌더한다.

* feat: 선택 가능한 상품을 누를 때 상품명, 잔돈 알림 문구 구현

* feat: 돈을 투입하고 5초간 입력이 없으면 잔돈 자동반환

* feat: 투입금액이 추가 발생하면 타이머 초기화

디바운싱을 이용하여 타이머를 초기화한다.

* rename: setTimer -> setDebounce

* remove: vendProductOutlet Component 삭제

* refactor: 투입 금액 보정 로직 변경

잔고가 있는 돈의 배열을 재귀로 돌면서 투입금액에 가까이 누적시킨다.

* feat: 지갑과 자판기에서 금액을 투입하면 선택 시간 3초 초기화

provider에서 useEffect를 사용하여 wallet, vm 컴포넌트에서 insertCoin의 sideEffect를 감지

* refactor: 화살표 함수 컨벤션 통일

* refactor: 중복 코드를 하나의 객체로 변경

* refactor: styled component 분리
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-FE New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants