[1단계 - 자판기] 병민(윤병인) 미션 제출합니다. #40

airman5573 · 2022-03-28T08:58:55Z

안녕하세요, 병민입니다 :D

이번 미션 피드백 잘 부탁드립니다 :D

데모 페이지

전체적인 구조

Store하나를 두고 각 컴포넌트에서 Store를 구독하는 형식입니다.
컴포넌트에서 action을 dispatch하면 Store의 상태가 변하고, 구독된 컴포넌트에게 알람이 갑니다. 그리고 컴포넌트는 store의 state를 바탕으로 다시 그려집니다.
컴포넌트는 HTMLElement를 상속받아 사용했습니다.
decorator를 사용해 CustomElementRepository에 클래스를 등록하는 과정을 간소화 했습니다.

Routing

netlify의 redirect를 사용해 새로고침을 해도 404로 이동되지 않도록 했습니다.

commitlint + commitizen 을 사용해서 commit message를 정형화한다

single quote, EOL, trailingComma 적용

store, reducer, creaAction 생성

component scope를 추가한다

redirect rule을 적용한다

event decorator를 사용하면 컴포넌트가 사라졌을때 등록한 event가 사라지기 때문에, 사용하기에 적절치 않다.

충전 금액이 음수일 경우 에러를 체크하는 테스트를 추가한다

austinpark420

안녕하세요 😄

리뷰를 맡은 오스틴입니다.
flux 패턴을 활용하셔서 개발하신 부분이 인상적이었습니다.
관련해서 코멘트 남겨놨으니 확인 부탁드릴게요!

UX/UI 관련

상품 관리 - 상품 현황 리스트 수정을 누르게되면 input 태그크기 때문인지 리스트 높이와 버튼 위치가 변하는데 이 부분 보기/수정 상관없이 리스트와 버튼의 위치가 동일하도록 css를 수정해보시면 좋을 것 같습니다.
피그마와 다른 내용이긴 하지만 상품 관리탭에 추가할 상품정보를 입력하는 부분에 placeholder으로 어떤 값을 넣어야하는 설명해주고 있지만 처음 값을 작성할 땐 어떤 항목에 대한 값을 작성하고 있는지 헷갈리는 경우가 있어 라벨이 있으면 어떨까라는 생각을 했습니다.

austinpark420 · 2022-03-28T11:56:58Z

src/components/navigation.ts

+    return `
+      <nav class="d-flex justify-content-center">
+        <button class="btn btn-secondary mr-1 ${
+          activeTab === Tab.ProductManageTab ? 'active' : ''


3항연산자 말고 &&을 활용하셔도 될것 같습니다.

activeTab === Tab.ProductManageTab && 'active' 를 써보니, false가 class에 들어갑니다.

아고 제가 리엑트 문법에 너무 익숙해져서 간과했던 부분이네요 🙏

austinpark420 · 2022-03-28T11:58:34Z

src/flux/createAction.ts

@@ -0,0 +1,10 @@
+import { Action } from '../types';
+
+const createAction = (type: string, payload: any): Action => {


austinpark420 · 2022-03-28T11:59:31Z

src/flux/reducer.ts

+  return coins;
+}
+
+const reducer = (state: AppState, { type, payload }: Action) => {


austinpark420 · 2022-03-28T12:02:40Z

src/flux/reducer.ts

+  if (type === ACTION.ADD_PRODUCT) {
+    newState.productList = [...newState.productList, { ...payload, isEditing: false }];
+  } else if (type === ACTION.CHANGE_EDIT_MODE) {
+    const { name, isEditing } = payload;
+    const index = newState.productList.findIndex((item) => item.name === name);
+    newState.productList[index].isEditing = isEditing;
+  } else if (type === ACTION.EDIT_PRODUCT) {
+    const { originalName, name, price, quantity } = payload;
+    const index = newState.productList.findIndex((item) => item.name === originalName);
+    newState.productList[index] = { name, price, quantity, isEditing: false };
+  } else if (type === ACTION.DELETE_PRODUCT) {
+    newState.productList = newState.productList.filter((item) => item.name !== payload);
+  } else if (type === ACTION.CHARGE_COINS) {
+    newState.chargedCoins = mergeCoins(newState.chargedCoins, moneyToCoin(payload));
+    newState.chargedMoney += payload;
+  } else if (type === ACTION.CHANGE_ACTIVE_TAB) {
+    newState.activeTab = payload;
+  }
+  return newState;


switch 문으로 리페토링 가능하시면 swith 문 활용하시는걸 추천드려요.

네, 그 편이 더 읽기 좋을것 같습니다. 감사합니다 :D

austinpark420 · 2022-03-28T12:04:06Z

src/components/purchase-product-tab.ts

+class PurchaseProductTab extends Component {
+  template(activeTab: Tab): string {
+    if (this.localName !== activeTab) return '';
+    return '<h3 class="text-center">🤖 페이지 건설중...</h3>';


UX/UI를 고려한 부분 좋네요. 👍

austinpark420 · 2022-03-29T03:03:19Z

src/validation/validators.ts

@@ -0,0 +1,70 @@
+import {


validators.ts 파일이 유독 가독성이 떨어져 보이네요. 🤔

맞습니다. 다시보니 읽기가 힘드네요. 수정하겠습니다 :D

austinpark420 · 2022-03-29T03:11:56Z

src/validation/validators.ts

+import { toInt } from '../utils';
+import ValidationResult from './validation-result';
+
+const isInteger = (str: string) => {


Intger은 javascript에는 없는 타입이네요.

네, 맞습니다. 다만 정수인지를 판단하는 일을 하는 함수라서
Integer가 직관적이라 더 어울려 보이는데, 리뷰어님의 생각은 어떠신가요?

toInt 함수와 동일한 의견입니다!

austinpark420 · 2022-03-29T03:17:50Z

src/components/product-inventory.ts

+    const productList = Store.instance.getState().productList;
+    const errorList = validateProduct(
+      { name, price, quantity },
+      productList.filter((item) => !(item.name === originalName && item.name === name))


개인적인 취향이지만 드모르간 법칙을 이용해 가독성좋게 바꿔보는건 어떨까요?

제안 감사합니다 :D

드모르간 법칙을 적용하면

const productList = Store.instance.getState().productList; const errorList = validateProduct( { name, price, quantity }, productList.filter((item) => item.name !== originalName || item.name !== name)

이렇게 되서, 더 잘 읽히는것 같습니다.

다만, 그래도 역시 한번에 이해하기 어려운 코드여서 아래와 같이 바꿔보았습니다.

const productList = Store.instance.getState().productList; const isEditingSameProduct = (item: ProductItem) => { if (originalName === name && item.name === name) return true; return false; }; const errorList = validateProduct( { name, price, quantity }, productList.filter(!isEditingSameProduct) ).filter((result) => result.hasError);

이 코드는 어떤가요?

austinpark420 · 2022-03-29T07:04:44Z

src/constants.ts

+export const MAX_LENGTH_OF_PRODUCT_NAME = 10;
+export const MIN_PRODUCT_PRICE = 100;
+export const MAX_PRODUCT_PRICE = 10000;
+export const MIN_COIN_UNIT = 10;
+export const MIN_PRODUCT_QUANTITY = 1;
+export const MAX_PRODUCT_QUANTITY = 20;
+export const MAX_CHARGABLE_MONEY = 100000;


관련있는 변수를 묶어서 관리해도 좋을 것 같습니다.

네, 수정하겠습니다 :D

austinpark420 · 2022-03-29T07:10:28Z

src/scss/_variables.scss

+
+$color-primary: $color-cyan-900;
+$color-secondary: $color-gray-400;
+// $color-cancel: $color-cyan-900;


불필요한 주석은 지워주시는게 좋을 것 같습니다.

네, 지우겠습니다 :D

austinpark420

안녕하세요. 병민님.
아직 리뷰요청을 주시진 않았는데요. 피드백 반영해주신 것으로 확인됩니다.
2단계 진행 및 리뷰 일정 확보를위해 머지하도록 하겠습니다.

step1 구현하시느라 고생많으셨습니다 👍

austinpark420 · 2022-04-01T07:33:31Z

src/validation/validators.ts

+import { toInt } from '../utils';
+import ValidationResult from './validation-result';
+
+const isInteger = (str: string) => {


toInt 함수와 동일한 의견입니다!

austinpark420 · 2022-04-01T07:34:55Z

src/utils.ts

+import { VALIDATION_ERROR_NAME } from './constants';
+import { Indexable } from './types';
+
+export const toInt = (str: string, defaultNum = 0) => {


디테일하시군요 👍
저는 그냥 보편적으로 숫자라고 생각했는데 병민님 의견 주신 부분이 맞는것 같습니다 🙏

austinpark420 · 2022-04-01T07:38:05Z

src/decorators/decortators.ts

+import Component from '../abstract/component';
+import { ComponentConstructor } from '../types';
+
+export function customElement(name: string) {


그렇군요. 제가 궁금했던건 customElement자체의 사용 이유 였습니다.
본인의 생각이 명확하시네요 👍

austinpark420 · 2022-04-01T07:41:01Z

src/components/navigation.ts

+    return `
+      <nav class="d-flex justify-content-center">
+        <button class="btn btn-secondary mr-1 ${
+          activeTab === Tab.ProductManageTab ? 'active' : ''


아고 제가 리엑트 문법에 너무 익숙해져서 간과했던 부분이네요 🙏

austinpark420 · 2022-04-01T08:12:30Z

src/components/product-inventory.ts

+    const productList = Store.instance.getState().productList;
+    const errorList = validateProduct(
+      { name, price, quantity },
+      productList.filter((item) => !(item.name === originalName && item.name === name))


austinpark420 · 2022-04-01T08:15:33Z

src/components/changes-inventory.ts

+      .map((unit) => {
+        return `
+        <tr>
+          <td>${unit.toLocaleString('ko-kr')}원</td>


prototype에 직접 추가하는 추천드리지 않습니다.. 어떤 사이드 이펙트가 생길지 모르기때문입니다.
store나 util로 따로 뺄순 있지않나 해서 말씀드렸습니다!

airman5573 and others added 30 commits March 23, 2022 15:39

🚚 chore(setup): 커밋 린트 설정

3d96d4c

commitlint + commitizen 을 사용해서 commit message를 정형화한다

🚚 chore(setup): webpack설정

9b4b681

🚚 chore(setup): lint와 formatter설정

3271886

🚚 chore(setup): dist폴더 추적 중단

c7f10de

📝 docs(todo): 구현할 기능 정리

a17868c

♻️ refactor(typo): typo scope 추가

a0a4316

♻️ refactor(typo): prettier 적용

e6b3bd9

single quote, EOL, trailingComma 적용

🚚 chore(setup): 함수 15줄 제한 해제

d7373e9

✨ feat(ui): component 생성

8f52110

🚚 chore(setup): typescript target 설정

b4a0297

🚚 chore(setup): commit scope flux 추가

f1be242

✨ feat(flux): store 생성

bfc5093

✨ feat(flux): flux 패턴 생성

0ca6b34

store, reducer, creaAction 생성

🚚 chore(setup): 명시적 any 선언을 허용한다

02c5ede

✨ feat(decorator): custom element decorator와 event bind decorator를 추가한다

6a7fc05

✨ feat(template): 상품관리탭 template, style 추가

c289836

🚚 chore(setup): 커밋 스코프 추가

df84428

component scope를 추가한다

♻️ refactor(component): 컴포넌트 분리

5e7798a

🚚 chore(setup): git에서 추적하지 않을 파일 추가

29c336c

✨ feat(ui): 상품 관리 탭에서 상품을 추가한다

68a57ef

✨ feat(component): 상품관리탭에서 상품 수정 기능 추가

d3cda8a

✨ feat(component): 상품 관리 탭에서 상품 삭제

49923c6

♻️ refactor(component): 반복되는 코드 함수로 분리

80b19e8

♻️ refactor(cleanup): 불필요한 코드 삭제

b0ad8c6

✨ feat(component): 상품 수정 삭제 기능 추가

ab5221a

♻️ refactor(ui): 상품 추가 필드 초기화 및 포커싱

1b2d9d9

✨ feat(component): 잔돈 충전 기능 추가

52b590b

♻️ refactor(cleanup): 사용하지 않는 type 삭제

3f3ed7d

♻️ refactor(cleanup): 불필요한 eslint disable comment 삭제

146aefa

✨ feat(routing): 라우팅 기능을 추가한다

62458e4

airman5573 and others added 18 commits March 28, 2022 00:55

👷 build: netlify 설정 파일 추가

fbb3d15

redirect rule을 적용한다

♻️ refactor(cleanup): 더미 데이터 삭제

870263d

🐛 fix(component): 동적으로 컴포넌트 그릴시 event가 재등록 되지 않는 문제 해결

f0ffe42

event decorator를 사용하면 컴포넌트가 사라졌을때 등록한 event가 사라지기 때문에, 사용하기에 적절치 않다.

✨ feat(component): 상품 구매 탭 임시 추가 및 라우팅 구현

99a449e

🐛 fix(component): 페이지 이동 시 nav 배경색이 변경되지 않는 에러 수정

7380b27

♻️ refactor(routing): 라우팅 시 state값을 기준으로 각 컴포넌트 내 동적 렌더링

7d5da7e

✨ feat(util): 객체 deep copy 유틸 함수 추가

26718e1

♻️ refactor(flux): state을 얕은 복사에서 깊은 복사로 변경

6c371e5

💄 style(ui): 1000이상 단위 콤마 추가

f565230

♻️ refactor(cleanup): 사용하지 않는 모듈 import 제거

dbb6fef

🐛 fix(routing): 라우팅 발생 전 Component 삽입

eb3d0fe

♻️ refactor(typo): config 파일에 lint 적용

9824bc7

✅ test(validation): validation test 추가

bab412b

♻️ refactor(cleanup): 불필요한 모듈 import 제거

6b14bb0

♻️ refactor: 상수 처리

d7e0fdc

✅ test(validation): 충전 금액 테스트 추가

84da723

충전 금액이 음수일 경우 에러를 체크하는 테스트를 추가한다

♻️ refactor: 에러 메세지에 들어간 숫자를 상수로 대체

7f6920c

🐛 fix(ui): 엔터 클릭으로 form 제출 방지

695df52

pocojang requested a review from austinpark420 March 28, 2022 10:32

pocojang assigned austinpark420 Mar 28, 2022

austinpark420 suggested changes Mar 29, 2022

View reviewed changes

austinpark420 approved these changes Apr 1, 2022

View reviewed changes

austinpark420 merged commit 960db37 into woowacourse:airman5573 Apr 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 자판기] 병민(윤병인) 미션 제출합니다. #40

[1단계 - 자판기] 병민(윤병인) 미션 제출합니다. #40

airman5573 commented Mar 28, 2022 •

edited

Loading

austinpark420 left a comment

austinpark420 Mar 28, 2022

airman5573 Mar 30, 2022

austinpark420 Apr 1, 2022

austinpark420 Mar 28, 2022

austinpark420 Mar 28, 2022

austinpark420 Mar 28, 2022

airman5573 Mar 30, 2022

austinpark420 Mar 28, 2022

austinpark420 Mar 29, 2022

airman5573 Mar 30, 2022

austinpark420 Mar 29, 2022

airman5573 Mar 30, 2022

austinpark420 Apr 1, 2022

austinpark420 Mar 29, 2022

airman5573 Mar 30, 2022

austinpark420 Apr 1, 2022

austinpark420 Mar 29, 2022

airman5573 Mar 30, 2022

austinpark420 Mar 29, 2022

airman5573 Mar 30, 2022

austinpark420 left a comment

austinpark420 Apr 1, 2022

austinpark420 Apr 1, 2022

austinpark420 Apr 1, 2022

austinpark420 Apr 1, 2022

austinpark420 Apr 1, 2022

austinpark420 Apr 1, 2022

		@@ -0,0 +1,10 @@
		import { Action } from '../types';

		const createAction = (type: string, payload: any): Action => {

[1단계 - 자판기] 병민(윤병인) 미션 제출합니다. #40

[1단계 - 자판기] 병민(윤병인) 미션 제출합니다. #40

Conversation

airman5573 commented Mar 28, 2022 • edited Loading

전체적인 구조

Routing

austinpark420 left a comment

Choose a reason for hiding this comment

UX/UI 관련

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

austinpark420 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

airman5573 commented Mar 28, 2022 •

edited

Loading