Conversation
| document.querySelector('#app section').innerHTML = TEMPLATES.PRODUCT_ADD; | ||
|
|
||
| products.forEach(product => { | ||
| renderProduct(product); |
There was a problem hiding this comment.
renderProduct 함수 내부를 보니까 product를 real dom에 append 해주고있더라구요.
그래서 forEach로 순회하면 매번 append 하면서 reflow가 발생하네요
한번에 모아서 붙이는 방법을 사용하면 성능 면에서 더 좋았을 것 같아요!
| $productAddMenuButton, | ||
| $vendingMachineManageMenuButton, | ||
| $productPurchaseMenuButton | ||
| ); |
There was a problem hiding this comment.
요 버튼 만드는 부분 위에서 배열 순회하면서 했던 방법처럼 하면 중복을 더 줄일 수 있을거같아용!
const [ $productAddMenuButton, $vendingMachineManageMenuButton, $productPurchaseMenuButton ] =
navList.map(({ tagName, id, innerText })=>
createEl({ tagName, id, innerText })
);| tagName: 'button', | ||
| className: 'purchase-button', | ||
| innerText: '구매하기', | ||
| }); |
| if (innerText) $el.innerText = innerText; | ||
|
|
||
| return $el; | ||
| }; |
There was a problem hiding this comment.
props로 children 받아서 자식 append도 할 수 있게 기능 추가하면 유용할것같아요!
export const createEl = ({ tagName, id, className, innerText, children }) => {
const $el = document.createElement(tagName);
if (id) $el.id = id;
if (className) $el.classList.add(className);
if (innerText) $el.innerText = innerText;
if(children) $el.append(children);
return $el;
};
| alert(ERROR_MSG.PRODUCT_MIN_PRICE); | ||
| } else if (price % 10) { | ||
| alert(ERROR_MSG.PRODUCT_MIN_PRICE_UNIT); | ||
| } else if (quantity < 1) { |
There was a problem hiding this comment.
else if로 걸면 100원 이상 양이 1 미만인 경우는 못거르지 않나요?
만약 그렇다면 if로 해야할 것 같아요
if (a조건 || b조건) 이런식으러?
| const newProduct = { | ||
| name: name, | ||
| price: price, | ||
| quantity: quantity, |
| tagName: 'tr', | ||
| className: 'product-manage-item', | ||
| }); | ||
| const [$newProduct_name, $newProduct_price, $newProduct_quantity] = [ |
There was a problem hiding this comment.
음 돔이라 그런걸 수도 있지만 변수명은 카멜케이스로 하는게 객체에 접근할 때 대괄호 접근자를 안 써도 돼서 장기적인 가독성 측면에서 더 좋을 것 같네요
| renderProductsList(); | ||
| renderChange(); | ||
|
|
||
| document.querySelector('form').addEventListener('submit', handleInsertCharge); |
There was a problem hiding this comment.
저도 못한 부분이지만 돔을 선언하는 모듈이 있고 그 모듈 돔 변수들을 사용하거나 정민님처럼 $ 유틸함수를 사용하면 가독성이 더 좋아질 것 같네요
| const currentCharges = getValueFromLocalStorage('charges', []); | ||
| currentCharges.forEach(charge => { | ||
| const currentChargeAmount = chargeAmount; | ||
| const coinCount = Math.min(parseInt(currentChargeAmount / charge.coinValue), charge.quantity); |
| const currentCharges = getValueFromLocalStorage('charges', []); | ||
| currentCharges.forEach(charge => { | ||
| const currentChargeAmount = chargeAmount; | ||
| const coinCount = Math.min(parseInt(currentChargeAmount / charge.coinValue), charge.quantity); |
There was a problem hiding this comment.
파스인트는 원래 목적이 스트링 to 숫자인걸로 알고있는데 Math.floor를 쓰면 목적이 더 분명할 것 같네요
4 participants
버그가 개많지만 시간이 없어서 일단 올립니다😭