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주차] 강나연 미션 제출합니다. #8

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

kongnayeon
Copy link
Member

안녕하세요 강나연입니다!

이번 과제를 하면서 자바스크립트 기초가 많이 부족하다는 사실을 깨닫게 된 것 같습니다
빨리 공부를 더 더 해야겠어요 😿 
DOM 조작하고 로컬 스토리지를 이용하는 것도 은근히 힘들었네요…

가장 어려웠던 점은 처음에 todo와 done을 나눠서 관리하다가 토글 함수를 구현하려고 하니 코드가 너무 지저분해져서 할 일 하나하나를 객체로 관리하는 식으로 구조를 변경했습니다
저는 이 부분을 구현할 때 다른 부분에 비해 시간도 많이 쓰고 고민도 많이 했던 것 같네요…!

✔️ 배포 링크: https://vanilla-todo-16th-kongnayeon.vercel.app/

이번 과제를 통해 부족한 부분들에 대해서도 깨닫게 됐고 자바스크립트에 대해서도 많이 배워 가는 것 같습니다!


💭 Key Questions

  • DOM은 무엇인가요?
    웹 페이지가 스크립트나 프로그래밍 언어에서 사용될 수 있게 연결시켜 주는 역할을 합니다
    개체 구조가 노드 트리 형태로 표현되어 있습니다
  • HTML (tag) Element를 JavaScript로 생성하는 방법은 어떤 것이 있고, 어떤 방법이 가장 적합할까요?
    createElement() 를 통해 원하는 태그를 생성할 수 있습니다
    setAttribute() 를 이용해 만든 태그에 속성과 값을 부여할 수 있습니다
    appendChild()를 이용해 속성을 부여해 준 태그를 자식 태그로 넣을 수 있습니다
    저는 이 방법만 알고 있어서 이걸 사용했는데 더 좋은 방법이 있다면 알려 주시면 감사하겠습니다!
  • Semantic tag에는 어떤 것이 있으며, 이를 사용하는 이유는 무엇일까요?
    <form> , <article>, <header>, <nav> 등이 있습니다
    웹 페이지의 구조를 설계할 때 태그에 의미를 부여하도록 함으로써 사이트 파악을 용이하게 해 줍니다
  • Flexbox Layout은 무엇이며, 어떻게 사용하나요?
    flexbox 내부의 아이템 간의 공간을 배분하고 정렬하는 레이아웃 모델입니다
    대표적으로 flex-direction 속성을 사용해 주축의 정렬 방향을 선택할 수 있습니다
    align-items 속성을 사용해 교차축에서의 정렬 방향을 선택할 수 있습니다
     row는 x축 정렬,  column 은 y축 정렬을 의미합니다
  • JavaScript가 다른 언어들에 비해 주목할 만한 점에는 어떤 것들이 있나요?
    컴파일 과정이 없기 때문에 굉장히 빠르고, 운영 체제에 제한이 딱히 없습니다
    객체 지향형 프로그래밍과 함수형 프로그래밍 두 가지가 모두 가능합니다
  • 코드에서 주석을 다는 바람직한 방법은 무엇일까요?
    다른 사람들이 코드를 보고 바로 이해할 수 있도록 다는 것이 중요하다고 생각합니다
    과도하지도 부족하지도 않은 게 좋은 것 같습니다...!

많은 리뷰 부탁드려요… 🥺

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.

안녕하세요! 프론트 파트장 주효정입니다👏

전반적으로 코드가 매우 깔끔하고 정돈되어있어서 이해하기 너무 쉬웠던 것 같아요! 실제 프로젝트를 진행할 때가 기대됩니다👍 깃 로그에 남겨진 깜찍한 이모지들도 너무 귀엽네요ㅎㅎ
이번 과제를 통해 많이 배웠다고 말씀해주셔서 기쁩니다. 이번 과제뿐만 아니라 다른 과제에서도 많은 것을 배워가실 수 있었으면 좋겠어요!

그럼 한 주 동안 수고 많으셨고 스터디 시간에 뵐게요~!🥳

<body>
<div class="container"></div>
<div class="container">
Copy link
Member

Choose a reason for hiding this comment

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

semantic 태그를 잘 활용하시는 것 같아요! 앞으로 개발하실때도 계속해서 태그를 잘 활용하시면 좋을 것 같아요👍

Copy link
Member Author

Choose a reason for hiding this comment

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

항상 div 남발하고 다녔는데 이번에 공부하면서 그러면 안 된다는 걸 알게 됐습니다... 🥲

<div class="container">
<h1>Things to do</h1>
<form id="input-form">
<input id="input-text" type="text" placeholder="📍 Enter your to-do" />
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
Member Author

Choose a reason for hiding this comment

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

감사합니다 ㅎㅅㅎ 열심히 골라 봤어요 🤓

<h1>Things to do</h1>
<form id="input-form">
<input id="input-text" type="text" placeholder="📍 Enter your to-do" />
<button id="input-button" onclick="addTodoList()">+</button>
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
Member Author

Choose a reason for hiding this comment

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

헉 읽어 보니 정말 유지보수가 힘들 것 같네요,,, 배워 갑니다...!!!

if (savedTodo) {
itemList = JSON.parse(savedTodo);

itemList.forEach((savedItem) => {
Copy link
Member

Choose a reason for hiding this comment

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

고차함수를 적절하게 잘 활용하시는 것 같아요~!

};
if (inputObject.text) {
// 빈 입력은 받지 않음
itemList = [...itemList, inputObject];
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
Member Author

Choose a reason for hiding this comment

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

이번에 공부하면서 알게 됐어요!!! 이번 과제로 몰랐던 거 많이 많이 배워 갑니다... 🥰

text: document.getElementById("input-text").value,
isDone: false,
};
if (inputObject.text) {
Copy link
Member

Choose a reason for hiding this comment

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

비어있는 입력값을 예외처리 해주셨는데, 공백이 들어간 값은 예외처리가 안되더라구요. 정규식(\s)이나 다른 함수(trim())를 사용하셔서 공백 처리를 해볼 수 있을 것 같아요~!

Copy link
Member Author

Choose a reason for hiding this comment

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

세상에 이것도 몰랐네요... 알려 주셔서 감사해요... 🥺

Comment on lines +47 to +48
const addTodoList = () => {
event.preventDefault;
Copy link
Member

Choose a reason for hiding this comment

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

addTodoList에서 event.preventDefault() 처리가 안되고 계속 새로고침이 일어나는 것 같아요! event를 인자로 받아주시고 호출 인자에 이벤트를 부여해주셔야 할 것 같아요!

Suggested change
const addTodoList = () => {
event.preventDefault;
const addTodoList = (event) => {
event.preventDefault();

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 이거 제출하고 나서야 깨달았네요... 수정하겠습니다!!

// 빈 입력은 받지 않음
itemList = [...itemList, inputObject];
localStorage.setItem("itemList", JSON.stringify(itemList)); // 로컬 스토리지에 저장
renderTodoItem();
Copy link
Member

Choose a reason for hiding this comment

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

값을 추가한 뒤에 계속해서 renderTodoItem()을 호출하시는데, renderTodoItem() 로직은 모든 데이터를 지우고 화면을 다시 그리는 것 같아요. 계속해서 데이터가 지워졌다가 그려지는 방식은 조금 비효율적일 수 있어서 DOM에 접근해서 Element를 추가하는 방식으로 접근해보면 어떨까요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 비효율적이라는 생각을 했는데 해결책을 못 떠올리고 그냥 이렇게 짜버렸네요... 🥲 방향 제시해 주셔서 감사해요 🥺

Comment on lines +74 to +77
const filteredList = itemList.filter(
(inputObject) =>
inputObject.text !== e.target.parentNode.innerText.slice(0, -2)
);
Copy link
Member

Choose a reason for hiding this comment

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

원하는 아이템의 text가 일치하는 경우 삭제해주는 방식을 사용하셨는데, 이렇게 되면 같은 텍스트를 입력한 경우 하나의 아이템을 삭제해도 두 개가 동시에 삭제됩니다! id 등 고유한 값으로 삭제하는게 좋을 것 같아요~~

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 그렇네요...!! 수정하겠습니다!

Comment on lines +71 to +83
.todo-section,
.done-section {
flex: 0.5;
border-top: 1px solid rgb(213, 213, 213);
}

#todo-list,
#done-list {
cursor: pointer;
list-style: none;
padding-left: 1rem;
margin: 0;
overflow: auto;
Copy link
Member

Choose a reason for hiding this comment

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

공통된 style이 적용되는 id, class는 공통된 하나의 속성으로 묶어줘도 좋을 것 같아요! Element가 많아질수록 css 파일 내에서 id 선언이 늘어나고 유지보수가 어려워질 수 있으니까요!

Copy link
Member Author

Choose a reason for hiding this comment

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

그렇네요... 수정하겠습니다!! 감사해요 🥺

@YooSeonHo
Copy link
Member

안녕하세요 나연님 css를 쓰는 방법이나 객체로 list 저장하는 방법이나 모두 생각치도 못했던 방법들이라 많이 배워갑니다!!
피드백해서 기술적으로 많은 도움이 되고 싶었는데 리팩토링도 잘 하시고 파트장님이 좋은 말씀 많이 해주신 거 같아서 제가 더 쓸 부분이 없는 거 같아요..ㅠ.ㅠ
정말정말 고생하셨습니다!

@@ -67,10 +70,20 @@ const toggleItem = (e) => {
// 로컬 스토리지에서 값을 삭제한다
const removeItem = (e) => {
const filteredList = itemList.filter(

Choose a reason for hiding this comment

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

사소한거긴 한데 여기서 filteredList 대신에 itemList라는 명칭을 그대로 쓰면 아래 count에서 filter 하지 않아도 되지 않을까 싶습니다 ..!!

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 여기서 사용한 filter는 제가 삭제하고자 하는 todo 객체의 text를 찾아 삭제하기 위해 사용했고 밑의 filter는 각각 객체 속 isDone의 true, false 값을 카운트 하기 위해 사용한 건데 혹시 제가 잘못 이해했을까요??.............

// todo, done의 수 카운트
const countItem = () => {
const todoCount = document.getElementById("todo-count");
todoCount.innerText = ` (${itemList.filter((item) => !item.isDone).length})`;

Choose a reason for hiding this comment

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

여기서용!

text: document.getElementById("input-text").value,
isDone: false,
};
// 중복은 못 받게 해야 하는데...

Choose a reason for hiding this comment

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

처음에 document.getElementById("input-text").value 불러올 때 로컬스토리지랑 비교해서 값이 있으면 alert 뜨게 하는건 어떨까요 !!

Copy link
Member Author

Choose a reason for hiding this comment

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

좋아요!! 영준님 코드 리뷰 하면서 저도 알림창 띄워야겠다 생각했는데 수정해 보겠습니다! 😸

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.

4 participants