-
Notifications
You must be signed in to change notification settings - Fork 11
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
base: master
Are you sure you want to change the base?
Changes from all commits
375cafc
4db03ea
e57bb0b
707bd09
ab75547
3a01a93
4fcce13
7002c7e
bf8e9ad
42ab8cb
bb987c6
4a1573e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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="할 일 추가" | ||
/> | ||
<button | ||
class="mainButton" | ||
type="submit" | ||
> | ||
추가 | ||
</button> | ||
</form> | ||
|
||
<ul id="toDoList"> | ||
|
||
</li> | ||
</ul> | ||
</div> | ||
</div> | ||
</div> | ||
Comment on lines
+21
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 과제 수행동안 고생많으셨습니다! container, nav, content, todoContainer와 같이 구간을 나눠서 진행한 점 인상깊었습니다. 다만 Semantic Tag 활용이 안 된 것 같아 Semantic Tag 활용이 되었다면 더 직관적으로 확인이 가능할 수 있을 것 같습니다! 추가적으로 class와 id가 모두 사용되고 있는데 사용의 근거가 궁금하여 여쭤보고 싶습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 리뷰 감사합니다!😊 |
||
</body> | ||
<script src="script.js"></script> | ||
</html> |
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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를 생성하는 로직을 저도 자주 사용할 것 같습니다! ** There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 올려주신 레퍼런스가 잘 열리질 않네요..😢 |
||
|
||
// 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
if (toDo) { | ||
const id = generateID(); | ||
const newToDo = { id: id, toDo: toDo }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
toDos.push(newToDo); | ||
saveToDos(); | ||
createToDoItem(toDo, id); | ||
inputField.value = ''; | ||
} | ||
}); | ||
Comment on lines
+36
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 간결한 방식으로 투두리스트 추가 로직을 구현한 것이 좋은 것 같습니다. 저도 id를 활용하는 방법을 더 공부해봐야 할 것 같습니다! |
||
|
||
// 투두 요소 만들기 | ||
var createToDoItem = (toDo, id) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 혹시 이 함수만 var로 선언한 이유가 있는지 여쭤보고 싶습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+74
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 삼항 연산자를 활용하여 이렇게 간소하게 작성하는 건 어떨까요?
|
||
}; | ||
Comment on lines
+74
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 디테일적인 부분이지만 투두 완료 시, 단순히 아이콘만 변경되고 텍스트에 변화를 주지 않아 완료된 항목인지 시각적으로 명확하지 않은 것 같아요. 완료된 할 일에 취소선이나 텍스트 색상 변화를 적용해보는 것도 UX 향상에 큰 도움이 됩니다! |
||
|
||
return icon; | ||
}; | ||
Comment on lines
+69
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 왜 페이지가 열리지 않는 지 모르겠지만..ㅠㅠ |
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. item 부분에서 text를 길게 썼을 때 박스를 벗어나는 걸 확인했었습니다! 관련해서 overflow-wrap이나 유연하게 변동 가능한 방법이 있다면 그 방법도 좋을 것 같습니다. 아마 height가 52px로 고정되어있어서 그런 것 같습니다. item 박스 내에 overflow: auto와 scroll-bar를 숨기는 방식으로 하는 방법도 있을 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이렇게나 꼼꼼하게 테스트 해주시다니.. 정말 감사합니다😚😚 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for 속성은 label 태그에서 특별한 의미를 가지기 때문에 div태그를 label태그로 바꾸는 건 어떨까요? 또한 for 속성은 해당 라벨이 어떤 입력 요소와 연관되어 있는지를 나타내기에 input태그의 id를 for의 속성값과 동일하게 표현하는 것도 좋을 거 같습니다!