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

[DOTORI] 1주차 두번째 PR #57

Merged
merged 13 commits into from
May 14, 2022

Conversation

mogooee
Copy link
Collaborator

@mogooee mogooee commented May 13, 2022

안녕하세요 json! 도토리입니다! 늦은 PR 죄송합니다 😭 리뷰 잘 부탁드립니다 감사합니다 🙏

🤔 step2 진행상황

🤹‍♀️ 데모페이지

[ 구현 완료 ]

  1. display mode(dark/light)

    • contextAPI, 커스텀훅(useDisplay) 학습 목적으로 다크모드/라이트 모드 구현
  2. Wallet component

    • 동전 투입 기능
      • 커스텀훅(useCoin) 사용
      • useCallback 사용
      • Coin component에 useMemo 적용
    • 잔고 관리 기능
      • coin state로 balance 연산하여 렌더링

[ 구현 예정 ]

  • 자판기 기능 구현
  • useReducer 적용

[ 고민하는 점 ]

  1. 컴포넌트 디렉토리 구조
    말씀해주신 역할 단위로 컴포넌트 디렉토리 구조를 변경해보려고 했습니다.
  • vend-controller 디렉토리가 약간 어색한 것 같은데 하위에 있는 ChangeOutlet, HistoryBox 컴포넌트들을 분리하는 것이 나을까요?

  • 디렉토리가 컴포넌트의 이름과 같으면 PascalCase를 사용하였고, coin처럼 내부에 관련된 여러개의 컴포넌트들(Coin, CoinContainer)이 들어있는 경우에는 별도의 네이밍 문법을 적용하지 않았습니다. 통일성을 위해 모두 단일한 컴포넌트로 분리한 후 PascalCase를 적용하는 것이 나을까요?

      components
       ┣ AppLayout
       ┃ ┗ AppLayout.jsx
       ┣ Balance
       ┃ ┗ Balance.jsx
       ┣ MainNav
       ┃ ┗ MainNav.jsx
       ┣ Routers
       ┃ ┗ Routers.jsx
       ┣ ToggleDisplay
       ┃ ┗ ToggleDisplay.jsx
       ┣ coin
       ┃ ┣ Coin.jsx
       ┃ ┗ CoinContainer.jsx
       ┣ common
       ┃ ┣ Button.jsx
       ┃ ┗ Nav.jsx
       ┣ vend-controller
       ┃ ┣ ChangeOutlet.jsx
       ┃ ┣ HistoryBox.jsx
       ┃ ┣ InsertCoin.jsx
       ┃ ┗ VendController.jsx
       ┣ vend-product
       ┃ ┣ VendProduct.jsx
       ┃ ┗ VendProductContainer.jsx
       ┗ index.jsx
    
  1. 리렌더링 최적화
    현재 context API(CoinProvider)사용으로 지갑의 금액을 클릭하면 지갑 컴포넌트와 지갑의 하위 컴포넌트가 전부 리렌더링됩니다.
    문제 해결을 위해 count에 변경사항이 있는 Coin, Balance 컴포넌트만 리렌더링이 되도록 Coin 컴포넌트에 useMemo를 적용해보았습니다. 하지만 익스텐션으로 확인해보니 여전히 리렌더링이 방지되지 못하고 있습니다.. ㅜㅜ

  2. 라우팅이 발생할 때 컴포넌트의 상태가 초기화!
    MainNav에서 VendingMachineWallet으로 라우팅이 발생할 때 각 컴포넌트 내부의 상태 역시 리렌더링과 동시에 초기화 되는 문제가 발생했습니다. 이를 해결하기 위해 컴포넌트 내부의 상태를 전역상태로 만들어 VendingMachineWallet 컴포넌트의 상위에 context로 상태를 전달하였습니다.
    하지만 이런 해결방식은 컴포넌트의 모든 상태들을 전역으로 관리해야한다는 단점이 있는 것 같습니다. 라우터를 사용할 때 전역이 아닌 각 컴포넌트 내에서 상태를 관리할 수 있는 방법이 있을까요??
    history 객체에 state를 저장해서 nav 버튼을 클릭할 때마다 useNavigate를 적용하는 방법도 고려해보았는데 결국 상위에서 state를 들고 있어야 각 라우터에 전달할 수 있으므로 위의 방식처럼 전역상태로 관리해야한다는 문제를 해결하지 못했습니다..

mogooee and others added 10 commits May 10, 2022 15:37
* update: money data

* design: Coin Component

* design: Balance Component

* design: Wallet Component
* 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
* feat: Button Component 구현

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

* chore: polished 라이브러리 설치

* refactor: Nav를 NavLink 적용하여 리팩토링
* [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 대문자로 변경
* refactor: components 디렉토리 구조 변경

* feat: common components - Nav 구현

* refactor: 라우터 모듈 분리

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

* refactor: CoinContainer, VendProductContainer Component 분리
* feat: 금액 버튼 클릭시 해당 금액 개수 감소 렌더링

useCoin 커스텀 훅 사용

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

useBalance 커스텀 훅 사용

* feat: CoinProvider 생성

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

* feat: InsertCoinProvider 구현

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

* rename: components 디렉토리 구조 수정
@kowoohyuk kowoohyuk assigned mogooee and unassigned kowoohyuk May 14, 2022
Copy link

@kowoohyuk kowoohyuk left a comment

Choose a reason for hiding this comment

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

도토리 안녕하세요!
이번 주도 개발하시느라 수고많으셨어요 😄

여러 고민과 시도들이 많이 보이는 코드였습니다!
커스텀훅과 다크모드가 돋보였어요! 👍

고민하는 점에 대한 답변

  1. 컴포넌트 디렉토리 구조
  • vend-controller 디렉토리의 하위에 있는 컴포넌트들이기에 해당 경로에 있어도 괜찮을 것 같아요!
    컴포넌트 분리는 필요성이 느껴지신다면 (어색하다거나, 더 나은 방향이 생각난다거나!) 바꿔도 좋을 것 같습니다!
  • 네이밍과 관련한 부분은 어색하지 않고, 명확한 사유가 존재한다면 모두 정답이라고 생각해요. 그렇기에 현재 구조에서 어색한 부분이 느껴지신다면, 파스칼 케이스로 변경해도 괜찮을 것 같아요!
  1. 리렌더링 최적화
  • 상태변경으로 인해 리렌더링이 발생하는 것 같은데요! 이 경우에는 컴포넌트의 구조를 변경하는 게 좋을 것 같아요! ✋
  1. 라우팅이 발생할 때 컴포넌트의 상태가 초기화
  • 지갑으로 갔다가 자판기로 돌아왔을 때, 상태값을 보존했으면 좋겠다는 말씀일까요?!
    그렇다면, 말씀하신 것처럼 전역으로 관리를 하시는 방법도 괜찮고! localStorage를 고려하셔도 괜찮을 것 같아요.

const { insertCoin, setInsertCoin } = useContext(InsertCoinContext);

const clickCoin = () => {
if (!count) return;

Choose a reason for hiding this comment

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

early return 👍

Comment on lines +17 to +18
const color = displayMode === "light" ? "white" : "black";
const icon = displayMode === "light" ? "🌚" : "🌝";

Choose a reason for hiding this comment

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

다크모드를 구현하셨군요!
밤에 확인을 해서 눈이 침침했었는데.. 감사합니다! 😄

Comment on lines +8 to +11
const copyArray = [...coin];
const findIndex = coin.findIndex((e) => e.unit === unit);
copyArray[findIndex] = { ...copyArray[findIndex], count: count - 1 };
setCoin(copyArray);

Choose a reason for hiding this comment

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

map으로 변경해도 될 것 같아요! ✋


const StyledVendProductContainer = styled.ul`
display: grid;
grid-template-columns: 25% 25% 25% 25%;

Choose a reason for hiding this comment

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

동일한 비율이라면 repeat을 적용해도 괜찮을 것 같아요!

return { displayMode, toggleDisplay };
}

export { useDisplay };

Choose a reason for hiding this comment

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

커스텀 훅을 적용해보셨군요! 👍

@kowoohyuk kowoohyuk merged commit 819428a into codesquad-members-2022:dotori May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants