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.

Sign up for GitHub

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

Jump to bottom

[1주차] 신동현 미션 제출합니다 #9

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

[1주차] 신동현 미션 제출합니다 #9

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f9d344d
기본적 html 구조
dhshin98 Sep 12, 2023
eed56d2
feat: todo list add, delete 기본 틀
dhshin98 Sep 15, 2023
7786bda
feat: 추가, 삭제 버튼 구체화
dhshin98 Sep 15, 2023
877d2e2
feat: 리스트 개수 세는 함수
dhshin98 Sep 15, 2023
89faf89
feat: local storage 저장
dhshin98 Sep 15, 2023
7e12519
feat: 오늘 날짜 추가
dhshin98 Sep 15, 2023
5889c51
feat: input null 예외처리
dhshin98 Sep 15, 2023
9352968
fix: css 수정
dhshin98 Sep 15, 2023
43a6a30
fix: css 수정
dhshin98 Sep 15, 2023
d36548e
fix: css 수정2
dhshin98 Sep 15, 2023
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
13 changes: 9 additions & 4 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,25 @@

<body>
<div class="container">
<div class = "header">
<div class = "round Red" ></div>
<div class = "round Yellow" ></div>
<div class = "round Green " ></div>
Comment on lines +13 to +15
Copy link
Member

Choose a reason for hiding this comment

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

어디서 많이 보던 코드인데.....

</div>
<h1>Todo List</h1>
<div class ="todo_input">
<div class ="todoInput">
<input type = "text" id="inputTodo" placeholder="📍 Enter your to-do">
Copy link

Choose a reason for hiding this comment

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

엔터를 쳤을 때, todolist에 반영이 안되어서 해당 input의 placeholder 텍스트를 바꾸어도 좋을 것 같아요!

<button> + </button>
<button class ="addBtn"> + </button>
</div>

<div class = "todoContainer">
<h2>List To Do</h2>
<div id = "todoList">과제</div>
<div id = "todoList"></div>
</div>

<div class = "doneContainer">
<h2>List Done</h2>
<div id = "doneList">수강신청</div>
<div id = "doneList"></div>
</div>

</div>
Expand Down
62 changes: 62 additions & 0 deletions script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
const inputTodo = document.querySelector("#inputTodo");
const todoItems = document.querySelector("#todoList");
const doneList = document.querySelector("#doneList");
const doneArray = document.querySelectorAll("#doneList .todoItem");
const addBtn = document.querySelector(".addBtn");
const todoTextArray = [];
const doneTextArray = [];

function createBtn(className, text) {
var button = document.createElement("div");

button.textContent = text;
button.setAttribute("class", "listBtns");
button.classList.add(className);
return button;
}

function deleteItem() {
var removeBtns = document.querySelectorAll(".deleteBtn");
for (i = 0; i < removeBtns.length; i++) {
removeBtns[i].addEventListener("click", function () {
if (this.parentNode)
this.parentNode.parentNode.removeChild(this.parentNode);
});
}
}

function todoToDone() {
var doneBtns = document.querySelectorAll(".doneBtn");

for (var i = 0; i < doneBtns.length; i++) {
doneBtns[i].addEventListener("click", function () {
if (this.parentNode.parentNode)
doneList.insertBefore(this.parentNode, doneList.childNodes[0]);
});
}
}

function newTodo() {
var newDiv = document.createElement("div");
var newText = document.createTextNode(inputTodo.value); // input 태그에 쓴 text 값
newDiv.appendChild(newText);
var doneBtn = createBtn("doneBtn", "✅");
var deleteBtn = createBtn("deleteBtn", "❌");
newDiv.appendChild(doneBtn);
newDiv.appendChild(deleteBtn);
newDiv.setAttribute("class", "todoItem");

todoItems.insertBefore(newDiv, todoItems.childNodes[0]);
inputTodo.value = "";
//deleteBtn 누르면 삭제 구현
deleteItem();

//doneBtn 누르면 todo -> done으로
todoToDone();
Copy link

Choose a reason for hiding this comment

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

현재 플로우가, 새로운 아이템을 추가하는 순간에 delete 버튼과 done 버튼에 event handler를 전달해주는 방식을 사용하신 것 같아요! 좋은 방법이긴 한데, 현재 deleteItem 함수와 todoToDone 함수에서는 지금 당장 추가하려는 항목의 버튼에만 event handler를 할당하는게 아니라, 각각의 class를 가진 모든 항목에 event handler를 할당고 있는 것 같아요, 그렇게 되면 각각의 버튼들에 event handler들이 쌓이게 되는 것 같아요!
스크린샷 2023-09-16 오전 3 50 18
위 사진을 보면, 총 3개의 todo를 추가했을 때, 가장 먼저 추가한 todo의 버튼들에 3개의 동일한 listener들이 할당되어 있는 것을 알 수 있어요!
그래서, 저는 새로운 항목을 추가할 때 해당 항목의 버튼에만 listener을 등록하는 방식을 사용했어요 한 번 생각해보시면 좋을 것 같아요~~

}

function init() {
addBtn.addEventListener("click", newTodo);
Copy link

Choose a reason for hiding this comment

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

  • 버튼 눌렀을 때만 todo에 추가되도록 해주셨는데, enter 눌렀을 때도 되게 해주시면 더 편할 것 같아요~!

Copy link
Member

Choose a reason for hiding this comment

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

이 부분은 form 태그를 사용하는 걸로 해결해볼 수 있을 것 같아요 😄

}

init();
51 changes: 48 additions & 3 deletions style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,46 @@ body{
align-items: center;
justify-content: center;
}


.container{
background-color: white;
width: 300px;
height: 600px;
height: 650px;
border-radius: 10px;
}
.todoContainer,
Copy link

Choose a reason for hiding this comment

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

스타일을 위해서 이상적인 높이를 지정하는 것은 좋은 것이라고 생각해요~~
다만, todo가 여러 개 들어갔을 때, 높이가 고정되어 있어 컨테이너 밖으로 내용이 넘치는 것을 볼 수 있어요! 이를 해결하려면,

  1. overflow-y: auto; 속성을 주어 자동으로 스크롤이 될 수 있도록 한다.
  2. min-height: 200px; 로 바꾸어 최소 높이를 지정하고 그 이후에는 맞춰서 자라도록 한다.
    정도의 방법이 있을 것 같아요~

Copy link
Author

Choose a reason for hiding this comment

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

저는 컨테이너 내용이 넘치는 부분은 생각하지 못했는데,, 꼼꼼한 코드리뷰 감사합니다.. 🙇‍♀️ 대균님 덕분에 좋은 정보 많이 얻고 많이 배웠어요!! ☺️

Copy link

Choose a reason for hiding this comment

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

CSS 단위 설정하실 때 픽셀(px) 단위가 아닌 rem, em 등을 사용해보시는 것도 좋을 것 같아요. REM이 반응형 디자인에서 유용하게 사용된다고 하더라구요! 저도 px 쓰는게 익숙한데 최대한 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 에 익숙해지도록 해야겠어요 :) 링크 감사합니당!!ㅎㅎ

.doneContainer {
flex: 1;
padding: 10px;
background-color: #f0f0f0;
margin: 10px;
height: 200px;
}
.header{
height: 15px;
width:100%;
display:flex;
}
.round{
width: 10px;
height: 10px;
border-radius: 50%;
margin:5px;
}
.round:hover{
cursor: pointer;
}
.Red {
background-color: #F5655B;
}

.round.Yellow {
background-color: #F6BD3B;
}

.Green {
background-color: #43C645;
}

.todo_input{
width:100%;
background-color: whitesmoke;
Expand All @@ -34,4 +66,17 @@ body{
padding-left: 10px;
background-color: rgb(233, 233, 233);

}
.deleteBtn{
Copy link

Choose a reason for hiding this comment

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

이건 스타일적인 부분이긴 한데, 만약 todo에 긴 문자열이 들어가게 되면, done, delete 버튼과 겹치게 되는데, 해당 버튼들을 absolute로 위치를 지정해주어서 그런 것 같아요! 만약, absolute로 지정하고 싶으면 그 옆에 있는 element에 최대 width를 설정해 주어서 겹치지 않게 하거나, 아니면 todoItem에 flex box를 적용했으니 absolute 말고 그냥 일반적으로 나열시키는 것도 좋을 것 같아요!
아마, 해당 버튼들을 오른쪽 끝에 배치하고 싶으셔서 absolute로 지정하신 것 같은데,

  1. text가 들어가는 부분을 div로 감싸고 flex-grow: 1; 속성을 준다.
  2. done button에 margin-left: auto; 속성을 준다.
    이 두 가지 방법으로 원하는 스타일링을 할 수 있을 거에요!

Copy link
Author

Choose a reason for hiding this comment

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

아 버튼을 오른쪽 끝에 어떻게 나열하나 고민했었는데, 저런 방법도 있었군요!! 덕분에 배웠습니당 👏

position:fixed;
right:280px;
}
.doneBtn{
position:fixed;
right:310px;
}
.todoItem{
display:flex;
justify-content: space-between;
width: 100%;
}