[tip-calculator-app] ddori(조배경) 미션 제출합니다. #2
Conversation
There was a problem hiding this comment.
코드 잘 읽었습니다!
show와 event, 복잡한 함수를 잘 나눈 코드를 읽었습니다. CSS도 물론 겹치는 것은 잘 합쳐서 더 읽기 좋았습니다.
다만 아쉬운 점이 몇가지 있는데, 조금만 나열해보자면,
- CSS에서 %를 쓰는 것은 저는 매우 좋아합니다. 다만, %를 쓰기전에 부모의 CSS의 크기를 짐작 가능하도록 고정된 크기(ex. px, rem, vw)같은 것을 같이 곁들여 쓰면 좋습니다.
- 위의 내용에 연장선상에서 375px보다 작아지면 자유롭게 변할 수 있도록 px로 하는 것보다 %같은 단위를 사용하면 좋습니다.
이번 미션도 시간이 없어 힘들었을텐데 수고 하셨습니다.
| } | ||
| } | ||
|
|
||
| function clickedBtn(event) { |
There was a problem hiding this comment.
Button을 Click 한다는 것에서 직관적으로 받아들일 수 있어 좋은 의미인것 같습니다.
다만 clickedBtn은 변경된 button이라는 생각이 들어서 clickBtn같이 동사형을 쓰면 더욱 직관적인 코드가 될 것 같습니다.
| } | ||
|
|
||
| function showResult() { | ||
| decideZero(); |
There was a problem hiding this comment.
showResult()가 결과를 보여주는 부분인 것은 알기쉬워 좋았습니다.
다만 0으로 나누는 경우 거치지 않고 출력하게 되었습니다.
0으로 나누는 부분을 검사하여 decideZero()를 출력하고, return을 하면 더욱 깔끔한 코드가 될 것 같습니다.
| for (let i = 0; i < selectBtn.length; i++) { | ||
| selectBtn[i].addEventListener("click", (e) => clickedBtn(e)); | ||
| } |
There was a problem hiding this comment.
for로 순회하면서 selectBtn에 함수를 넣는 것이 좋았습니다.
다만, Array.prototpy을 이용하면 더 깔끔한 코드가 될 수 있습니다.
또한 addEventListener의 경우에는 this 바인딩이 가능하여 함수를 집어 넣어도 됩니다.
selectBtn.forEach((button) => {
button.addEventLisnter("click, clickedBtn);
}| .bill, | ||
| .number { | ||
| margin: 10px 0 20px 0; | ||
| border: none; | ||
| background-color: hsl(189, 41%, 97%); | ||
| color: hsl(184, 14%, 56%); | ||
| height: 35px; | ||
| text-align: right; | ||
| background-repeat: no-repeat; | ||
| background-position: 15px center; | ||
| font-size: 24px; | ||
| cursor: pointer; | ||
| } |
| </head> | ||
| <body> | ||
| <div class="top-content">SPLI</div> |
There was a problem hiding this comment.
아 어쩐지..저것도 svg 파일이 있는 줄 몰랐어요..ㅎㅎ
|
|
||
| .select-btn { | ||
| display: grid; | ||
| grid-template-columns: repeat(3, 1fr); |
| border-radius: 5px; | ||
| background-color: hsl(189, 41%, 97%); | ||
| color: hsl(183, 100%, 15%); | ||
| text-indent: -5%; |
| } | ||
|
|
||
| customTip.addEventListener("keyup", inputCustom); | ||
| peopleNum.addEventListener("keyup", showResult); |
There was a problem hiding this comment.
input이 여러 개 있으면 사람들은 대부분 위에서 아래로 입력하는 경향이 있지만, 아닌 경우를 고려해 어느 input이든 showResult가 이벤트 리스너로 있으면 좋을 것 같아요
| for (let i = 0; i < Object.keys(selectBtn).length; i++) { | ||
| selectBtn[i].classList.remove("clicked"); | ||
| } |
There was a problem hiding this comment.
Javascript에서 제공하는 api를 사용해봐도 좋을 것 같습니다!
| for (let i = 0; i < Object.keys(selectBtn).length; i++) { | |
| selectBtn[i].classList.remove("clicked"); | |
| } | |
| const selectBtnsKey = Object.keys(selectBtn) ; | |
| selectBtnsKey.forEach((element) => { | |
| // ... some logic | |
| }) |
| const tipAmount = document.querySelector(".tip-amount"); | ||
| const totalAmount = document.querySelector(".total-amount"); | ||
| const inputBill = document.querySelector(".bill"); | ||
| const selectBtn = document.querySelectorAll("button"); |
There was a problem hiding this comment.
배열이거나 컬렉션인 경우 변수명에 복수형을 의미하는 s를 붙여주면 좋을것 같습니다.
| const selectBtn = document.querySelectorAll("button"); | |
| const selectBtns = document.querySelectorAll("button"); |
이번 학기는 앱을 하다 보니 확실히 js 공부량이 줄어서 혼자 힘으로 미션을 해내기까지 시간이 꽤 걸렸던 것 같습니다. 이번 미션은 전에 페어 하면서 배웠던 것들이 떠올라서 혼자 적용해보는 시간이 되어 유익했습니다! 반응형을 혼자 짜는 게 처음이었는데, 이 방법이 맞나 잘 모르겠네요... 그리고 변수명, css에 대해 더 공부해야겠다고 느꼈습니다. 또 한 가지 느낀 점은 요구 사항을 명확히 하는 게 중요한 것 같습니다. 이번에는 잘 작성하진 못했는데, 코드를 작성하다 보니 필요한 요구사항들이 자꾸 생각나서 앞으론 잘 정리 해야겠다고 생각했습니다!