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주차] 이희원 미션 제출합니다. #9

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
49 changes: 46 additions & 3 deletions index.html
Original file line number Diff line number Diff line change
@@ -1,14 +1,57 @@
<!DOCTYPE html>
<html lang="en">
<head>
<link
href="https://fonts.googleapis.com/css2?family=Material+Icons"
rel="stylesheet"
/>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta
name="viewport"
content="width=device-width, initial-scale=1.0"
/>
<title>Vanilla Todo</title>
<link rel="stylesheet" href="style.css" />
<link
rel="stylesheet"
href="style.css"
/>
</head>

<body>
<div class="container"></div>
<div class="container">
<!-- 네비게이션 -->
<div class="nav">✔️ To Do</div>

<!-- content -->
<div class="content">
<div class="title">
<p class="mainTitle">오늘 할 일</p>
<p id="date"></p>
</div>
<div id="todoContainer">
<form id="toDoForm">
<div for="form">✔️</div>

<input
type="text"
id="inputValue"
placeholder="할 일 추가"
/>
Comment on lines +33 to +39
Copy link

Choose a reason for hiding this comment

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

<label for="inputValue">✔️</label>
<input type="text" id="inputValue" placeholder="할 일 추가" />

for 속성은 label 태그에서 특별한 의미를 가지기 때문에 div태그를 label태그로 바꾸는 건 어떨까요? 또한 for 속성은 해당 라벨이 어떤 입력 요소와 연관되어 있는지를 나타내기에 input태그의 id를 for의 속성값과 동일하게 표현하는 것도 좋을 거 같습니다!

<button
class="mainButton"
type="submit"
>
추가
</button>
</form>

<ul id="toDoList">

</li>
</ul>
</div>
</div>
</div>
Comment on lines +21 to +54
Copy link

Choose a reason for hiding this comment

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

과제 수행동안 고생많으셨습니다! container, nav, content, todoContainer와 같이 구간을 나눠서 진행한 점 인상깊었습니다. 다만 Semantic Tag 활용이 안 된 것 같아 Semantic Tag 활용이 되었다면 더 직관적으로 확인이 가능할 수 있을 것 같습니다! 추가적으로 class와 id가 모두 사용되고 있는데 사용의 근거가 궁금하여 여쭤보고 싶습니다!

Copy link
Author

Choose a reason for hiding this comment

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

리뷰 감사합니다!😊
저는 보통 스타일을 조정할 때 class를 사용하고,
날짜나, 투두 내용처럼 값을 활용해서 특정 태그를 불러와야할 때, id를 활용하고 있습니다.
따라서 혼자만의 규칙이지만 class는 다른 태그에도 중복적으로 사용하기도 하고 id는 주민번호마냥 고유하게 사용합니다!

</body>
<script src="script.js"></script>
</html>
111 changes: 111 additions & 0 deletions script.js
Original file line number Diff line number Diff line change
@@ -1 +1,112 @@
//😍CEOS 20기 프론트엔드 파이팅😍

const days = [
'일요일',
'월요일',
'화요일',
'수요일',
'목요일',
'금요일',
'토요일',
];

const toDoList = document.getElementById('toDoList');
const dateElement = document.getElementById('date');

// 날짜 세팅하기
const date = new Date();
const month = date.getMonth() + 1;
const day = date.getDate();
const getDay = days[date.getDay()];

dateElement.textContent = `${month}월 ${day}일 ${getDay}`;

// localStorage에 담길 투두항목
let toDos = JSON.parse(localStorage.getItem('item')) || [];

// id 생성기
const generateID = () => Math.random().toString(36).substring(2, 9);
Copy link

@yyj0917 yyj0917 Sep 8, 2024

Choose a reason for hiding this comment

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

id가 랜덤으로 생성되었을 때 같은 id를 가진 todoItem이 생성될 수 있을 것 같습니다! 이번에 알게 된 html5에서 data속성을 html 요소에 넣어서 key 값을 사용할 수 있는 방법이 있어서 링크남기겠습니다! date-key로 활용하여 id를 생성하지 않고, 날짜에 따른 data-date-key값으로 todoItem을 관리할 수 있을 것 같습니다.

** 다시 확인해보니 경우의 수가 약 78억 정도라서 겹치지는 않을 것 같네요,,,고유한 id로 구별할 때 위처럼 id를 생성하는 로직을 저도 자주 사용할 것 같습니다! **

https://inpa.tistory.com/entry/JS-%F0%9F%93%9A-HTML-%EB%8D%B0%EC%9D%B4%ED%84%B0%EC%85%8Bdata-%EC%86%8D%EC%84%B1

Copy link
Author

Choose a reason for hiding this comment

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

올려주신 레퍼런스가 잘 열리질 않네요..😢
리뷰를 읽어보니 예전에 봤던 key값 생성 영상이 생각나서 다시 찾아보게되었습니다!
key생성규칙 이게 맞을까요?
말씀주신 방법을 활용하면 정말 안겹치는 key를 만들 수 있을것 같습니다. (아무리 78억 가지라도 78억분의 1의 확률로 겹칠수도 있으니까요..ㅎㅎ)


// localStorage에 저장
const saveToDos = () => {
localStorage.setItem('item', JSON.stringify(toDos));
};

// 투두리스트에 추가하기
document
.getElementById('todoContainer')
.addEventListener('submit', function (event) {
event.preventDefault();
const inputField = document.getElementById('inputValue');
const toDo = inputField.value;
Copy link

Choose a reason for hiding this comment

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

const toDo = inputField.value.trim();
사용하여 입력값의 앞뒤 공백을 제거하는 코드도 좋을 거 같아요!


if (toDo) {
const id = generateID();
const newToDo = { id: id, toDo: toDo };
Copy link

Choose a reason for hiding this comment

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

const newToDo = { id, toDo };
객체 리터럴 단축 구문을 사용하여 { id, toDo }로 간소화할 수도 있을 거 같아요!

toDos.push(newToDo);
saveToDos();
createToDoItem(toDo, id);
inputField.value = '';
}
});
Comment on lines +36 to +51
Copy link

Choose a reason for hiding this comment

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

간결한 방식으로 투두리스트 추가 로직을 구현한 것이 좋은 것 같습니다. 저도 id를 활용하는 방법을 더 공부해봐야 할 것 같습니다!


// 투두 요소 만들기
var createToDoItem = (toDo, id) => {
Copy link

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.

아 이건 호이스팅 관련해서 테스트해보다가 안바꿔버렸네요!
감사합니다╰(°▽°)╯

const newToDo = document.createElement('li');
const toDoTitle = document.createElement('div');
toDoTitle.textContent = toDo;
toDoTitle.classList.add('toDo');
newToDo.classList.add('toDoItem');

newToDo.appendChild(createIsCompleteButton());
newToDo.appendChild(toDoTitle);
newToDo.appendChild(createDeleteButton(newToDo, id));

toDoList.appendChild(newToDo);
};

// 투두 완료 버튼 생성
const createIsCompleteButton = () => {
const icon = document.createElement('span');
icon.classList.add('material-icons');
icon.textContent = 'radio_button_unchecked';

icon.onclick = function () {
if (icon.textContent === 'check_circled') {
icon.textContent = 'radio_button_unchecked';
} else {
icon.textContent = 'check_circled';
}
Comment on lines +75 to +79
Copy link
Member

Choose a reason for hiding this comment

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

image

현재 할일 완료 버튼을 클릭하면 텍스트가 오른쪽으로 이동하는 버그가 있는 것 같아요. 원인을 찾아봤더니 icon.textContent를 변경할 때, span 요소의 크기나 레이아웃이 의도치 않게 텍스트 콘텐츠의 크기가 변경되면서 텍스트의 위치에 영향을 주는 것 같습니다.

해결방법으로는 텍스트 콘텐츠를 변경하지 않고, CSS 클래스를 통해 아이콘의 상태를 변경해보는 것도 좋을 것 같아요 😃😃

Comment on lines +74 to +79
Copy link

Choose a reason for hiding this comment

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

삼항 연산자를 활용하여 이렇게 간소하게 작성하는 건 어떨까요?

icon.onclick = function () {
    icon.textContent = icon.textContent === 'check_circled' ? 'radio_button_unchecked' : 'check_circled';
  };

};
Comment on lines +74 to +80
Copy link
Member

Choose a reason for hiding this comment

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

디테일적인 부분이지만 투두 완료 시, 단순히 아이콘만 변경되고 텍스트에 변화를 주지 않아 완료된 항목인지 시각적으로 명확하지 않은 것 같아요. 완료된 할 일에 취소선이나 텍스트 색상 변화를 적용해보는 것도 UX 향상에 큰 도움이 됩니다!


return icon;
};
Comment on lines +69 to +83
Copy link
Member

Choose a reason for hiding this comment

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

투두 완료 상태는 로컬스토리지에 저장하지 않아서 할일완료후 새로고침하면 풀리는 현상이 나타나는 것 같아요.

할일 완료를 상태로 유지하고 이에 대한 값도 로컬스토리지에 저장하는 코드를 추가하면 좋을 것 같아요~!


// 투두 삭제 버튼 추가
const createDeleteButton = (item, id) => {
const deleteButton = document.createElement('div');
deleteButton.textContent = '삭제';
deleteButton.classList.add('mainButton');
deleteButton.onclick = function () {
toDoList.removeChild(item);
console.log(id);
Copy link
Member

Choose a reason for hiding this comment

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

불필요한 console.log가 곳곳에 있는 것 같아요. 디버깅 목적으로 사용하는 것은 좋지만, 최종 코드에서는 제거하는 것이 좋습니다. :)

toDos = toDos.filter((todo) => todo.id !== id);
console.log(toDos);
saveToDos();
};

return deleteButton;
};

// 투두 리스트 렌더링
const renderToDoList = () => {
toDoList.textContent = '';
toDos.forEach((item) => {
createToDoItem(item.toDo, item.id);
});
};

// 시작시 렌더링
window.onload = function () {
renderToDoList();
};
Comment on lines +110 to +112
Copy link

Choose a reason for hiding this comment

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

페이지 로딩 및 렌더링 관련해서 window.onload / document.addEventListener('DOMContentLoaded', function () / 이벤트 핸들러 없이 실행 -> 하는 3가지 방법을 알고 있습니다! 각 방법마다 목적과 장단점이 다르기 때문에 한번 고려해보시는 것도 좋을 것 같습니다. 이번 투두리스트 코드에서는 리소스 의존성이 적기 때문에 2번째 방법으로 빠른 DOM 요소 업데이트 및 로드가 좋을 수도 있을 것 같다는 생각이 듭니다!

이번 기회로 로드 방법에 따라 목적과 장단점이 어떻게 되는지를 공부할 수 있어 좋은 기회가 되었습니다.

https://mesonia.tistory.com/entry/%ED%8E%98%EC%9D%B4%EC%A7%80-%EB%A1%9C%EB%93%9C-%ED%9B%84-%EC%8B%A4%ED%96%89%ED%95%98%EA%B8%B0-windowonload-documentready-documentready%EB%A5%BC-%EC%88%9C%EC%88%98%EC%9E%90%EB%B0%94%EC%8A%A4%ED%81%AC%EB%A6%BD%ED%8A%B8%EB%A1%9C-DOMContentLoaded
적당한 레퍼를 찾지 못하였지만 이 링크도 꽤 도움이 되어서 공유드립니다!

Copy link
Author

Choose a reason for hiding this comment

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

왜 페이지가 열리지 않는 지 모르겠지만..ㅠㅠ
영준님의 script.js 에 있는 관련 코드를 보고 추가로 학습할 수 있었습니다!
로드방법에 대해 저도 이번기회에 고민해볼 수 있었습니다. 감사합니다

111 changes: 111 additions & 0 deletions style.css
Original file line number Diff line number Diff line change
@@ -1 +1,112 @@
/* 본인의 디자인 감각을 최대한 발휘해주세요! */
body {
background-color: rgb(43, 43, 43);
margin: 0px;
}

.nav {
background-color: black;
padding: 8px 32px;
font-size: 18px;
color: aliceblue;
border-bottom: 1px solid rgb(130, 130, 130);
}

.mainButton {
/* font-size: 12px; */
cursor: pointer;
border-radius: 12px;
padding: 1px;
background-color: transparent;
color: rgb(150, 130, 224);
border: 1px solid rgb(150, 130, 224);
display: flex;
align-items: center;
justify-content: center;
}
Comment on lines +15 to +26
Copy link

Choose a reason for hiding this comment

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

버튼에 패딩이 조금 더 추가되면 미적으로 빛이 날 것 같습니다++


.material-icons {
color: rgb(150, 130, 224);
cursor: pointer;
}
.mainButton:hover {
background-color: rgb(19, 18, 18);
}

.content {
display: flex;
min-height: 100vh;
height: auto;
padding: 16px;
width: 50%;
margin: 0 auto;
flex-direction: column;
background-color: black;

.title {
display: flex;
flex-direction: column;
.mainTitle {
font-size: 18px;
color: white;
margin-bottom: 0;
}
#date {
font-size: 10px;
color: rgb(220, 220, 220);
}
}

form {
border: 16px;
margin-top: 12px;
display: flex;
flex-direction: row;
align-items: center;
font-size: 14px;
height: 52px;
padding: 0 16px;
gap: 8px;
border-radius: 12px;
color: rgb(150, 130, 224);
background-color: rgb(37, 37, 37);

input {
flex-grow: 1;
color: white;
background-color: rgb(37, 37, 37);
border: none;
outline: none;
font-size: 16px;
}
input::placeholder {
color: rgb(150, 130, 224);
}
}
}

ul {
/* background-color: aliceblue; */
display: flex;
flex-direction: column-reverse;
list-style-type: none;
padding-inline-start: 0;
}
.toDoItem {
/* border: 16px; */
margin-top: 12px;
display: flex;
flex-direction: row;
align-items: center;
font-size: 14px;
height: 52px;
padding: 0 16px;
gap: 8px;
border-radius: 12px;
color: white;
background-color: rgb(37, 37, 37);
.toDo {
flex-grow: 1;
color: white;
}
}
Comment on lines +95 to +112
Copy link

Choose a reason for hiding this comment

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

item 부분에서 text를 길게 썼을 때 박스를 벗어나는 걸 확인했었습니다! 관련해서 overflow-wrap이나 유연하게 변동 가능한 방법이 있다면 그 방법도 좋을 것 같습니다. 아마 height가 52px로 고정되어있어서 그런 것 같습니다. item 박스 내에 overflow: auto와 scroll-bar를 숨기는 방식으로 하는 방법도 있을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

이렇게나 꼼꼼하게 테스트 해주시다니.. 정말 감사합니다😚😚
그런 문제가 있는지 방금 알게 되었네요ㅜㅜ!!
해결 방법도 참고해서 공부해보도록 하겠습니다!