Skip to content
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주차] 이규호 미션 제출합니다. #8

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@
</div>
<div class="todoList">
<div class="todoCards">
<!--여기에 todolist 추가-->
<!--여기에 todo 추가-->
</div>
</div>
<!-- <div class="todoList"> //done list로 구분
<div class="todoCards">

<div class="doneCards">
<!--여기에 done 추가-->
</div>
</div> -->
</div>
</div>
</body>
<script src="script.js"></script>
Expand Down
94 changes: 79 additions & 15 deletions script.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function addTodo(){
return;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용자의 입력이 빈 배열이거나 80자가 넘을 시 alert 띄워주는 부분을 addTodo 함수로 넘기기 전에 eventListener안에서 처리를 해주면 Todo를 추가해야하는 함수인 addTodo 함수의 기능이 좀 더 잘 분리되지 않을까 생각합니다 ㅎㅎ.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eventListner에서 예외처리를 해주는게 함수 기능에 더 맞은 구현이겠네요!! 감사합니다!

else if(todoText.length > 60){
alert("80자 이하로 입력해주세요.");
alert("60자 이하로 입력해주세요.");
return;
}
//todo card 추가
Expand All @@ -33,9 +33,62 @@ function addTodo(){
todoCard.appendChild(todoDelete);

todoCards.appendChild(todoCard);

saveTodos();
addTodoInput.value="";
}
function saveTodos(){
const todoCards = document.querySelectorAll(".todoCard");
const todos = [];
todoCards.forEach((todoCard)=>{
const todoElem = todoCard.querySelector(".todoElem").textContent;
const isChecked = todoCard.querySelector("input[type='checkbox']").checked;
todos.push({text: todoElem , checked : isChecked});
}
)
localStorage.setItem("data",JSON.stringify(todos));

}
Comment on lines +39 to +50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 함수에서 사용하는 todoCards는 함수가 실행될 때 마다 새로 만들어서 관리하는 것 보다
전역변수로 사용하면서 필요시 추가 / 삭제를 하는것이 조금 더 효율적이지 않을까 싶어요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saveTodos가 불릴 때마다 배열을 새로 생성하는 것보다 전역변수를 사용하는게 훨씬 효율적이겠네요😥 감사합니다 ㅎㅎ

function loadTodos(){
const localTodos = localStorage.getItem("data");
if(localTodos){
const todos = JSON.parse(localTodos);
todos.forEach((todo)=>{
const todoCard = document.createElement("div");
todoCard.classList.add("todoCard");

const todoElem = document.createElement("div");
todoElem.classList.add("todoElem");
todoElem.textContent = todo.text;

const checkbox = document.createElement("input");
checkbox.type = "checkbox";
checkbox.checked = todo.checked;

const todoDelete = document.createElement("button");
todoDelete.classList.add("todoDelete");
todoDelete.textContent = "X";

todoCard.appendChild(todoElem);
todoCard.appendChild(checkbox);
todoCard.appendChild(todoDelete);
if(todo.checked){
const doneCards= document.querySelector(".doneCards");
doneCards.appendChild(todoCard)
todoCard.classList.add("checked");
todoElem.style.textDecoration = "line-through";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

취소선 디테일 👍👍👍

}
else{
const todoCards= document.querySelector(".todoCards");
todoCards.appendChild(todoCard)
todoElem.style.textDecoration = "none";
}
})
}
else{//localStorage가 없는경우
const localTodos = "[]";
localStorage.setItem("data",localTodos);
Comment on lines +88 to +89
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 조금 더 직관적으로 없을 경우에는 빈 배열을 넣는다라고 하기 위해

Suggested change
const localTodos = "[]";
localStorage.setItem("data",localTodos);
localStorage.setItem("data",'[]');

와 적는것은 어떠한지 개인적인 의견을 드려봅니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

훨씬 직관적인거 같습니다! 앞으로 코드 작성할 때 이렇게 구현해보겠습니다 ㅎㅎ

}
}
function handleClick(event){//click 핸들러
const clickedElement = event.target; // click event element
if(clickedElement.classList.contains("todoDelete")){
Expand All @@ -47,27 +100,35 @@ function handleClick(event){//click 핸들러
}
function deleteTodo(deleteButton){//Todo 지움
const todoCard = deleteButton.parentElement;
const todoCards = document.querySelector(".todoCards");
todoCards.removeChild(todoCard);
const Cards = todoCard.parentElement;
if(todoCard.checked){//done에서 지우기
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변수의 시작은 소문자로 통일해 주는게 좋을 거 같아요 ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lowerCamelCase를 잘 지켜야겠네요! 감사합니다 ㅎㅎ

// const doneCards = document.querySelector(".doneCards"); 오류 발생
Cards.removeChild(todoCard);
}
else{
// const todoCards = document.querySelector(".todoCards");
Cards.removeChild(todoCard);
}
saveTodos();
}
function handleCheckBoxClick(checkedElement){
const todoCard = checkedElement.parentElement;
const todoElem = todoCard.querySelector(".todoElem");
if(checkedElement.checked){ // checked opacity는 css
todoCard.classList.add("checked");
todoCard.style.order = "2"; //체크된 항목 아래로
if(checkedElement.checked){//체크되면 done으로 옮기기
const doneCards = document.querySelector(".doneCards");
todoCards.removeChild(todoCard);
doneCards.appendChild(todoCard);//doneCards로 붙이기
todoCard.classList.add("checked");//checked로 바꾸기
todoElem.style.textDecoration= "line-through"; //취소선 긋기
}
else{
else{//체크 풀리면 todo 으로 옮기기
const todoCards = document.querySelector(".todoCards");
doneCards.removeChild(todoCard);
todoCards.appendChild(todoCard);
todoCard.classList.remove("checked");
todoCard.style.order = "1"; //체크되지 않은 항목 위로
todoElem.style.textDecoration= "none";
}
const todoList=Array.from(todoCards.children); // list를 통해 sort, checked와 아닌거 구분
todoList.sort((a,b)=> a.style.order - b.style.order);
todoList.forEach((todo)=>{
todoCards.appendChild(todo);
})
saveTodos();
};
//plusButton과 add Todo enter 클릭 이벤트 핸들러
plusButton.addEventListener('click',addTodo);
Expand All @@ -76,9 +137,11 @@ addTodoInput.addEventListener("keyup", (e)=>{
addTodo();
}
})
//deleteButton 이벤트핸들러
//deleteButton,checkbox click 이벤트핸들러
const todoCards = document.querySelector(".todoCards");
const doneCards = document.querySelector(".doneCards");
todoCards.addEventListener("click",handleClick);
doneCards.addEventListener("click",handleClick);
// clock 출력
function clock(){
const date = new Date();
Expand All @@ -95,6 +158,7 @@ function clock(){
function init(){
clock();
setInterval(clock,1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

초단위의 rerendering을 위해 setInterval 사용하신 점 인상깊네요 👍

loadTodos();
}

init();
15 changes: 6 additions & 9 deletions style.css
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ body{
height: 150px;
width: 500px;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

px도 좋지만 특정 화면 뿐만이 아니라 모든 화면에서 서비스를 잘 이용할 수 있도록 media-query 및 rem 단위로 작업을 하시는 것도 좋은 선택이 될 것 같습니다~~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앞으로 rem 단위로 작업하는 습관을 들여야겠네요!!

align-items: center;
/* font-weight: 700; */
/*flex-wrap: wrap;*/
}
.logo{
font-size: 2em;
Expand Down Expand Up @@ -58,26 +56,25 @@ input[type=text]{/*input 태그 따로 설정*/
border-radius: 6px;
width: 35px;
margin-left: 5px;
transition : border 0.3s;
transition : border 0.3s, color 0.3s;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transition을 통한 animation효과도 완성도를 높이는데 한 몫 한 것 같아요!! 🔥🔥🔥🔥

}
#plusButton:hover{
border : solid 2px #5f5f5f;
color : #5f5f5f;
}
.todoCard{
background-color: #ECECEC;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자주 쓰이는 색상 코드의 경우 사용자 정의변수로 등록하여 var(--)함수를 통해 직관적인 변수를 통해 이용할 수도 있을 것 같아요~!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반복되는 색상을 등록하여 사용하는게 훨씬 가독성이 좋겠네요!

color : black;
display:flex;
/*todoElem 세로축 가운데 정렬*/
display:flex; /*todoElem 세로축 가운데 정렬*/
align-items:center;
justify-content:space-between;
/* item 사이 간격 균일하게 */
justify-content:space-between;/* item 사이 간격 균일하게 */
height: 100px;
margin : 10px 0;
margin : 7px 0;
border-radius: 8px;
}
.todoCard.checked{
transition : opacity 0.3s;
opacity: 0.5;
transition: opacity 0.3s;
}
.todoCard.checked:hover{ /*마우스 올렸을 때 살짝 밝게*/
opacity: 0.6;
Expand Down