-
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?
Conversation
<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> |
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.
과제 수행동안 고생많으셨습니다! container, nav, content, todoContainer와 같이 구간을 나눠서 진행한 점 인상깊었습니다. 다만 Semantic Tag 활용이 안 된 것 같아 Semantic Tag 활용이 되었다면 더 직관적으로 확인이 가능할 수 있을 것 같습니다! 추가적으로 class와 id가 모두 사용되고 있는데 사용의 근거가 궁금하여 여쭤보고 싶습니다!
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.
리뷰 감사합니다!😊
저는 보통 스타일을 조정할 때 class
를 사용하고,
날짜나, 투두 내용처럼 값을 활용해서 특정 태그를 불러와야할 때, id
를 활용하고 있습니다.
따라서 혼자만의 규칙이지만 class
는 다른 태그에도 중복적으로 사용하기도 하고 id
는 주민번호마냥 고유하게 사용합니다!
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 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를 생성하는 로직을 저도 자주 사용할 것 같습니다! **
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.
올려주신 레퍼런스가 잘 열리질 않네요..😢
리뷰를 읽어보니 예전에 봤던 key값 생성 영상이 생각나서 다시 찾아보게되었습니다!
key생성규칙 이게 맞을까요?
말씀주신 방법을 활용하면 정말 안겹치는 key를 만들 수 있을것 같습니다. (아무리 78억 가지라도 78억분의 1의 확률로 겹칠수도 있으니까요..ㅎㅎ)
window.onload = function () { | ||
renderToDoList(); | ||
}; |
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.
페이지 로딩 및 렌더링 관련해서 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 comment
The reason will be displayed to describe this comment to others. Learn more.
왜 페이지가 열리지 않는 지 모르겠지만..ㅠㅠ
영준님의 script.js
에 있는 관련 코드를 보고 추가로 학습할 수 있었습니다!
로드방법에 대해 저도 이번기회에 고민해볼 수 있었습니다. 감사합니다
.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; | ||
} | ||
} |
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.
item 부분에서 text를 길게 썼을 때 박스를 벗어나는 걸 확인했었습니다! 관련해서 overflow-wrap이나 유연하게 변동 가능한 방법이 있다면 그 방법도 좋을 것 같습니다. 아마 height가 52px로 고정되어있어서 그런 것 같습니다. item 박스 내에 overflow: auto와 scroll-bar를 숨기는 방식으로 하는 방법도 있을 것 같습니다!
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.
이렇게나 꼼꼼하게 테스트 해주시다니.. 정말 감사합니다😚😚
그런 문제가 있는지 방금 알게 되었네요ㅜㅜ!!
해결 방법도 참고해서 공부해보도록 하겠습니다!
.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; | ||
} |
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.
버튼에 패딩이 조금 더 추가되면 미적으로 빛이 날 것 같습니다++
}); | ||
|
||
// 투두 요소 만들기 | ||
var createToDoItem = (toDo, id) => { |
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.
혹시 이 함수만 var로 선언한 이유가 있는지 여쭤보고 싶습니다!
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.
아 이건 호이스팅 관련해서 테스트해보다가 안바꿔버렸네요!
감사합니다╰(°▽°)╯
document | ||
.getElementById('todoContainer') | ||
.addEventListener('submit', function (event) { | ||
event.preventDefault(); | ||
const inputField = document.getElementById('inputValue'); | ||
const toDo = inputField.value; | ||
|
||
if (toDo) { | ||
const id = generateID(); | ||
const newToDo = { id: id, toDo: toDo }; | ||
toDos.push(newToDo); | ||
saveToDos(); | ||
createToDoItem(toDo, id); | ||
inputField.value = ''; | ||
} | ||
}); |
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.
간결한 방식으로 투두리스트 추가 로직을 구현한 것이 좋은 것 같습니다. 저도 id를 활용하는 방법을 더 공부해봐야 할 것 같습니다!
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.
희원님 과제 수행하시느라 고생많으셨습니다!
중간에 합류하게되어서 설명도 제대로 못들으셨는데도, 과제 취지에 맞게 정말 열심히 해주신것 같아 감사드립니다 😂😂😂
주석이 잘 달려있어서, 코드리뷰하기 편했던것 같아요. 몇몇가지 디테일들까지 챙기면 더 완벽한 결과물이 될 것 같아요! 그에 대한 의견 몇개 남기고 갑니다!!
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'; | ||
} | ||
}; | ||
|
||
return icon; | ||
}; |
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.
투두 완료 상태는 로컬스토리지에 저장하지 않아서 할일완료후 새로고침하면 풀리는 현상이 나타나는 것 같아요.
할일 완료를 상태로 유지하고 이에 대한 값도 로컬스토리지에 저장하는 코드를 추가하면 좋을 것 같아요~!
icon.onclick = function () { | ||
if (icon.textContent === 'check_circled') { | ||
icon.textContent = 'radio_button_unchecked'; | ||
} else { | ||
icon.textContent = 'check_circled'; | ||
} | ||
}; |
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.
디테일적인 부분이지만 투두 완료 시, 단순히 아이콘만 변경되고 텍스트에 변화를 주지 않아 완료된 항목인지 시각적으로 명확하지 않은 것 같아요. 완료된 할 일에 취소선이나 텍스트 색상 변화를 적용해보는 것도 UX 향상에 큰 도움이 됩니다!
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 comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 console.log가 곳곳에 있는 것 같아요. 디버깅 목적으로 사용하는 것은 좋지만, 최종 코드에서는 제거하는 것이 좋습니다. :)
if (icon.textContent === 'check_circled') { | ||
icon.textContent = 'radio_button_unchecked'; | ||
} else { | ||
icon.textContent = 'check_circled'; | ||
} |
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.
icon.onclick = function () { | ||
if (icon.textContent === 'check_circled') { | ||
icon.textContent = 'radio_button_unchecked'; | ||
} else { | ||
icon.textContent = 'check_circled'; | ||
} |
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.
삼항 연산자를 활용하여 이렇게 간소하게 작성하는 건 어떨까요?
icon.onclick = function () {
icon.textContent = icon.textContent === 'check_circled' ? 'radio_button_unchecked' : 'check_circled';
};
.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 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 }; |
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.
const newToDo = { id, toDo };
객체 리터럴 단축 구문을 사용하여 { id, toDo }로 간소화할 수도 있을 거 같아요!
<div for="form">✔️</div> | ||
|
||
<input | ||
type="text" | ||
id="inputValue" | ||
placeholder="할 일 추가" | ||
/> |
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.
<label for="inputValue">✔️</label>
<input type="text" id="inputValue" placeholder="할 일 추가" />
for 속성은 label 태그에서 특별한 의미를 가지기 때문에 div태그를 label태그로 바꾸는 건 어떨까요? 또한 for 속성은 해당 라벨이 어떤 입력 요소와 연관되어 있는지를 나타내기에 input태그의 id를 for의 속성값과 동일하게 표현하는 것도 좋을 거 같습니다!
배포 링크
느끼고 배운 점 :
외부 라이브러리를 사용하지 않고 과제를 시작하니 개발을 처음 배울 때의 기분이 들었다. 원하는 기능을 구현하기 위해 다양한 속성을 찾아보고 공부하면서 역시 기본이 중요하다는 생각이 들었다. 이와 동시에 라이브러리의 효율성과 편리함이 얼마나 대단한지 생각해볼 수 있었다. 이전에 bootstrap과 같은 라이브러리를 사용할 때 무작정 결과물만 내기 위해서 아무거나 가져다 써서 효율적으로 코드를 쓰지 못했는데, 이렇게 공부해보니 제대로 알아야 필요한 것만 빠르게 활용하는 데에 도움이 되겠다고 느꼈다.
또한, localStorage를 통해 데이터를 관리할 때에도 좀 더 체계적인 접근이 필요하다고 느꼈다. 해당 기능을 추가할 때에는 단순히 투두리스트의 내용만 저장하면 되겠지라 생각했는데, 구현을 하다보니 삭제 및 공부 완료 여부에 대한 내용도 필요해서 추가작업이 필요했다. 이번 과제를 통해 앞으로의 개발에서는 더 나은 코드를 쓰는 개발자로 성장하고 싶다.
추가로 이번 과제를 제출하면서 아쉬운 부분이 좀 많았다. 앞으로 열심히 공부해서 최종적으로는 내가 만족할 수 있는 결과물을 만들어내고싶다.
DOM은 무엇인가요?
DOM(Document Object Model)은 마크업 언어(HTML, XML)로 작성된 문서를 프로그래밍 언어가 조작할 수 있도록 하는 인터페이스다.
이벤트 흐름 제어(버블링 & 캡처링)이 무엇인가요?
이벤트가 특정 요소에서 시작되어 관련된 상위 요소들로 전파되는 "이벤트 플로우(흐름)"는 javascript의 기본 동작이다.
클릭 이벤트가 발생할 때, 이벤트 흐름을 통해 이벤트 흐름 제어를 살펴봤다.
위의 사진에서는
div
가 이벤트의 시발점이다.만약
div
를 클릭했을 때,body
와html
요소에도 클릭 이벤트가 있다면, 이 두 요소 또한 해당 이벤트에 참여하게 된다.즉, 이벤트가 발생하면 해당 요소뿐만 아니라, 해당 요소와 관련된 것들이 모두 실행된다. 실행은 캡처링 혹은 버블링 단계에서 실행될 수 있다.
이때 문제는, "이벤트 처리 순서"이고, 이벤트 흐름 제어로 문제를 해결한다.
이벤트 흐름 제어 방법
(current target : 이벤트의 진짜 주인, target : 이벤트 시발점)
div 관점 :
div
div
body 관점 :
body
div
target != current target인 경우, event flow phase 중 어느 단계에서 실행되고 싶은지 선택할 수 있다.
capture vs. bubble
이와 같이 어디서 이벤트를 실행할 지 결정할 수 있다.
기본 값은 버블링할때 이벤트가 실행된다.
만약 캡처링할 때에 실행하고 싶은 경우, 코드 작성 시 구문의 마지막 인자로
true
값을 넣으면 된다.이벤트를 미세하게 다룰 때, 활용하면 효과적이다.
클로저와 스코프가 무엇인가요?
렉시컬 환경 : 코드가 작성된 위치를 기준으로 변수가 어떤 범위에서 유효한지를 결정한다.