Skip to content

[1주차] 정대헌 미션 제출합니다.#4

Open
jdaeheon wants to merge 6 commits intoCEOS-Developers:masterfrom
jdaeheon:master
Open

[1주차] 정대헌 미션 제출합니다.#4
jdaeheon wants to merge 6 commits intoCEOS-Developers:masterfrom
jdaeheon:master

Conversation

@jdaeheon
Copy link

@jdaeheon jdaeheon commented Mar 17, 2022

Project

안녕하세요, CEOS 15기 정대헌입니다. 바닐라 자바스크립트를 마지막으로 만진지 3개월 밖에 안지났는데 그새 많은 부분을 잊어 버렸어요. 과거 연습 Todo를 만들었던 경험이 있어서, 당시 프로젝트 파일 열어서 복기하면서 작업했습니다. 세상에 그래도 React Native 만지다가 브라우저 만지니까 너무 좋은 것 같아요. 작은 오류들 디버깅 하기 너무 편리하네요. 컨셉은 독특하게 북한 느낌으로 해봤습니다.

Vercel Link

https://vanilla-todo-15th-dprk-kpip68tgb-asiloveyou.vercel.app/

추가적 기능

  • 움직이는 그라디언트 배경 (css animation 기반입니다, 원 파일 링크 적어두었어요)

어려운 부분

  • 효율적인 코드는 아닌 것 같습니다. 특히 현재님 푸시 메시지에서 나오듯 state 관리가 안되니까 갯수 추적를 위해서 여기저기 함수를 뿌리게 되네요. 아마 방법이 있을 것 같은데, 누군가 고수 분을 통해 알게되지 않을지…

  • 적응적 UI를 적용할까 하다가 미뤄두었습니다. CSS의 수많은 규격과 불안한 젠가를 하는듯한 레이아웃 규칙들을 생각하면 어려움이 있었습니다. 이번에도 문서들 살펴보면서 하다보니 초기 레이아웃 구성하는데 오랜 시간이 걸린 것 같습니다. 아마 앞으로 웹으로 프로젝트 계속 진행하게 된다면 익숙해질 필요가 있을 것 같습니다.

  • 기능 단위 Commit을 처음 해보았습니다. 사실 만들고 나서 알게되서 임시 방편으로 다만든 파일을 나누어서 커밋한 걸 보시면서 다소 이상하실 수 있습니다.

Key Question

1. DOM은 무엇인가요?

DOM(Document Object Model), html 문서의 태그들을 객체로 만들어서 브라우저가 인식할 수 있도록 하는 모델을 말합니다. DOM은 tree 형태의 자료구조를 사용합니다. 따라서 root node를 기준으로 하위요소를 따라 아래로 퍼져나가는 형태로 그립니다. non-SPA 프레임워크인 DOM은 전체 페이지에 대한 정보를 담고 있기 때문에, 페이지를 리프레시 할때 마다 모든 DOM을 다시 구조화해야한 다는 단점을 가지고 있습니다. React로 대표되는 SPA 프레임워크들은 DOM을 다수의 가상화 DOM으로 나누어 업데이트 시 필요한 DOM만 업데이트해 페이지 최적화를 추구합니다.

과거 읽어보다가 미뤄둔 좋은 글이 있어서 첨부합니다.
https://d2.naver.com/helloworld/59361

2. HTML (tag) Element를 JavaScript로 생성하는 방법은 어떤 것이 있고, 어떤 방법이 가장 적합할까요?

우선 html 요소의 태그를 기준으로 직접 가져오는 방법이 있습니다. id(getElementById), tag name(getElementsByTagName), class name(getElementsByClassName)을 사용할 수 있습니다. 이어 CSS 셀렉터를 사용해 CSS 요소를 가져오는 방법(querySelectorAll, querySelector)이 있습니다. 이 경우 특정 클래스의 하위 엘리먼트 등을 지정하는 등 보다 자유롭게 할 수 있다는 장점이 있습니다. const x = document.querySelectorAll("p.intro"); 마지막으로 object collection을 통한 방법이 존재합니다. Form, p, section등 object를 불러와 그중 일치하는 id를 찾는 방식입니다. const x = document.forms["frm1"];

4. Semantic tag에는 어떤 것이 있으며, 이를 사용하는 이유는 무엇일까요?

Semantic tag는 의미를 가진 html tag를 의미합니다. Non-semantic Tag의 예로는 <div>, <span>이 존재합니다. Semantic Tag의 예로는 <form>, <table>, <article>이 존재합니다. Semantic Tag 사이에는 관계가 존재하는데, 예를들어 <header> <footer>등은 문서의 처음과 마지막에 위치합니다.

6. Flexbox Layout은 무엇이며, 어떻게 사용하나요?

레이아웃 규칙으로 block, inline, table, positioned의 규칙 이후로 정의된 반응형 레이아웃 입니다. Row, column을 기본적인 순서로 사용할 수 있고, wrap 등을 통해 반응형으로 요소들이 정렬되도록 할 수 있습니다. 또한, 중앙정렬, 시작정렬, 끝정렬, 사이공간 등 다양한 형태로 요소간 공간을 지정할 수 있습니다. *Flexbox는 React Native에서 유일하고 기본으로 지원하는 정렬 규격입니다!

7. JavaScript가 다른 언어들에 비해 주목할 만한 점에는 어떤 것들이 있나요?

Javascript는 인기가 많은 인터프리터 언어입니다. 클라이언트 렌더링을 위해 사용하는 스크립트 언어의 특징도 가지고 있습니다. 따라서 다양한 브라우저에서 규격에 따라 실행될 수 있고 문법과 언어가 단순합니다. 동적으로 자료형을 정의합니다. 다만, JIT(JITC)을 이용해 브라우저 엔진에 따라 인터프리터와 컴파일러의 장점을 결합하여 Runtime Profiler를 이용해 자주 사용되는 요소에 대해 유동적으로 컴파일 할 수 있습니다. 그럼에도 JavaScript는 C, Java보다 느릴 수 있고 디버깅이 다소 어렵다는 단점이 있습니다. 또한, OOP에 대한 개념적 지원이 다른 언어와 비교해 다소 미비합니다. (클래스가 아닌 프로토타입 기반, 다중상속 미지원).

8. 코드에서 주석을 다는 바람직한 방법은 무엇일까요?

과도하게 친절한 주석은 가독성을 방해하고, 충분하지 못한 주석은 코드 리뷰에 어려움을 줄 수 있습니다. 저도 이번 주석을 달며 읽는 사람의 관점에서 어떤 정보들이 필요할 지 고민하면서 적어보았습니다. 다만, 공개적으로 코드리뷰에 참여하는 것이 처음이라 주석과 관련해 어려운 부분이 있다면 지적해주시면 감사하겠습니다.

Daeheon Jeong added 6 commits March 17, 2022 22:36
초기화 실행 동작 정의
등록 이벤트 핸들러, 아이템 수 추적 함수 정의
List 관련 조작 함수들 선언
Item 관련 조작함수 선언
Storage 조작 함수들 선언
Copy link
Member

@corinthionia corinthionia left a comment

Choose a reason for hiding this comment

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

안녕하세요 대헌 님 🙌🏻

우선 배포 링크를 들어가자마자 눈에 띄는 디자인과 독특한 컨셉이 매우 인상적이었습니다. 특히 배경의 그라데이션이 계속해서 바뀌도록 구현해 주신 점도 정말 마음에 들었어요 👍🏻 전체적으로 주석도 많이 달아 주시고, 코드도 군더더기 없이 작성해 주셔서 리뷰하는 데 정말 편했지만, 할 일 목록과 완료 목록에 대한 함수를 각각 만들어 주셔서 중복되는 부분이 다소 있는 것 같습니다. 다음주 과제를 진행하시면서 이러한 부분을 어떻게 하면 줄여 나갈 수 있을지 생각해 보시면 좋을 것 같아요! 그 부분 외에는 전체적으로 잘 작성해 주셔서 크게 수정할 만한 부분은 없었고, 몇 가지 제안 정도만 남겨 드립니다!

과제하시느라 고생 많으셨고 그럼 스터디 시간에 뵙겠습니다~!

Comment on lines +20 to +35
window.addEventListener("DOMContentLoaded", () => {
updateCnt();
let items = getLocalStorage();

if (items.length > 0) {
items.forEach(function (item) {
if (item.value.type === "todo") {
createTodoItem(item.id, item.value.text);
} else if (item.value.type === "done") {
createDoneItem(item.id, item.value.text);
}
});
}

// LocalStorage에서 Item을 불러오고, 비어있는 경우 예외
});
Copy link
Member

Choose a reason for hiding this comment

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

현재 로컬스토리지에서 값을 불러온 후 개수가 제대로 카운팅되지 않는 것 같습니다. 아래와같이 수정하시면 정상적으로 카운팅됩니다!

Suggested change
window.addEventListener("DOMContentLoaded", () => {
updateCnt();
let items = getLocalStorage();
if (items.length > 0) {
items.forEach(function (item) {
if (item.value.type === "todo") {
createTodoItem(item.id, item.value.text);
} else if (item.value.type === "done") {
createDoneItem(item.id, item.value.text);
}
});
}
// LocalStorage에서 Item을 불러오고, 비어있는 경우 예외
});
window.addEventListener("DOMContentLoaded", () => {
let items = getLocalStorage();
if (items.length > 0) {
items.forEach(function (item) {
if (item.value.type === "todo") {
createTodoItem(item.id, item.value.text);
} else if (item.value.type === "done") {
createDoneItem(item.id, item.value.text);
}
});
}
updateCnt();
// LocalStorage에서 Item을 불러오고, 비어있는 경우 예외
});

Choose a reason for hiding this comment

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

대헌님 코드도 좋다 생각했는데 리뷰해주신 부분도 좋은 것 같아서, 글 읽으면서 배워 갑니당 👍

Copy link
Author

Choose a reason for hiding this comment

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

맞아요, updateCnt()가 뒤에 들어가야겠군요. 감사합니다!

}

function addToLocalStorage(id, value) {
console.log(value);
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는 지워 주시는 게 좋습니다!

Copy link
Author

Choose a reason for hiding this comment

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

체크도 안하고 제출했군요 ㅋㅋㅋ ㅠ 감사합니다

Comment on lines +173 to +174
let items = getLocalStorage();
items.push(item);
Copy link
Member

@corinthionia corinthionia Mar 18, 2022

Choose a reason for hiding this comment

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

ES6 문법인 spread 연산자를 사용해 봅시다!

Suggested change
let items = getLocalStorage();
items.push(item);
let items = [...getLocalStorage(), item];

또한 JS의 push() 메소드는 원본 배열의 내용 자체를 직접 변경합니다. 하지만 원본 배열을 직접 변경하는 경우 의도치 않은 결과가 발생할 수도 있기 때문에, 위 코드처럼 새로운 배열로 만드는 방법을 추천드립니다. 이 글 참고하시면 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다. spread operator에게 이런 장점이 있다는 것은 처음 알았어요.

Comment on lines +181 to +185
items = items.filter(function (item) {
if (item.id !== id) {
return item;
}
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
items = items.filter(function (item) {
if (item.id !== id) {
return item;
}
});
items = items.filter((item) => item.id !== id);

filter() 메소드는 새로운 배열을 반환하기 때문에 별도의 return문을 작성하지 않으셔도 됩니다. 또한 arrow function도 사용해 보세요!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다, 참고하겠습니다!

place-items: center;
background: linear-gradient(-45deg, #fc466b, #3f5efb, #fc466b, #3f5efb);
background-size: 400% 400%;
animation: gradient 15s ease infinite;
Copy link
Member

Choose a reason for hiding this comment

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

애니메이션까지 넣어 주시다니... 👍🏻

Copy link
Author

Choose a reason for hiding this comment

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

상당히 화려한 애니메이션...

Comment on lines +13 to +15
<div class="container">
<div class="todoInput">
<div class="todoTitle">
Copy link
Member

Choose a reason for hiding this comment

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

HTML 태그의 id/class 네임은 보통 kebab-case로 짓습니다!

Suggested change
<div class="container">
<div class="todoInput">
<div class="todoTitle">
<div class="container">
<div class="todo-input">
<div class="todo-title">

Copy link

@S-J-Kim S-J-Kim left a comment

Choose a reason for hiding this comment

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

대헌님 안녕하세요

프론트엔드 멘토 김선종입니다.

깔끔하게 코드를 잘 작성해주셨는데요, 네이밍이 명확하여 코드 읽는 입장에서 아주 편안했습니다. 다만 중복되는 코드를 줄여보는 방법에 대해 고민해보시면 더 좋은 코드를 작성하실 수 있을것이라 생각이 듭니다. 운영진들이 좋은 코멘트를 이미 남겨주었기에 크게 드릴 말씀은 없었구요, 몇가지 참고하면 좋으실만한 내용을 코멘트로 남겨보았습니다.

과제 하느라 고생하셨습니다


// ====== 초기화시 실행 동작 ======= //

window.addEventListener("DOMContentLoaded", () => {
Copy link

Choose a reason for hiding this comment

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

보통 많은 분들이 페이지에 처음 접근했을 때 이벤트를 실행시키고 싶으면, onload 이벤트에 핸들러를 바인딩하는 경우가 많은데요, 대헌님처럼 DOMContentLoaded에 바인딩하는게 퍼포먼스에 더 유리하다고 합니다. 특히 첫 인터랙션까지 상대적으로 오래걸리는 SPA에서는 빠르게 이벤트를 바인딩 할 수 있으면 좋겠죠.

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.

그리고 북한 컨셉 너무 좋았어요

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다. DOMContentLoaded를 습관적으로 사용하고 있었는데, 이런 장점이 있는 줄은 몰랐습니다.
아 컨셉은 ㅋㅋㅋ 네 감사합니다 ㅎㅎ


window.addEventListener("DOMContentLoaded", () => {
updateCnt();
let items = getLocalStorage();
Copy link

Choose a reason for hiding this comment

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

items가 꼭 let이여아 할 필요가 있을까요? 부득이한 경우를 제외하고는 const를 사용하는 것이 일반적입니다.

Suggested change
let items = getLocalStorage();
const items = getLocalStorage();

Copy link
Author

Choose a reason for hiding this comment

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

맞습니다. 참고하겠습니다!

Comment on lines +26 to +30
if (item.value.type === "todo") {
createTodoItem(item.id, item.value.text);
} else if (item.value.type === "done") {
createDoneItem(item.id, item.value.text);
}
Copy link

Choose a reason for hiding this comment

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

좀 깊게 들어가는 감이 없지않긴 한데, else if를 사용하는 경우에는 else에 대한 처리도 해주는 것이 좋습니다. switch문의 default를 빼먹지 않는것이 좋은 것 처럼요.

Copy link
Author

Choose a reason for hiding this comment

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

참고하겠습니다. 디버깅시 유리하도록 하는게 좋을 것 같습니다.

Comment on lines +65 to +90
function moveToDone(e) {
const todoItem = e.currentTarget.parentElement;
const id = todoItem.getAttribute("id");
const itemText = todoItem.textContent.slice(0, -3);

todoList.removeChild(todoItem);
editLocalStorage(id, { text: itemText, type: "done" });

createDoneItem(id, itemText);
updateCnt();

// button 기준 parentELement = doneItem
// Unique ID를 기준으로 localStorage 수정
}
function moveToTodo(e) {
const doneItem = e.currentTarget.parentElement;
const id = doneItem.getAttribute("id");
const itemText = doneItem.textContent.slice(0, -3);

//textContent로 불러올 시 " \n"이 더해지는 현상이 있어 slice로 처리

doneList.removeChild(doneItem);
editLocalStorage(id, { text: itemText, type: "todo" });

createTodoItem(id, itemText);
updateCnt();
Copy link

Choose a reason for hiding this comment

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

삭제하는 대상이 누구냐만 다르고 정확히 같은 코드로 작성하셨네요. 무언가 이동하는 함수를 구현 한 다음 무언가를 인자로 넣음으로서 한개의 함수에서 처리할 수 있을 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

맞습니다, 하나로 처리할까 하다가 지나가게 되었는데, 함께 처리하면 더 좋을 것 같습니다.

// 새로운 div element 생성 후 인자로 전달된 unique id를 attribute으로 설정
// 'item' 클래스는 각 아이템의 css 스타일 프리셋

todoItem.innerHTML = `<button class="itemFinish" type="button"><p class="itemTitle">${itemText}</p></button>
Copy link

Choose a reason for hiding this comment

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

Suggested change
todoItem.innerHTML = `<button class="itemFinish" type="button"><p class="itemTitle">${itemText}</p></button>
todoItem.innerHTML = `
<button class="itemFinish" type="button">
<p class="itemTitle">${itemText}</p>
</button>`

템플릿 리터럴은 줄바꿈과 들여쓰기도 저장하므로 컴포넌트의 구조가 보일 수 있도록 쓰는게 좋겠죠.

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다. 해당 부분 참고해서 리터럴 작성하도록 하겠습니다!

}
.formControl {
display: flex;
flex-direction: row;
Copy link

Choose a reason for hiding this comment

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

flex-direction의 default value는 row이므로 이 부분은 넣지 않아도 무방합니다.

Copy link
Author

Choose a reason for hiding this comment

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

앗...RN의 흔적입니다. 맞습니다, 참고하겠습니다!

Copy link
Member

@jhj2713 jhj2713 left a comment

Choose a reason for hiding this comment

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

안녕하세요! 코드리뷰 파트너 주효정입니다:smile:
깔끔하고 보기 쉽게 저보다 너무 잘 작성해주셔서 어떤 리뷰를 남겨야하나 고민을 많이 했습니다.. 되려 제가 코드 보면서 배운 것도 많은 것 같아요! 그래도 제가 코드 보면서 든 생각들 짧게나마 적어봤습니다!

<div class="doneList">
</div>
</section>
<p class="footer">21세기 혁명의 정신을 완수하자!</p>
Copy link
Member

Choose a reason for hiding this comment

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

제가 시맨틱 태그를 아직 많이 사용해보지 않아서 조심스럽지만 클래스로 footer를 주는 것 말고 footer 태그를 사용해볼 수도 있지 않을까요..? !

Copy link
Author

Choose a reason for hiding this comment

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

맞습니다. 또한 클래스명과 태그명이 중복되는 것도 좋지 않을 것 같네요. 감사합니다!

doneCountField.textContent = doneList.children.length;
}

// ====== Todo, Done List 조작 관련 함수들 ======= //
Copy link
Member

Choose a reason for hiding this comment

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

관련된 함수들 모아두고 주석으로 표시해둔 거 보기 좋고 이해하기 편한 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다. 계속 작성하도록 하겠습니다!

}

formInput.value = "";
updateCnt();
Copy link
Member

Choose a reason for hiding this comment

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

value 초기화하는 부분이랑 updateCnt 함수 호출하는 부분도 value가 값이 있는 경우에만 의미있을 것 같아서 조건문 안에 넣어도 괜찮지 않을까요?!

Copy link
Author

Choose a reason for hiding this comment

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

맞습니다. 조건문을 통해 불필요한 함수 호출을 제어할 수 있겠군요.

@poodlepoodle
Copy link

아니 대헌님ㅋㅋㅋㅋㅋ 배포하신 거 보고 도저히 그냥 지나칠 수가 없네요
컨셉 너무 인상깊었습니다..... 영감을 얻고 갑니다 ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants