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주차] 김류원 미션 제출합니다. #10

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
Binary file added .DS_Store
Binary file not shown.
1 change: 1 addition & 0 deletions icon/NotCheck.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 8 additions & 0 deletions icon/check.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions icon/checkComplete.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 19 additions & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,25 @@
</head>

<body>
<div class="container"></div>
<div class="container">
<div class="header">✓ To Do</div>
<div class="todoContainer">
<h1 class="title">TO DO LIST</h1>
<p id="todayDate">9월 6일 금요일</p>

<!-- input container -->
<div class="WriteForm">
Copy link

Choose a reason for hiding this comment

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

WriteForm이나 TodoInput, check_icon에서 네이밍 컨벤션이 다른 부분이 보이는데, 혹시 의도하신걸까요?!

<form id="todoForm" class="writeTodoForm">
<img src="./icon/check.svg" class="check_icon">
<input type="text" class="TodoInput" placeholder="Add a To Do">
<button class="addBtn">Add</button>
</form>
</div>

<!-- todo list -->
<ul class="todoList"></ul>
</div>
</div>
</body>
<script src="script.js"></script>
</html>
83 changes: 82 additions & 1 deletion script.js
Original file line number Diff line number Diff line change
@@ -1 +1,82 @@
//😍CEOS 20기 프론트엔드 파이팅😍
const addBtn = document.querySelector('.addBtn');
const TodoInput = document.querySelector('.TodoInput');
Copy link

Choose a reason for hiding this comment

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

여기서도 TodoInput이나 Todo처럼 camelCase를 사용하지 않는 부분이 있어서, 따로 기준을 가지고 계신지 궁금해요!

const todoList = document.querySelector('.todoList');
const todoForm = document.querySelector('.writeTodoForm');
const todoIcon = document.querySelector('.todoForm img');


function addTodo(e) {
e.preventDefault();

const Todo = TodoInput.value.trim();

if (Todo) {
createTodoElement(Todo);
TodoInput.value = '';
} else {
alert('To Do를 입력하세요');
}
}

todoForm.addEventListener('submit', addTodo);
Copy link

Choose a reason for hiding this comment

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

submit 이벤트를 이용해주셔서 enter로도 todo를 추가할 수 있어서 편리했어요👍👍👍


//투두 추가 함수
function createTodoElement(Todo) {
const listItem = document.createElement('li');

// 완료 토글 아이콘
const toggleIcon = document.createElement('img');
toggleIcon.src = './icon/notCheck.svg';
toggleIcon.alt = 'Toggle unComplete';
toggleIcon.classList.add('toggle-icon');

toggleIcon.addEventListener('click', () => {
listItem.classList.toggle('completed');
todoText.classList.toggle('completed-text');
if (listItem.classList.contains('completed')) {
toggleIcon.src = './icon/checkComplete.svg';
toggleIcon.alt = 'Toggle Complete';
} else {
toggleIcon.src = './icon/notCheck.svg';
toggleIcon.alt = 'Toggle unComplete';
}
});

// 투두 텍스트
const todoText = document.createElement('span');
todoText.textContent = Todo;

// 삭제 버튼
const deleteBtn = document.createElement('button');
deleteBtn.textContent = '삭제';
deleteBtn.classList.add('delete-btn');
deleteBtn.addEventListener('click', () => {
todoList.removeChild(listItem);
});

// HTML 구조에 맞게 요소 추가
Copy link
Collaborator

Choose a reason for hiding this comment

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

다혜님이 말씀해주신것처럼, appendchild 메서드로 요소를 하나씩 추가하는 것보다 한번에 추가하는 방법을 사용하는게 최적화 측면에서도 좋아보여요!
appendChild가 호출될 때마다 브라우저가 DOM을 업데이트하고, 리플로우와 리페인팅 발생 가능성이 있어요. 때문에, 성능이 저하가 될 수 있어 가급적 dom 조작을 최소화하는 방식을 취하는 것이 좋아보여요.

가빈께 남긴 코드리뷰중에, 리플로우와 리페인팅에 대한 설명을 참고해보시면 좋을 것 같아요!
#4 (comment)

listItem.appendChild(toggleIcon);
listItem.appendChild(todoText);
listItem.appendChild(deleteBtn);
Comment on lines +94 to +96
Copy link

Choose a reason for hiding this comment

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

혹시 listItem.append(toggleIcon, todoText, deleteBtn);처럼 element를 한번에 추가할 수 있다는 걸 알고 계신가용?! 저도 이번 과제에서 처음 알았는데 유용한 것 같아서 공유드립니다!
https://developer.mozilla.org/en-US/docs/Web/API/Element/append 에서 더 자세히 확인하실 수 있어요🫶🫶

todoList.appendChild(listItem);
}


// 날짜 표시 함수
function formatDateKorean(date) {
const days = ['일요일', '월요일', '화요일', '수요일', '목요일', '금요일', '토요일'];
const months = ['1월', '2월', '3월', '4월', '5월', '6월', '7월', '8월', '9월', '10월', '11월', '12월'];
Comment on lines +102 to +103
Copy link

Choose a reason for hiding this comment

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

이렇게 상수를 선언해 주신 점 좋습니다👍
그런데 JS에서 이런 상수를 선언할 때는 UPPER_SNAKE_CASE로 선언하는 방법이 더 널리 쓰인다고 알고 있어요! 한번 찾아보시면 좋을 것 같아요!!


const month = months[date.getMonth()];
const day = date.getDate();
const dayOfWeek = days[date.getDay()];

return `${month} ${day}일 ${dayOfWeek}`;
}
Comment on lines +101 to +110

Choose a reason for hiding this comment

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

month표현도 day처럼 표현한다면, months라는 변수를 사용하지 않아도 되니까 더 간결하고 일관된 코드가 될 수 있을 것 같습니다!


// 페이지 로드 시 오늘 날짜 표시
document.addEventListener('DOMContentLoaded', () => {
const todayDateElement = document.getElementById('todayDate');
const today = new Date();
todayDateElement.textContent = formatDateKorean(today);
});
45 changes: 44 additions & 1 deletion style.css
Original file line number Diff line number Diff line change
@@ -1 +1,44 @@
/* 본인의 디자인 감각을 최대한 발휘해주세요! */
* {
margin: 0;
padding: 0;
box-sizing: border-box;
}

li {
list-style: none;
display: flex;
align-items: center;
}

/**/

.todoContainer {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pasted Graphic 9

류원님의 투투리스트에서 항목이 height을 초과하는 경우에, 컨테이너 밖으로 삐져나오게 되는 것 같아요.

이때 css의 overflow 속성을 사용하면, 컨텐츠가 컨테이너를 초과할 때 어떻게 처리할지 설정해줄 수 있을 것 같아요!!
https://developer.mozilla.org/ko/docs/Web/CSS/overflow
류원님 코드에 overflow-y: auto; 속성 하면, 항목이 넘치면 아래와 같이 세로로 스크롤 되도록 처리할 수 있네요!!
image

background-color: #f1f3ff;}
header{
padding-left: 45px;

}
Copy link

Choose a reason for hiding this comment

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

스크린샷 2024-09-09 오후 3 12 08
overflow 처리가 안되어 있어서 todolist 목록이 많아졌을 때,
컨테이너 외부로 넘치고 있어요!
max-height, overflow 속성 적용하면 더 좋을 것 같아요 🤩

Suggested change
}
max-height: 70vh;
overflow-y: auto;
}

overflow docs
😃😃



.completed-text {
text-decoration: line-through;
color: #888;
}

.toggle-icon, .delete-icon {
width: 20px;
height: 20px;
cursor: pointer;
}

.toggle-icon {
margin-right: 10px;
}

.delete-icon {
margin-left: auto;
}

span {
flex-grow: 1;
}