이유진 - 자판기 온보딩 프로젝트#10
Conversation
| window.onload = function () { | ||
| attachEventListeners(); | ||
| }; |
There was a problem hiding this comment.
이 애플리케이션에서는 크게 상관없는데 JS를 추가할 때는 대체로 onload 보다는 DOMContentLoaded 이벤트가 더 선호됩니다. 혹시 모르고 계셨다면 왜 DOMContentLoaded가 더 선호되는지 찾아보시면 좋겠습니다. :)
There was a problem hiding this comment.
사실 DOMContentLoaded 가 더 선호되는지 모르고 있어서 찾아봤습니다!
DOMContentLoaded는 css나 이미지와 같은 외부 자원들이 로드되지 않아도 DOM이 준비되면 바로
실행되는 반면에 onload는 페이지에 포함된 모든 자원이 로드된 후에 실행되어서 페이지가 로드 완료
되는데 더 오랜 시간이 걸리기 때문에 onload를 사용해야 하는 경우가 아니면 DOMContentLoaded를 사용하는 게
더 효율적일 거 같습니다
이 애플리케이션에서는 css나 외부 자원들이 많지 않기 때문에 크게 상관없다 하신 말씀도 이해 갔습니다
감사합니다^^
수정 [b0d9bfe]
| attachEventListeners(); | ||
| }; | ||
|
|
||
| // 금액 포맷 함수: 항상 숫자만을 표기(양수만 입력가능),세 자리마다 쉼표를 표시합니다. |
There was a problem hiding this comment.
이왕 설명을 쓸거라면 코드 에디터의 자동완성 혜택을 누릴 수 있도록 JSDoc 형태로 작성해보면 어떨까요?
|
|
||
| // 투입, 반환 버튼 클릭할 때 | ||
| inputControls.addEventListener('click', function (event) { | ||
| if (event.target && event.target.matches('button.control-btn')) { |
There was a problem hiding this comment.
여기서는 event.target?.matches('button.control-btn')로 줄일 수 있습니다.
| // 투입, 반환 버튼 클릭할 때 | ||
| inputControls.addEventListener('click', function (event) { | ||
| if (event.target && event.target.matches('button.control-btn')) { | ||
| const action = event.target.getAttribute('data-action'); |
There was a problem hiding this comment.
데이터 속성에 접근할 때 getAttribute 사용하는 것이 익숙해서 사용했는데
이 코드에서는 dataset을 사용하는 게 더 편리하고 직관적인 방법 같아서 수정했습니다
감사합니다:)
수정[c805c57]
| const button = document.createElement('button'); | ||
| button.classList.add('product-button'); | ||
| button.setAttribute('data-price', product.price); | ||
|
|
||
| const nameDiv = document.createElement('div'); | ||
| nameDiv.classList.add('product-name'); | ||
| nameDiv.textContent = product.name; | ||
|
|
||
| const priceDiv = document.createElement('div'); | ||
| priceDiv.classList.add('product-price'); | ||
| priceDiv.textContent = `${formatAmount(product.price)}원`; | ||
|
|
||
| button.appendChild(nameDiv); | ||
| button.appendChild(priceDiv); |
There was a problem hiding this comment.
다른 분들께도 드렸던 피드백인데 DOM을 일일이 생성하는 것보다는 template 태그를 사용해보면 어떨까요?
There was a problem hiding this comment.
HTML에 template태그를 추가해서 기본 구조를 미리 정의해 두고 자바스크립트에서 복제해 동적으로 추가하는 방식으로
수정 했습니다 감사합니다:)
수정[409bd97]
| return ''; | ||
| } | ||
| // 금액을 세 자리마다 쉼표를 넣어서 포맷 | ||
| let formattedAmount = Number(amount).toLocaleString('en-US'); |
There was a problem hiding this comment.
const로 선언하는 게 더 적합한데 제가 let으로 잘못 선언한 거 같습니다ㅜㅜ
바로 수정 했습니다 감사합니다:)
수정[769e707]
| <!-- 투입 금액 표시 화면 --> | ||
| <div class="input-group"> | ||
| <div class="input-display"> | ||
| <input type="text" id="input-display" placeholder="0"></span> |
There was a problem hiding this comment.
type="number"를 사용하지 않은 이유가 있을까요?
There was a problem hiding this comment.
금액 투입 화면에서 숫자를 입력할 때 formatAmount() 함수가 입력된 값에 쉼표를 추가하여
반환을 하기 때문에 입력 필드의 타입을 text로 설정했습니다!
만약 타입을 number를 사용하면 쉼표 문자를 입력할 수 없기 때문에 제가 원하는 금액 투입 시
포맷을 자유롭게 처리하지 못하더라구요 ㅜㅜ
| const inputDisplay = document.getElementById('input-display'); // 금액 투입 박스 | ||
| const inputControls = document.querySelector('.input-controls'); // 투입,반환버튼 | ||
| const amountDisplay = document.getElementById('amount-display'); // 상품 금액 표시 화면 | ||
| const productButtonsContainer = document.querySelector('.product-buttons'); // 상품 버튼 컨테이너 | ||
| const logBox = document.querySelector('.log-box'); //로그 컨테이너 | ||
| const logList = document.getElementById('log-list'); //로그 리스트 박스 |
There was a problem hiding this comment.
테스트 가능성을 고려하여 인수로 애플리케이션 루트 엘리먼트를 받고, 루트 엘리먼트 내부에서 클래스로 검색해서 대상 엘리먼트를 가져오도록 작성해 볼 수 있을까요? 그리고 클래스로 검색한 대상 엘리먼트는 "없을 수도 있다"고 가정해봅시다.
2 participants
체크 리스트