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
14 changes: 2 additions & 12 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,9 @@
<input id="addTodo" type="text" placeholder=" Add TODO">
<button id="plusButton">+</button>
</div>
<div class="todoCard">
<div class="todoElem">운동하기</div>
<input type="checkbox">
<div class="todoCards">
<!--여기에 todolist 추가-->
</div>
<div class="todoCard">
<div class="todoElem">밥먹기</div>
<input type="checkbox">
</div>
<div class="todoCard">
<div class="todoElem">공부하기</div>
<input type="checkbox">
</div>
<!-- 미구현 -->
</div>
</div>
</body>
Expand Down
66 changes: 60 additions & 6 deletions script.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,66 @@
const addTodo = document.getElementById('addTodo');
const plusButton = document.getElementById('plusButton');
const addTodoInput = document.getElementById('addTodo'); //add todo input
const plusButton = document.getElementById('plusButton'); //plusbutton press
const clockTarget = document.getElementById("clock-js"); //clock

plusButton.addEventListener('click',()=>{
console.log("clicked");
})
function addTodo(){
const todoText = addTodoInput.value.trim();

Choose a reason for hiding this comment

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

trim 메서드를 사용하여 공백 입력 방지 하는 부분 인상깊네요 ㅎㅎ

if(todoText==""){
alert("TODO를 입력하세요.");
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 > 80){
alert("80자 이하로 입력해주세요.");
return;
}
//todo card 추가
const todoCards= document.querySelector(".todoCards");
const todoCard = document.createElement("div");
todoCard.classList.add("todoCard");

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

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

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

todoCard.appendChild(todoElem);
todoCard.appendChild(checkbox);
todoCard.appendChild(todoDelete);
Comment on lines +31 to +33
Copy link
Member

Choose a reason for hiding this comment

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

여러 개의 child를 append해야하는 경우 append()메서드를 사용하여

Suggested change
todoCard.appendChild(todoElem);
todoCard.appendChild(checkbox);
todoCard.appendChild(todoDelete);
todoCard.append(todoElem, checkbox, todoDelete);

와 같이 한줄로도 작성할 수 있습니다~!

Copy link
Author

Choose a reason for hiding this comment

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

코드를 훨씬 간결하게 작성할 수 있겠네요! 다음번엔 적용시켜서 작성하겠습니다!


todoCards.appendChild(todoCard);

addTodoInput.value="";
}
function handleClick(event){//click 핸들러
const clickedElement = event.target;
if(clickedElement.classList.contains("todoDelete")){
deleteTodo(clickedElement);
Copy link

Choose a reason for hiding this comment

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

handleClick 함수에 요소의 클래스 이름 여부에 따라 수행하는 동작을 나누는 방법 참신한거 같아요!!!

Choose a reason for hiding this comment

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

clickedElement가 특정 클래스명을 포함하느냐에 따라 반복문을 구성하셨는데, 클래스 이름 덕분에 굉장히 직관적으로 기능에 따른 할 일이 구분되어져있는 것 같네요~!

}
else if(clickedElement.classList.contains("checkbox")){
//체크박스 시 밑으로 내리기(local로 처리하든 정렬해야될듯)
}
}
function deleteTodo(deleteButton){//Todo 지움
const todoCard = deleteButton.parentElement;
const todoCards = document.querySelector(".todoCards");
todoCards.removeChild(todoCard);
}
//plusButton과 add Todo enter 클릭 이벤트 핸들러
plusButton.addEventListener('click',addTodo);
addTodoInput.addEventListener("keyup", (e)=>{
if(e.key === "Enter"){
addTodo();
}
})
Comment on lines +135 to +139
Copy link
Member

Choose a reason for hiding this comment

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

이 부분은 확실하진 않은데, 한글을 입력한 후 엔터키를 통해 todo를 등록하려 하면
엔터키가 두번 눌린 것으로 인식되는 에러가 있는 것 같아요.
아래 제가 해당 현상을 해결할 때 사용했던 참고자료 링크로 달아놓을게요 참고해보시면 좋을 것 같습니다.
참고자료

Copy link
Author

Choose a reason for hiding this comment

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

참고자료 다 읽어보았습니다! 해당 에러가 발생하는 줄 몰랐는데 앞으로 keyup/keydown 한글 입력 시에 링크 방법을 적용해서 구현해야겠습니다 ㅎㅎ

//deleteButton 이벤트핸들러
const todoCards = document.querySelector(".todoCards");
todoCards.addEventListener("click",handleClick);
// clock 출력
const clockTarget = document.getElementById("clock-js");
function clock(){
const date = new Date();
const month = date.getMonth();
Copy link
Member

Choose a reason for hiding this comment

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

date객체에서 getMonth를 통해 달을 가져올 때

Suggested change
const month = date.getMonth();
const month = date.getMonth() + 1;

을 통해 변수의 초기값 자체를 고친 값으로 수정해주면 변수를 재사용할 때 좀더 용이하지 않을까 싶네요!

Expand Down
16 changes: 14 additions & 2 deletions style.css
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ input[type=text]{/*input 태그 따로 설정*/
color : black;
display:flex; /*todoElem 세로축 가운데 정렬*/
align-items:center;
justify-content:space-between;/*item 사이 간격 균일하게*/
justify-content:space-between;
/* item 사이 간격 균일하게 */
height: 100px;
margin : 5px 0;
border-radius: 8px;
Expand All @@ -77,5 +78,16 @@ input[type=text]{/*input 태그 따로 설정*/
padding-left:30px;
}
input[type=checkbox]{
margin-right: 25px;
margin-right: -300px;
}
.todoDelete{
border: none;
margin-bottom: 80px;
margin-right: 5px;
font-size: 7px;
border-radius: 2px;
transition : background-color 0.3s;

Choose a reason for hiding this comment

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

transition 디테일 좋습니다 😊

}
Comment on lines +89 to +96
Copy link
Member

Choose a reason for hiding this comment

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

이 부분에서 x 버튼을 flex를 통해 잡아주셨는데,
부모 component에 position: relative;를 달아주고
todoDelete에 position: absolute; 를 통해 부모의 범위 안에서 right, top값의 조절을 통해 달아주어도 괜찮을 것 같다는 의견입니다!

Copy link
Author

Choose a reason for hiding this comment

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

x버튼을 일관된 위치에 지정하는 방법을 찾다가 해결책으로 flex-grow를 사용하였는데, position을 활용하면 훨씬 간단한 문제였네요 ㅠㅠ 감사합니다!!

.todoDelete:hover{
background-color: #b1b1b1;
}