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주차] 박지수 미션 제출합니다. #11

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
47 changes: 35 additions & 12 deletions index.html
Original file line number Diff line number Diff line change
@@ -1,14 +1,37 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vanilla Todo</title>
<link rel="stylesheet" href="style.css" />
</head>

<body>
<div class="container"></div>
</body>
<html lang="ko">

<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>To Do</title>
<link rel="stylesheet" href="style.css">
</head>

<body>
<header>
<h1>✔ To Do</h1>
</header>

<main>
<section class="todo-container">

Choose a reason for hiding this comment

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

섹션 태그를 사용하셔서 코드 구조가 한눈에 보이니 좋네요! 항상 컨테이너 태그에만 신경을 쓰고 section 태그는 사용해보지 않았는데 이번 기회에 배워갑니다! 👍

<div class="todo-header">
<h2>오늘 할 일</h2>
<p id="currentDate"></p>
</div>

<ul id="todoList">
<!-- 할 일 목록이 여기에 추가됨 -->
</ul>

<div class="todo-input">
<input type="text" id="newTodo" placeholder="할 일 추가">
<button id="addButton">추가</button>
</div>
Comment on lines +27 to +30

Choose a reason for hiding this comment

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

클래스와 id 각각에 적절한 네이밍 규칙을 잘 지켜주신 점 정말 좋은 것 같습니다! 생각해보니 저는 대부분 클래스명만 사용했어서 id도 적절히 활용해야겠다는 생각이 듭니다😂

</section>
</main>

<script src="script.js"></script>
</html>
</body>

</html>
38 changes: 37 additions & 1 deletion script.js
Original file line number Diff line number Diff line change
@@ -1 +1,37 @@
//😍CEOS 20기 프론트엔드 파이팅😍
// 날짜 출력
const currentDateElement = document.getElementById('currentDate');
const today = new Date();
const days = ['일', '월', '화', '수', '목', '금', '토'];
currentDateElement.textContent = `${today.getMonth() + 1}월 ${today.getDate()}일 ${days[today.getDay()]}요일`;

// 할 일 추가 기능
const addButton = document.getElementById('addButton');
const newTodoInput = document.getElementById('newTodo');
const todoList = document.getElementById('todoList');

addButton.addEventListener('click', addTodo);

function addTodo() {
const newTodo = newTodoInput.value.trim();

Choose a reason for hiding this comment

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

trim() 메서드로 불필요한 공백문자를 제거하셔서 리스트가 깔끔하게 출력되니 좋네요! ✨

if (newTodo) {
const todoItem = document.createElement('li');
const textNode = document.createTextNode(newTodo);
const removeButton = document.createElement('button');
removeButton.textContent = '삭제';
removeButton.addEventListener('click', () => {
todoList.removeChild(todoItem);
});

todoItem.appendChild(textNode);
todoItem.appendChild(removeButton);
todoList.appendChild(todoItem);
Comment on lines +16 to +27

Choose a reason for hiding this comment

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

노드객체를 생성하고 DOM에 연결시켜주는 작업이 굉장히 깔끔해서 DOM 개념을 정확히 이해하고 쓰신 듯해요👍👍 특히 저는 innerText를 사용했었는데 코드리뷰하면서 textContent가 css에 영향을 받지 않는다는 점에서 더 효율적임을 알게 되었는데 이 부분도 잘 고려하신 것 같아 인상적입니다!

Comment on lines +25 to +27

Choose a reason for hiding this comment

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

다른 분들 코드리뷰 보면서 저도 알게 된 것인데요, appendChild로 동일 대상에 여러 번 노드 객체를 추가하고 있는 부분을 append로 한 번에 쓰면 더 간결할 것 같습니다!

todoItem.append(textNode, removeButton);


newTodoInput.value = '';
}
}
// 엔터키로 할 일 추가 가능
newTodoInput.addEventListener('keydown', (e) => {
if (e.key === 'Enter') {
addTodo();
}
});
Comment on lines +32 to +37

Choose a reason for hiding this comment

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

Screen Shot 2024-09-08 at 6 29 04 PM

배포 링크에서 투두 작성을 해보았는데 엔터 키로 투두를 추가할 경우, 위와 같이 앞선 입력의 마지막 글자가 같이 생성되는 문제를 발견했습니다!

마지막 문자가 특수문자인 경우에는 엔터가 한 번으로 인식되지만 일반문자일 경우 두 번으로 인식되는 걸 보니 아마 브라우저가 엔터의 기본 동작을 처리하고 있는 것 같아요. 저도 엔터키로 생성하는 기능을 구현했는데 엔터 키가 브라우저의 기본 동작으로 처리되어서 엔터가 두 번씩 인식된다고 하더라고요! 저는 이 글을 참고해서 e.preventDefault()로 기본 동작을 막도록 처리했습니다. 혹시 도움되실까 해서 링크 남깁니다!

Suggested change
// 엔터키로 할 일 추가 가능
newTodoInput.addEventListener('keydown', (e) => {
if (e.key === 'Enter') {
addTodo();
}
});
// 엔터키로 할 일 추가 가능
newTodoInput.addEventListener('keydown', (e) => {
if (e.key === 'Enter') {
e.preventDefault();
addTodo();
}
});

Copy link
Author

Choose a reason for hiding this comment

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

안녕하세요 가빈님!!
생각 못한 부분이었는데 참고글까지 주시고 정말 도움되는 리뷰네요🥹

81 changes: 80 additions & 1 deletion style.css
Original file line number Diff line number Diff line change
@@ -1 +1,80 @@
/* 본인의 디자인 감각을 최대한 발휘해주세요! */
* {
margin: 0;
padding: 0;
box-sizing: border-box;
}

body {
font-family: Arial, sans-serif;
background-color: #121212;
color: white;
display: flex;
justify-content: center;
align-items: center;
height: 100vh;
}

header {
position: absolute;
top: 0;
left: 0;
padding: 20px;
}

main {
width: 300px;
}

.todo-container {
background-color: #1f1f1f;
padding: 20px;
border-radius: 10px;
}

.todo-header {
margin-bottom: 20px;
}

.todo-header h2 {
margin-bottom: 5px;
}

#todoList {
list-style: none;
padding: 0;
margin-bottom: 20px;
}

#todoList li {
background-color: #2c2c2c;
padding: 10px;
border-radius: 5px;
margin-bottom: 10px;
display: flex;
justify-content: space-between;
}
Comment on lines +48 to +55
Copy link

@billy0904 billy0904 Sep 8, 2024

Choose a reason for hiding this comment

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

Screen Shot 2024-09-08 at 7 40 12 PM

숫자를 연속해서 입력하는 경우, 그 길이가 길어지면 줄바꿈이 일어나지 않고 칸을 뚫고 나가는(?) 현상이 있습니다! word-break: break-all; 속성을 사용하여 단어 중간이나 연속된 숫자 중간에서도 줄바꿈이 일어나도록 해주시면 좋을 것 같아요🤗

Suggested change
#todoList li {
background-color: #2c2c2c;
padding: 10px;
border-radius: 5px;
margin-bottom: 10px;
display: flex;
justify-content: space-between;
}
#todoList li {
background-color: #2c2c2c;
padding: 10px;
border-radius: 5px;
margin-bottom: 10px;
display: flex;
justify-content: space-between;
word-break: break-all;
}

Copy link
Author

@jsomnium jsomnium Sep 8, 2024

Choose a reason for hiding this comment

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

가빈님 정말 지식이 풍부하신 것 같아요. 테스트케이스도 다양하게 해보셨네요. 리뷰에서 많이 배워갑니다. 감사합니다!! 😊

Choose a reason for hiding this comment

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

이 부분은 사실 제 과제에서도 발생한 문제라서...ㅋㅎㅎㅎ 저도 뒤늦게 발견했는데 지수 님 코드에도 같은 오류가 있어 공유해봤습니다!😶✨ 도움 되셨다면 다행이에요! 좋은 코드 감사합니다!!👍


.todo-input {
display: flex;
justify-content: space-between;
}

input {
flex-grow: 1;
padding: 10px;
border-radius: 5px;
border: none;
margin-right: 10px;
}

button {
padding: 10px 20px;
background-color: #4caf50;
border: none;
border-radius: 5px;
cursor: pointer;
}
Comment on lines +70 to +76

Choose a reason for hiding this comment

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

Screen Shot 2024-09-08 at 7 19 17 PM
Suggested change
button {
padding: 10px 20px;
background-color: #4caf50;
border: none;
border-radius: 5px;
cursor: pointer;
}
button {
padding: 10px 20px;
background-color: #4caf50;
border: none;
border-radius: 5px;
cursor: pointer;
white-space: nowrap;
}

위와 같이 투두의 내용이 길어져서 요소의 높이와 삭제 버튼의 높이가 함께 높아질 때 "삭제" 텍스트가 그 높이에 따라 줄바꿈을 하는 현상이 있습니다. 개인적인 취향으로 일관적인 UI 제공을 위해 white-space: nowrap;으로 줄바꿈 방지를 해주시면 훨씬 예쁜 페이지가 될 것 같습니다!😊

Copy link
Author

Choose a reason for hiding this comment

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

빌드 테스트 할 때 대부분 글 짧게 작성하고 올렸어서 몰랐는데, 이런 부분이 있었네요.
테스트 할 때 다양한 케이스로 많이 시도해야 이런 부분을 잡을 수 있는 것 같아요.
줄바꿈 방지도 새로 배워갑니다! 감사합니다😊


button:hover {
background-color: #45a049;
}