[1주차] 김윤일 미션 제출합니다. #4

cabrerainbu · 2024-08-27T16:15:52Z

안녕하세요! 유레카 프론트엔드 1기 김윤일입니다.

이번 미션은 저의 실력을 마주하게 된 미션이었습니다. 제가 뭘 모르는 지 알게 되었고 리액트가 얼마나 좋은지에 대해서도 조금이나마 느끼게 되었습니다. 시작은 막막했지만 하나하나 익혀가면서 만들고 직접 구현하고 나니 뿌듯하네요🥰

기능 구현을 최우선으로 한 코드라 정신 없고 가독성이 많이 떨어집니다. 더 나은 방법이나 조언은 코드리뷰로 해주시면 수정하겠습니다.
여러분의 피드백은 저를 더 성장하게 합니다! 감사합니다😃

알게 된 부분

div와 section 태그의 구분
DOM에 접근하는 방법
form태그의 기본 렌더링 기능을 없애야 할 때 preventDefault, 이벤트 전파가 일어났을 때 stopPropagation

개선하고 싶은 부분

커밋 컨벤션 적용해보기
Local Storage 적용해보기
기능 구현 시 의존성을 줄이고 재사용성 높이기
코드 리뷰 참고 후 리팩토링

KEY QUESTION

1) DOM이란 무엇인가요?

문서 객체 모델(DOM)로 HTML 문서 구조를 객체로 표현한 것입니다.

2) JavaScript로 DOM을 조작하는 방법은 어떤 것이 있나요?

querySelector, getElementById 같은 메서드를 이용하여 요소에 접근하거나 createElement를 사용하여 요소를 생성할 수 있습니다. 추가적으로 innerHTML로 html태그를 변경하고, 태그 내용을 수정 가능합니다.

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

header, main, footer, aside, nav 등
웹 문서의 구조와 의미를 명확하게 해주고 SEO를 향상시키는데 중요한 역할을 합니다.

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

레이아웃을 구성하는 css의 방법으로 Flexbox에는 container와 items로 나눠집니다.
Flexbox Container : display: flex, flex-direction, justify-content, align-items 등
Flex Items : flex-grow, flex-shrink, align-self 등

배포

Vercel 사용하여 배포했습니다.
https://vanilla-todo-kja1wl5px-kyoul10121s-projects.vercel.app

corinthionia

안녕하세요 윤일 님!
과제 하시느라 고생 많으셨습니다 🙂 코멘트에도 남기긴 했지만 완료/삭제 함수가 추가 함수 내부에 구현되어 있어 이를 따로 분리해 보시면 좋을 것 같아요! reset.css 까지 찾아서 적용하시고 노력이 돋보이는 과제였습니다 👍🏻 코멘트 몇 개 남겼으니 확인 부탁드려요 🙌

corinthionia · 2024-08-28T11:52:59Z

reset.css

+   v2.0 | 20110126
+   License: none (public domain)
+*/
+


우와 이런 것까지 적용해 보셨군요 👍🏻👍🏻
reset.css는 말 그대로 기본 스타일을 모두 초기화해 버려서 일일이 새로 지정하기 번거로울 수 있어요! 비슷하게 normalize.css라는 게 있는데, 이건 브라우저마다 상이한 부분만 스타일을 통일시켜 주는 역할을 합니다 🙂 이것도 알아보시면 좋을 것 같아요 🙌

이런 게 있다는 걸 윤일님 덕분에 처음 알고 갑니다..🫢 기본적으로 적용된 마진 때문에 고민이 많았는데 이런 방법이 있었다니..!!
참고해서 다음 과제부터 적용해보겠습니다🥰🥰

맞아요! 모두 초기화하는 reset.css보단 부분적으로 초기화해주는 normalize.css가 편리한 거 같아요! 다음 과제에 바로 적용해보려구요👍🏻👍🏻👍🏻

corinthionia · 2024-08-28T11:53:38Z

script.js

+let todoList = document.getElementById('todo-list'); // 할 일 리스트창
+let addTodoBtn = document.getElementById('add-todo-btn'); // 버튼
+let doneTodoList = document.getElementById('done-todo-list'); // 완료된 할 일 리스트창
+let todoListTitle = document.getElementById('todo-list-title'); // 할 일 제목
+let doneTodoListTitle = document.getElementById('done-todo-list-title'); // 완료된 할 일 제목


값이 재할당되는 경우가 아니라면 const 키워드를 사용하는 게 좋습니다!

corinthionia · 2024-08-28T11:57:28Z

script.js

이런 처리 아주 좋습니다! 👍🏻

corinthionia · 2024-08-28T11:58:00Z

script.js

+    todoList.appendChild($li);
+    todoInput.value = '';
+    todoInput.focus();


디테일을 많이 챙기셨네요~

corinthionia · 2024-08-28T12:02:14Z

script.js

+  }
+  // 완료
+  function doneTodo(event) {


addTodo() 안에 삭제하는 로직과 doneTodo()이 함께 있는데 따로 분리해 보면 좋을 것 같아요!

또 doneTodo()에서 인자로 받는 event는 사용하지 않으니 지워도 될 것 같습니다!

corinthionia · 2024-08-28T12:09:30Z

script.js

id 프로퍼티를 추가로 지정하신 이유가 있나요? id 값은 고유하게 사용되어야 하는데 완료한 일에 동일한 id가 사용되어 적절하지 않을 것 같습니다!

li태그와 button태그에 style을 추가하려고 넣었는데 여러 개가 추가될 수 있단 점을 간과했네요 ! 수정하겠습니다 😀

corinthionia · 2024-08-28T12:18:58Z

script.js

+  let count = todoList.getElementsByTagName('li').length;
+  todoListTitle.innerText = `📋 TO DO (${count})`;
+}


위의 사진처럼 초기 로딩 시 todoCount 함수가 호출되지 않아 투두의 개수가 카운팅되지 않고 있습니다! (초기에는 0으로 보여지는 게 더 일관성 있겠죠?) 이럴 때에는 DOMContentLoaded 이벤트를 사용할 수 있습니다 🙂 DOMContentLoaded 이벤트는 브라우저가 DOM 트리를 완성하는 즉시 발생하는 이벤트로, 아래 코드처럼 document 객체에 이벤트 리스너를 등록하여 사용할 수 있습니다.

document.addEventListener('DOMContentLoaded', () => { todoCount(); });

이걸 사용하면 localStorage에서 초기에 데이터를 불러오는 데에도 사용할 수 있지 않을까요~?

참고 자료 - DOMContentLoaded, load, beforeunload, unload 이벤트

corinthionia · 2024-08-28T12:21:15Z

style.css

+  align-items: center;
+  justify-content: center;
+  background: radial-gradient(#e66465, #9198e5);


그라데이션 예쁘네욤

corinthionia · 2024-08-28T12:21:30Z

style.css

+}
+.header {
+  /* justify-content: center; */


안 쓰는 코드는 지워 주세요!

corinthionia · 2024-08-28T12:24:04Z

style.css

+  height: 60%;
+  margin: 1%;
+  border: 0px;
+  background-color: transparent;
+}


버튼에 cursor: pointer 옵션을 추가하여 hover 시 커서 모양을 바꾸는 것도 적용하면 좋을 것 같아요~

addTodo와 doneTodo의 기능 분리, 할 일 삭제, 할 일 개수 업데이트 기능 함수 세분화

jejukyj

변수명과 클래스명을 어떤 기능을 하는 요소인지 직관적으로 명시해두셔서 코드 보기 편했습니다😆
css reset 하는 법 등 유용한 지식도 얻고 가요😚

jejukyj · 2024-08-29T02:27:54Z

reset.css

+   v2.0 | 20110126
+   License: none (public domain)
+*/
+


이런 게 있다는 걸 윤일님 덕분에 처음 알고 갑니다..🫢 기본적으로 적용된 마진 때문에 고민이 많았는데 이런 방법이 있었다니..!!
참고해서 다음 과제부터 적용해보겠습니다🥰🥰

jejukyj · 2024-08-29T02:35:33Z

script.js

+    alert('공백은 입력할 수 없습니다.');
+  }


윤일님도 truthy로 조건문을 구성하셨네요! '==='을 사용하는 것보다 훨씬 깔끔한 것 같아요

jejukyj · 2024-08-29T02:42:05Z

script.js

+  todoList.appendChild($li);
+  todoInput.value = ''; // 입력 필드 초기화
+  todoInput.focus(); // 입력 필드로 커서 이동


오 사용자의 편의성을 고려한 코드 아주 좋아요👍

ongheong · 2024-08-29T01:59:29Z

index.html

-    <div class="container"></div>
+    <div class="container">
+      <div class="header">
+        <header>📚 투두리스트</header>


직관적인 시멘틱 태그 사용 아주 좋습니다🙌

ongheong · 2024-08-29T02:05:07Z

reset.css

+blockquote:before,
+blockquote:after,
+q:before,
+q:after {
+  content: '';
+  content: none;
+}


blockquote와 q를 :before, :after 요소를 적용해서 이와 같이 스타일링한 이유가 궁금해요!

reset.css는 제가 초기화한게 아닌 양식입니다! 제가 사용한 양식 사이트 올려드릴게요.

사용하면서 느낀 점은 모든 것을 다 초기화 해주기 때문에 <h1>태그 같은 경우에는 폰트나 굵기가 다 사라져서 태그를 사용하는 의미가 사라진다고 느꼈습니다. 그래서 위 코드에서는 h태그 관련 초기화를 삭제하고 사용했습니다. 이러한 점들 때문에 위에서 주현님께서 추천해주신 normalize.css를 사용해보려고 합니다! 여은님도 한번 사용해보시면 좋을 거 같아요👍🏻👍🏻👍🏻

오! 이런게 있는지 몰랐어요! 기본 태그에서 필요없는 속성을 한번에 제거할 수 있어서 너무 편하네요😀 또 새롭게 알아갑니당

ongheong · 2024-08-29T02:11:26Z

script.js

li와 btn 변수 앞에 $기호를 붙이는 표기법이 궁금해요! DOM 요소인 점을 나타내는걸까요??👀

달러($)기호는 document.getElementById()의 아이디 값이나, document.querySelector()의 태그 값 혹은 아이디나 클래스처럼 한 개만 선언할 때, 유일한 변수를 표시하기 위해 사용했어요! 하지만 41번째 줄에서는 doneLi를 선언하면서 달러 기호를 붙이지 않아서 일관성이 없는 부분 때문에 수정하려고 합니다!

오홍 그렇군요! 저도 비슷한 상황에서 한번 적용해보겠습니다!

ongheong · 2024-08-29T02:16:41Z

script.js

+    alert('공백은 입력할 수 없습니다.');
+  }


공백 입력 방지를 알리는 부분 좋아요! 추가로 return 문을 넣는다면 공백 입력된 li 태그가 생성되는 것을 완전히 막을 수 있을 것 같습니다!

return으로 공백 방지 조건 후 함수를 종료하면 다른 경우 없이 막을 수 있겠네요!! 정확한 피드백 감사합니다😊

ongheong · 2024-08-29T02:27:39Z

script.js

+  // 삭제 버튼 클릭 시 해당 li 삭제
+  $btn.addEventListener('click', (e) => {
+    e.stopPropagation(); // 이벤트 전파 막기 -> li태그의 자식요소로 삭제버튼이 있기 때문에 버튼을 눌러도 완료항목으로 이동하는 문제발생


오옹 e.preventDefault()에 대해서만 알고 있었는데, 부모 엘리먼트에 이벤트 전파를 막는 e.stopPropagation()는 처음 알았네요! 좋은 지식 얻고갑니다👍🏻👍🏻

네 맞습니다!! 다른 방법으로는 HTML태그를 세분화해서 특정 태그에만 클릭이벤트를 적용하면 e.stopPropagation()을 사용 하지 않아도 된다는 사실까지 알아두시면 좋을 거 같아요🥰

ongheong

안녕하세요 윤일님! 이번 주차에 윤일님의 코드리뷰를 하게 되어 기쁩니다!
기능 구현을 최우선으로 한 코드라고 하셨지만 함수별로 기능을 잘 구분하여 나눠주셔서 보기 편했습니다! 투두리스트 페이지의 스타일도 깔끔하고 좋아요!
저도 이번 미션을 하면서 부족한 점을 많이 느꼈는데, 앞으로도 미션 해결하면서 같이 실력 키워나가봐요~~~!!

corinthionia · 2024-08-30T08:29:09Z

script.js

+function addTodo(event) {
+  event.preventDefault(); // form 태그의 기본 새로고침 기능 지우기


주석 굿굿!

corinthionia · 2024-08-30T08:30:19Z

script.js

해당 변수들이 재할당되는 일이 없기 때문에 이런 경우 가급적이면 const 키워드를 사용하는 게 좋습니다!

corinthionia · 2024-08-30T08:36:10Z

script.js

+
+  $li.innerText = todoText;
+  todoList.appendChild($li);


append()와 appendChild()의 차이를 아시나요?!
둘 다 비슷한 기능을 해서 해당 코드에서는 크게 상관은 없지만, 이번 기회에 알아보는 것도 좋을 것 같습니다 🙂
참고자료 - append와 appendChild

corinthionia · 2024-08-30T08:47:14Z

script.js

+
+// 할 일 개수 업데이트
+function todoFlagCount(isTodo) {


(개인적인 의견입니다)

할 일 개수를 카운팅 하는 함수를 따로 빼고 중복을 줄인 점 아주 좋습니다!! 다만, 해당 함수를 사용할 때 todoFlagCount(true) 형식으로 사용하게 되는데, 이러면 '투두 플래그(?)를 카운팅 하겠다', '투두 플래그(?)를 카운팅 하지 않겠다'라고 생각할 것 같아요. 그러면 결국 todoFlagCount가 뭐지? 하면서 해당 함수를 찾아보게 되고, 다시 읽어 보면서 그 의미를 파악하는 데 시간이 걸릴 것 같습니다 🥵

말이 길어지긴 했는데, 결국은 함수명이 조금 모호한 것 같다는 생각입니다! 그래서 차라리 아래처럼 todoFlagCount 함수의 인자를 객체 형태로 받으면 key와 value를 명시하면서 어떠한 값이 true일 때를 나타내는지 알기 쉬울 것 같아요!

Suggested change

function todoFlagCount(isTodo) {

function todoFlagCount({ isTodo }) { ... }

todoFlagCount({ isTodo: true });

todoFlagCount({ isTodo: false });

그리고 이건 다른 말이긴 하지만,, 제가 투두리스트를 만들었을 때에는 할일/완료한일 목록을 각각 배열에 저장하여 관리했습니다! 그러면 할일 개수를 셀 때 배열의 length 값을 사용하면 되니까 편하더라구요!

아 배열에 저장하면 개수를 셀 때나 가져올 때가 편하겠군요! react로 할 때 반영해보겠습니다!
함수의 기능을 나누면서 이게 맞는걸까.. 고민했는데 알려주신대로 함수의 인자를 객체 형태로 받으면 가독성이 더 좋아질 거 같습니다!
감사합니다☺️

corinthionia · 2024-09-01T08:30:09Z

style.css

+  flex-direction: row;
+}
+header {
+  margin: 5% 5% 0;
+  font-size: 24px;
+}


.header가 <header>를 감싸는 부분 같은데, 이럴 경우 wrapper를 붙여 .header-wrapper와 같은 이름을 많이 사용합니다!

감사합니다 바로 반영했습니다 !

cabrerainbu added 9 commits August 27, 2024 16:09

test commit

6a081da

prettierrc 초기 설정파일 추가

2abeb8f

초기화 css파일 추가

2ca1a6a

기본 style 추가

010c469

할일 추가 기능 구현

8c568aa

스타일: 삭제 버튼, ul태그에 스크롤 추가

5d54979

기능: 삭제,완료 항목 추가

337b578

todo count 기능 구현

a311f77

스타일: 버튼, 완료 목록

84b9511

corinthionia reviewed Aug 28, 2024

View reviewed changes

cabrerainbu added 2 commits August 29, 2024 01:08

♻️ Refactor:

189d5e3

addTodo와 doneTodo의 기능 분리, 할 일 삭제, 할 일 개수 업데이트 기능 함수 세분화

Chore: 불필요한 주석 삭제

c336ed4

jejukyj reviewed Aug 29, 2024

View reviewed changes

ongheong reviewed Aug 29, 2024

View reviewed changes

This comment was marked as duplicate.

Sign in to view

ongheong reviewed Aug 29, 2024

View reviewed changes

ReFactor(Style): setAttribute 사용하는 대신, css 선택자로 스타일 적용되도록 리팩토링

0a66a7c

corinthionia reviewed Sep 1, 2024

View reviewed changes

cabrerainbu added 2 commits September 2, 2024 09:14

chore: header-wrapper로 클래스명 수정

0157ce0

Refactor: let을 const로 수정

874f411

		function addTodo(event) {
		event.preventDefault(); // form 태그의 기본 새로고침 기능 지우기


		// 할 일 개수 업데이트
		function todoFlagCount(isTodo) {

-function todoFlagCount(isTodo) {
+function todoFlagCount({ isTodo }) { ... }
+todoFlagCount({ isTodo: true });
+todoFlagCount({ isTodo: false });

[1주차] 김윤일 미션 제출합니다. #4

Are you sure you want to change the base?

[1주차] 김윤일 미션 제출합니다. #4

Conversation

cabrerainbu commented Aug 27, 2024 • edited Loading

알게 된 부분

개선하고 싶은 부분

KEY QUESTION

배포

corinthionia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corinthionia Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jejukyj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

This comment was marked as duplicate.

ongheong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cabrerainbu commented Aug 27, 2024 •

edited

Loading

corinthionia Aug 28, 2024 •

edited

Loading