-
Notifications
You must be signed in to change notification settings - Fork 0
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
[1주차 심화 + 2주차 기본/심화] 다현이의 가계부 #4
base: main
Are you sure you want to change the base?
The head ref may contain hidden characters: "week2-\uAC00\uACC4\uBD80"
Conversation
2주차 심화까지 고생많았어 다현아. 나는 9일인가 걸린걸 13시간만에 하다니 리스펙할게 ㅋㅋㅋ 나도 막 찾아보면서 너 코드 읽으니까 많이 배우게 된 것 같아서 고마워 :) 내가 하는 리뷰가 얼마나 도움이 될지 모르겠지만, 많은 도움이 되길 바래!!
근데 내가 남긴 댓글은 왜 카운팅이 안되지..? file changed가서 리뷰 남기는거 아닌감 |
const newPrice = document.querySelector(".new_price"); | ||
const newContent = document.querySelector(".new_content"); | ||
const YesCancel = document.querySelector(".cancel_yes"); | ||
const NoCancel = document.querySelector(".cancel_no"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
얘네들 따로 빼서 관리해보자!! 아니면 함수 내에서 선언해도 좋을듯 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
왜 내 코드리뷰가 안보일까 흑
const addModal = document.querySelector(".add_modal"); | ||
let transactions = []; | ||
let transaction = { category: "", content: "", price: null }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 price값을 0말고 null을 준 이유가 있을까??
return number.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ","); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍👍👍👍👍👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나는 정규식도 좋지만 메소드가 더 직관적이고 가독성이 좋다고 생각해서,
메소드로 대체할 수 있는 부분은 정규식보단 메소드를 활용해주는 편이야!
다현이가 더 잘 활용할 수 있는 방법으로 통일해서 구현해줘도 좋겠다 ..!
modalBackground.classList.add("modal-background"); | ||
|
||
const addButton = document.querySelector(".add_list"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전반적으로 전역변수들이 많은 것 같아! 얘네들 함수 안으로 집어넣는게 좋을 것 같고, 아니면 예를들어 constants 폴더 안에 constants.js에 모두 집어넣고 가져다 쓰는 방식도 생각해보면 좋을듯!
.map( | ||
(category) => ` | ||
<option value="${category}">${category}</option> | ||
` | ||
) | ||
.join(""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와우 👍👍👍👍
incomeModalButton.style.background = "salmon"; | ||
outcomeModalButton.style.background = "white"; | ||
// 로컬 스토리지에서 수입 카테고리를 읽어옵니다. | ||
const categories = JSON.parse(localStorage.getItem("categories")); | ||
const incomeCategories = categories ? categories.income : []; | ||
categorySelect.innerHTML = ""; | ||
|
||
// 드롭다운을 업데이트합니다. | ||
categorySelect.innerHTML = incomeCategories | ||
.map( | ||
(category) => ` | ||
<option value="${category}">${category}</option> | ||
` | ||
) | ||
.join(""); | ||
|
||
console.log(categorySelect); | ||
} | ||
function modal_outcome_button() { | ||
incomeModalButton.style.background = "white"; | ||
outcomeModalButton.style.background = "salmon"; | ||
const categories = JSON.parse(localStorage.getItem("categories")); | ||
const outcomeCategories = categories ? categories.outcome : []; | ||
categorySelect.innerHTML = ""; | ||
|
||
// 드롭다운을 업데이트합니다. | ||
categorySelect.innerHTML = outcomeCategories | ||
.map( | ||
(category) => ` | ||
<option value="${category}">${category}</option> | ||
` | ||
) | ||
.join(""); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 두 함수가 로직이 많이 겹치는 것 같은데, 하나로 묶어도 괜찮을 거 같고, 겹치는 로직을 따로 함수로 만들어서 더 간결하게 해도 좋을듯행
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어라 윗 부분 짤렸네
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단위 수정하는거 은근 까다로웠을텐데 최대한 리뷰 반영하려고 한 게 보인다 ..!!!
프리티어 설치까지 ...!! 너무너무 수고했어 💜
전체적으로 사용하지 않는 주석, 콘솔 지워주고 여러 번 사용되거나 공통 함수로 뺄 수 있는 부분은 함수로 분리해주면 더더 좋을 것 같아 !!
저번 주차 심화과제부터 이번 주차 과제까지 진짜 너무너무 고생했다잉 !!! 🤩
<div class="list-header-2"> | ||
</div> | ||
<div class="list-header-1">< 10월 1일 ></div> | ||
<div class="cancel_modal"> | ||
삭제하시겠습니까? | ||
<button class="cancel_yes">예</button> | ||
<button class="cancel_no">아니오</button> | ||
</div> | ||
<div class="list-header-2"> | ||
<div class="title">내역 리스트</div> | ||
<div class="button"> | ||
<input type="checkbox" checked>수입</input> <!-- checked로 default --> | ||
</div> | ||
<div class="button"> | ||
<input type="checkbox" checked>지출</input> | ||
</div> | ||
<input type="checkbox" id="incomeCheckbox" value="income" checked /> | ||
<label for="incomeCheckbox">수입</label> | ||
</div> | ||
<div class="button"> | ||
<input type="checkbox" id="outcomeCheckbox" value="outcome" checked /> | ||
<label for="outcomeCheckbox">지출</label> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다현이도 전체적으로 클래스명 짓는 방식이 혼합되어 있는 것 같은데,
가독성이나 유지보수를 위해서라도 하나로 통일해주면 좋을 것 같아 !
특히 css에서는 bem
방식으로 class나 id 명을 짓는게 좋다고 해!
최근에 계속 react를 사용하다보니 class를 따로 지정해줄 일이 없어서 생각하지 못하고 있던 부분인데,
bem 방식을 활용하면 CSS를 쉽게 읽고, 쉽게 이해하고, 확장하기에 용이하다.
고 하더라구!
이 자료 참고해보면 이해하기 쉬울 것 같아!
<section class="each_category"> | ||
<header class="category_header">수입 카테고리</header> | ||
<div class="category_box"> | ||
<div class="category_block" id="incomeCategories"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기 보면 class를 지정해줬는데, id를 한 번 더 지정해준 이유가 뭘까용!?
1주차/4.가계부/script.js
Outdated
// // 쉼표를 추가한 형식으로 값을 업데이트합니다. | ||
// if (!isNaN(newPrice.value) || newPrice.value == "") { | ||
// let inputValue = newPrice.value; | ||
// inputValue = parseInt(inputValue.replace(/,/g, "")); | ||
// newPrice.value = formatWithCommas(inputValue); | ||
// } else { | ||
// newPrice.value = ""; | ||
// } | ||
// }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 가독성을 위해서 불필요한 주석은 삭제하는게 좋다고 생각해!
그치만 나중을 위해 일부러
남겨둔 주석이라면 한 줄 정도의 설명을 추가해보는건 어떨까?!
이런 저런 자료를 찾아보다 발견한 예시인데, 가독성에도 큰 영향을 미치지 않고
나중에도 찾기 편할 것 같아서 이런 식으로 남겨둬도 괜찮을 것 같다는 생각이 든다!
/*
* XX와 동일한 기능을 제공함으로 해당 메소드는 주석처리함.
*
public JSONObject getSessionDetailJson(int sessionId) throws SQLException {
String sql = "select * from session where session_id = ?";
...
return session;
}
*/
return number.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ","); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나는 정규식도 좋지만 메소드가 더 직관적이고 가독성이 좋다고 생각해서,
메소드로 대체할 수 있는 부분은 정규식보단 메소드를 활용해주는 편이야!
다현이가 더 잘 활용할 수 있는 방법으로 통일해서 구현해줘도 좋겠다 ..!
content: content, | ||
}; | ||
console.log(transaction); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트나 디버깅용으로 쓰이는 콘솔이 실 서비스에서는 메모리 릭/ 메모리 누수
를 유발할 수 있다고 해..!
memory leak: 개발자가 의도하지 않은 메모리를 점유하고 있는 현상
더이상 사용하지 않는 콘솔들은 지워주기로 하자 !!
function show_list(history_list) { | ||
initial_list.innerHTML = ""; // 이전 내역 지우기 | ||
total_money = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 그러게 정안오빠가 말해준 것처럼 income, outcome은 0으로 초기화해줬는데
total_money는 왜 null로 초기화한건지 궁금하다 !
const NoCancel = document.querySelector(".cancel_no"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오잉 다른 애들은 camelCase로 선언해줬는데, 얘네만 왜 PascalCase로 선언해준건지 궁금해 !
total_income -= removeItem.price; | ||
} | ||
if (removeItem.price < 0) { | ||
total_outcome -= removeItem.price; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기는 3항 연산자를 활용해서 이렇게 수정해볼 수 있겠다!
if (removeItem.price > 0) { | |
total_income -= removeItem.price; | |
} | |
if (removeItem.price < 0) { | |
total_outcome -= removeItem.price; | |
} | |
removeItem.price > 0 ? total_income -= removeItem.price : total_outcome -= removeItem.price; |
income.textContent = `+${total_income.toLocaleString()}`; | ||
outcome.textContent = `${total_outcome.toLocaleString()}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분이 계속 반복되는 것 같은데 공통함수로 묶어주고 필요할 때마다 호출해서 사용하는게
중복코드도 없애고 유지보수에도 좋을 것 같당 !
item.price < 0 | ||
? `${item.price.toLocaleString()}원` | ||
: `+${item.price.toLocaleString()}원`; | ||
if (item.price < 0) { | ||
price_li.style.color = "blue"; | ||
total_outcome += item.price; | ||
// console.log(item.price, "가격"); | ||
} else { | ||
price_li.style.color = "red"; | ||
total_income += item.price; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
item.price
에 관한 조건 처리가 조금 겹치는 부분이 있는 것 같아서 ..! 이런 식으로 수정해봤어!
price_li.textContent = | |
item.price < 0 | |
? `${item.price.toLocaleString()}원` | |
: `+${item.price.toLocaleString()}원`; | |
if (item.price < 0) { | |
price_li.style.color = "blue"; | |
total_outcome += item.price; | |
// console.log(item.price, "가격"); | |
} else { | |
price_li.style.color = "red"; | |
total_income += item.price; | |
} | |
if (item.price < 0) { | |
price_li.style.color = "blue"; | |
price_li.textContent = `${item.price.toLocaleString()}원` | |
total_outcome += item.price; | |
} else { | |
price_li.style.color = "red"; | |
price_li.textContent = `+${item.price.toLocaleString()}원`; | |
total_income += item.price; | |
} |
나는 중복 코드를 최대한 없애는걸 좋아하는 편이라 이렇게 수정해봤는데,
개인적인 의견이라 참고정도로 생각해도 좋을 것 같아 !
✨ 구현 기능 명세
🧩 2주차 기본 과제
최초 데이터
상수로
INIT_BALANCE
,HISTORY_LIST
데이터를 가집니다. (상수명은 자유)INIT_BALANCE
= 0HISTORY_LIST
: 입출금 내역 리스트 (4개
)최초 실행시 위 상수 데이터들 이용해 렌더링합니다. (즉, html에 직접 박아놓는 하드코딩 ❌)
→ 나의 자산은
INIT_BALANCE
로부터 4개의 입출금 내역 리스트를 반영하여 보여줍니다.총수입 / 총지출
HISTORY_LIST
에 있는 수입 내역과 지출 내역을 계산해서 총수입, 총지출을 보여줍니다.수입/지출 필터링
리스트 삭제
X
버튼을 누르면 해당 리스트가 삭제됩니다.리스트 추가
수입
지출
버튼카테고리
를 선택수입
을 선택하면 수입 관련된 항목들이,지출
을 선택하면 종류에 지출 관련된 항목들이 나옵니다.금액
과내용
입력 input저장하기
버튼닫기
버튼🧩 2주차 심화 과제
리스트 삭제 모달
x
버튼 클릭시 삭제 모달이 나타납니다.→
예
클릭시 삭제를 진행합니다.→
취소
클릭시 모달이 사라집니다.리스트 추가
alert
를 띄워 막습니다.alert
를 띄워 막습니다.모달 백그라운드 & 애니메이션 (삭제, 추가)
+
클릭시 추가 모달이 아래에서 위로 올라옵니다.카테고리 추가
Enter
키를 누르면 카테고리가 추가됩니다.금액
,
로 표시됩니다. (나의 자산, 총수입/지출, 내역 리스트, 리스트 추가 input)🧩 1주차 심화 과제
내역 내용
이 일정 길이보다 길어질 경우...
로 말줄임 처리됩니다.💎 PR Point
내역 리스트를 돌면서, 금액이 음수면 총지출에 반영, 금액이 양수면 총수입에 반영.
전체 금액은 총수입-총지출로 계산했습니다.
수입 버튼, 지출 버튼의 체크 여부를 변수로 할당하여, 만약 둘다 체크되어있다면 true를(전체 표시),
수입 버튼만 체크가 되어있다면 금액이 양수인 리스트만 반환, 지출인 경우도 같은 로직으로 구현했습니다.
리스트를 렌더링하면서, x버튼도 만들어줍니다.
x버튼을 클릭하면, this(x)의 parentNode(<ul class="list-indiv> ... )를 변수에 할당합니다.
listItem.remove()로 listItem 요소를 제거하고, 그 요소의 부모 노드로부터 분리시킵니다.
YesCancel.removeEventListener("click", handler);
은 리스트 삭제를 여러번 할때, 클릭된 요소가 중복되어 총지출/총수입의 계산이 어긋나는 것을 방지하기 위해 한번 삭제한 요소는 이벤트 리스너를 제거합니다.어두운 배경을 만듭니다.
그리고 모달을 열면,
display: block;
으로 변경시켜줍니다.모달은 어두운 배경에 가려지지 않아야 하므로,
z-index:999;
로 css를 설정하여 어두운 배경 위로 배치해줍니다.초기데이터를 설정해줍니다.
만약 아직 로컬 스토리지에 저장된 값이 없다면, 초기 카테고리 데이터를 로컬 스토리지에 설정. (key값은 categories)
데이터를 객체로 반환받기 위해 파싱해줍니다
categoryType은 "income"이나 "outcome"중 하나로 할당하여, 객체에 push해줍니다. 그리고 변경된 부분을 로컬스토리지에 업데이트합니다
인터넷에 찾아보니 두가지 방법이 있길래 둘 다 써봤습니당
toString().replace(/\B(?=(\d{3})+(?!\d))/g, ",");
페이지 이동을 어떤 식으로 구현해야할지 고민하다가, 결국에는 버튼 클릭 이벤트에 따라 요소를 보이게하고 숨기게 하는 식으로 구현하였습니다.
홈 페이지에 있을 때에는
<div class="home"></div>
의 display는 block<div class="category_page"></div>
의 display는 none. 카테고리 페이지에 있을 때에는 같은 로직을 이용해서 display를 반대로 설정했습니다.input에
display: none
설정하여 체크를 숨겼습니다.label을 사용하여 체크 버튼의 클릭 가능한 영역이 텍스트와 레이블 영역까지 확장되게 하였습니다. (웹접근성에도 도움이 된다고 합니당)
px 대신 rem이랑 %로 대체했습니다
🥺 소요 시간, 어려웠던 점
13~ 16h
🌈 구현 결과물
왼쪽은 클릭한 버튼, 오른쪽은 클릭하지 않은 버튼
Screen.Recording.2023-10-27.at.4.41.37.PM.mov
Screen.Recording.2023-10-27.at.5.07.18.PM.mov
Screen.Recording.2023-10-27.at.5.03.33.PM.mov
Screen.Recording.2023-10-27.at.5.12.21.PM.mov