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주차] 지민재 미션 제출합니다. #7

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

Conversation

mimizae
Copy link

@mimizae mimizae commented Sep 7, 2024

🍀배포 링크

🔥1주차 미션을 수행하며...

  • React와 같은 라이브러리 없이 js만을 가지고 무언가를 만들어 본 게 너무 오랜만이라 까먹은 개념들이 많아서 처음 과제를 시작하려니 조금은 막막했습니다. 이전에 스터디한 내용들이나 구글링으로 조금씩 감을 찾아가며 코드를 한 자 한 자 적었지만 계속 감만 대충 아는 느낌이라 확실하게 코드의 전체적인 동작을 이해하기 위해 주석을 열심히 썼습니다. (주석을 가독성 있게 쓴 것 같지는 않습니다... ㅠㅠ) 그러나 찾아보면 찾아볼 수록 스스로 여전히 javascript에 대한 이해도가 부족하다는 생각이 들어 이번 미션에서는 js를 활용하기 보단 공부해서 바로 적용하는 방식으로 임했다 보니 디자인에 좀 더 신경을 쓰지 못하고 더 많은 기능을 추가하지 못한 것이 아쉬움으로 남습니다.

  • key question을 공부하기 전에는 DOM이 무엇인지 알고 js로 html를 조작하는 것이 아니라, 주입식 교육처럼 'js는 기능을 추가할 수 있는 언어이다.' 라고 알았는데, 답변을 공부한 후 js가 어떤 메서드로 어떻게 html에 접근해서 요소를 추가하고 조작하는지 DOM이 html과 js를 이어주는 인터페이스 역할이라는 DOM의 정확한 의미와 기능을 알게 될 수 있어 의미있는 시간이었습니다!

  • 캡처링과 버블링을 공부하며 이벤트의 흐름에 대해서도 처음으로 깊게 이해할 수 있었습니다. 그동안 내가 썼던 코드들에서 왜 이상한 컴포넌트에서도 의도하지 않은 이벤트가 발생하는 건지 아리송할 때가 많았는데, 이벤트 버블링이 기본값으로 설정되어 있기 때문에 그런 현상이 발생했다는 것을 알았습니다. 그러나 아직 체화한 것은 아니기에 미션 제출 후 이벤트 흐름과 이벤트 흐름 제어에 대해 추가로 공부할 계획입니다.

  • 클로저에 대해서는 개념부터 잘 와닿지 않아 자세하게 공부하지 못했습니다. 마감기한을 생각하다 보니 마음이 급해서인 것 같기도 해 이것도 미션 제출 후 추가적으로 공부할 예정입니다.

  • 이렇게 코드 리뷰를 통해 가감없는 피드백을 받을 기회가 얼마 없었어서 성장할 수 있는 소중한 기회라고 생각합니다. 또 항상 과제를 끝내면 제출하기 급급했는데 제출이 끝이 아니라 어떤 것을 느꼈고 뭘 배웠는지 회고할 수 있다는 점이 가장 좋았습니다.

거침없는 피드백 부탁 드립니다!! 💙

🗝️ 1. DOM은 무엇인가요?

DOM(Document Object Model)은 HTML 문서의 구조화된 표현을 제공하며 javaScript와 같은 프로그래밍 언어가 DOM 구조에 접근할 수 있는 방법을 제공하여 그들이 문서 구조, 스타일, 내용 등을 변경할 수 있게 돕는다.

image

DOM은 nodes와 objects로 문서를 표현한다. 이들은 웹 페이지(HTML)를 스크립트 또는 프로그래밍 언어들에서 사용될 수 있게 연결시켜주는 역할을 담당한다.

이때, 프로그래밍 언어인 JavaScript는 HTML에 document라는 전역객체를 통해 접근할 수 있다.

⇒ JavaScript의 document 객체는 DOM 구조를 접근하는 관문이며, document 객체는 HTML 문서를 나타낸다!!

DOM의 주요 특징 및 개념

  1. 트리 구조: DOM은 트리 구조로 표현되며, 각 노드는 웹 페이지의 다양한 부분 (예: 요소, 속성, 텍스트 등)을 나타낸다.
  2. 동적 콘텐츠 업데이트: DOM을 통해 웹 페이지의 콘텐츠를 동적으로 변경할 수 있다. 예를 들어, 사용자가 버튼을 클릭했을 때 페이지의 내용이 즉시 바뀐다.
  3. 조작 가능: 자바스크립트를 사용하여 DOM 요소를 선택, 수정, 추가 또는 제거할 수 있다.

주요 DOM 관련 메서드 및 속성

  • 요소 선택 (접근)
    • document.getElementById(id): 주어진 ID를 가진 요소를 반환한다.
    • document.getElementsByTagName(name): 주어진 태그 이름을 가진 요소들의 목록을 반환한다.
    • document.getElementsByClassName(name): 주어진 클래스 이름을 가진 요소들의 목록을 반환한다.
    • document.querySelector(selector): 주어진 CSS 선택자와 일치하는 첫 번째 요소를 반환한다.
    • document.querySelectorAll(selector): 주어진 CSS 선택자와 일치하는 모든 요소의 목록을 반환한다.
  • 요소 조작
    • element.textContent 또는 element.innerText: 요소의 텍스트 내용을 가져오거나 설정한다.
    • element.innerHTML: 요소의 내부 HTML을 가져오거나 설정한다.
    • element.setAttribute(name, value): 요소의 속성 값을 설정한다.
    • element.getAttribute(name): 요소의 속성 값을 가져온다.
    • element.appendChild(childElement): 요소에 자식 요소를 추가한다.
    • element.removeChild(childElement): 요소에서 자식 요소를 제거한다.
  • 이벤트 리스너
    • element.addEventListener(event, function): 요소에 이벤트 리스너를 추가한다.
    • element.removeEventListener(event, function): 요소에서 이벤트 리스너를 제거한다.

image

→ (이해한대로, 한 문장으로 정리하자면) DOM은 HTML 문서를 브라우저가 이해할 수 있도록 만든 Tree 자료구조이며 JS가 HTML 문서를 조작할 수 있도록 돕는 인터페이스이다.

그러니까, html 문서 자체로 웹 페이지를 만드는 방법도 있고 js를 통해 html을 조작해서 웹 페이지를 만드는 방법이 있다는 것. 이때, js가 html을 조작할 수 있도록 도와주는 interface가 DOM! DOM의 document.getElementById(id) 이러한 메서드 등을 이용하는 것.

🗝️ 2. 이벤트 흐름 제어(버블링 & 캡처링)이 무엇인가요?

HTML 문서의 각 엘리먼트들은 태그 안의 태그가 위치하는 식으로 계층적으로 이루어짐을 볼 수 있다. 이러한 계층적 구조 특징 때문에 만일 HTML 요소에 이벤트가 발생할 경우 연쇄적 이벤트 흐름이 일어나게 된다.

예를 들어, 가장 자식 요소인 p 박스를 클릭하면 onclick 이벤트가 p 뿐만 아니라 그의 부모인 div와 form 요소에서도 발생한다.

<form onclick="alert('form')">FORM    
	<div onclick="alert('div')">DIV    	
		<p onclick="alert('p')">P</p>    
	</div>
</form>

이러한 현상을 이벤트 전파라고 부르며, 전파 방향에 따라 버블링과 캡처링으로 구분된다.
image

  1. 버블링: 자식 요소에서 발생한 이벤트가 바깥 부모 요소로 전파 (기본 값)
  2. 캡쳐링: 자식요소에서 발생한 이벤트가 부모 요소부터 시작해서 안쪽의 자식 요소까지 도달하는 것!

버블링, 안 쪽에서 바깥쪽으로!

image

  • 자바스크립트 addEventListener() 함수로 요소의 이벤트를 등록하면 기본적으로 버블링 전파 방식으로 이벤트가 흐르게 되어, 만일 자식 요소에 이벤트가 발생하면 부모 요소도 버블링 통해 이벤트가 전파되어 리스너가 호출되게 된다.
  • 사진에서 만약 가장 안쪽 요소인 child를 클릭하면 child의 이벤트 리스너가 실행되고 그 밖의 parent 요소에도 이벤트가 전달되어 만약 동일한 이벤트 리스너가 등록되어있다면 실행되고 또 그 밖의 ancestor 요소에 이벤트가 전달되어 실행된다!

캡처링? 상위에서 하위로!

image

버블링과 완전한 반대이다. 이벤트가 상위에서부터 하위로 전달된다. 만약 가장 안쪽 요소를 클릭했다면 이벤트가 ancestor 요소에 먼저 전달되어 이벤트 리스너가 실행되고, 다음으로 안쪽 요소인 parent 요소에 전달되어 이벤트 리스너가 실행된 후 마지막으로 child 요소에 이벤트가 전달되어 실행된다.

** ⚠️ 버블링 & 캡처링 문제점**

만일 부모와 자식 둘 다 이벤트를 등록한 상태에서, 자식 요소만 클릭 했을 때만 이벤트를 발생하고 부모 요소는 이벤트를 발생시키고 싶지 않은 상황이 있다면?

이때 브라우저는 기본적으로 캡처링 - 버블링으로 동작 되기 때문에 이벤트 동작 자체를 바꿀 순 없다. 그리고 html 구조를 봐도  자식 요소가 부모 요소 영역 안에 위치하고 있기 때문에 자식 요소만을 클릭하였다 하더라도 다른 시각으로 보면 부모 요소도 클릭한 셈이 되어버린다.

** 🔧 해결 방법**

아래의 메소드들을 사용한다!

e.stopPropagation(): stopPropagation() 메소드를 호출하면 버블링 또는 캡처링 설정에 따라 상위, 하위로 가는 이벤트 전파를 막을 수 있다.

e.stopImmediatePropagation(): stopImmediatePropagation() 메소드를 호출하면, 이벤트 전파와 더불어 형제 이벤트 실행을 중지한다.

e.preventDefault() : 이벤트 전파 중지 + 형제 이벤트 실행 중지 + 이벤트 기본 동작 중지

🗝️ 3. 클로저와 스코프가 무엇인가요?

유효범위(Scope)

프로그래밍 언어에서 유효 범위는 어느 범위까지 참조하는지를 뜻한다.

유효범위의 종류에는 크게 두 가지가 있다.

  • 전역 스코프 (Global scope) : 스크립트 전체에서 참조되는 것을 의미하며, 어느 곳에서든 참조 된다.
  • 지역 스코프 (Local scope) : 정의된 함수 내에서만 참조되는 것을 의미하며, 밖에서는 참조 되지 않는다.

Scope의 특징

  • 함수 단위의 유효범위(function-level-scope) : 함수 코드 블럭 내에서 선언된 변수는 함수 코드 블럭 내에서만 유효하고 함수 외부에서는 유효하지 않다.
  • 글로벌 영역에 변수를 선언하면 이 변수는 어느 곳에서든지 참조할 수 있는 global scope를 갖는 전역 변수가 된다.
  • 암묵적 선언(implied globals) : 명시적으로 변수 앞에 var를 붙여주지 않으면 암묵적 전역변수가 된다.
  • Lexical scoping(Static scoping) : 자바스크립트는 함수가 선언된 시점에서의 유효범위를 갖는다.
var x = 1;

function foo() {
  var x = 10;
  bar();
}

function bar() {
  console.log(x);
}

foo();// 1
bar();// 1

위 예제의 실행 결과는 함수 bar의 상위 스코프가 무엇인지에 따라 결정된다.

함수를 어디서 호출하였는지에 따라 상위 스코프를 결정한다면 함수 bar의 상위 스코프는 함수 foo와 전역일 것이고,  함수를 어디서 선언하였는지에 따라 함수의 스코프를 결정한다면 함수 bar의 스코프는 전역일 것이다.

프로그래밍 언어는 이 두 가지 방식 중 하나의 방식으로 상위 스코프를 결정한다. 첫 번째 방식을 **동적 스코프(Dynamic Scope)**라 하고, 두 번째 방식을 렉시컬 스코프(Lexical Scope) 또는 정적 스코프(Static Scope) 한다.

렉시컬 스코프는 함수를 어디서 호출하는지가 아니라 어디에 선언하였는지에 따라 결정한다. 자바스크립트는 렉시컬 스코프를 따르므로 함수를 선언한 시점에 상위 스코프가 결정된다. 함수를 어디에서 호출하였는지는 스코프 결정에 아무런 의미를 주지 않는다.

클로저(Closure)

  • 클로저는 함수가 선언된 환경의 (렉시컬) 스코프를 기억하여 함수가 스코프 밖에서 실행될 때에도 이 스코프에 접근할 수 있게 하는 기술

내부함수는 외부함수의 지역변수에 접근 할 수 있는데 외부함수의 실행이 끝나서 외부함수가 소멸된 이후에도 내부함수가 외부함수의 변수에 접근 할 수 있다.

이러한 메커니즘을 클로저라고 한다

Copy link

@hae2ni hae2ni left a comment

Choose a reason for hiding this comment

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

진짜 섬세하게 잘 짜신 것 같아요! 많이 배우고 갑니다!

Copy link

Choose a reason for hiding this comment

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

처음에 모두 영어로 할 게 아니라면
<html land='ko'>로 바꿔주는 것도 좋을 것 같아요!

index.html Outdated
<p class="count">✅ : <span class="totalCount">0</span> 🎉 : <span class="completedCount">0</span></p>
<form>
<input type="text" placeholder="할 일 추가">
<button id = "submitBtn">추가</button>
Copy link

Choose a reason for hiding this comment

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

form 태그 안에 있는 button 태그는 type도 같이 지엏utton 태그는 type도 같이 지정해주는 게 좋아요!

태그 내에서 을 사용할 때, 타입 명시가 없다면 'submit'이 기본적으로 처리가 되는데, 이때 버튼을 클릭하면 페이지가 새로 고침이 되는게 디폴트거든요!!!
크게 문제가 되진 않지만(form 태그 내부에 쓰이지 않았다면) form 태그 내부에 있는 button은 타입 명시가 없다면 자동적으로 submit으로 동작해 새로고침되므로 조금만 주의해 주셔도 좋을 것 같아요 😄

}
// 이미 같은 내용의 투두가 있는지 확인

const isDuplicate = todoList.some((todo) => todo.text === newTodo); //some 메서드, 배열의 각 요소를 순회하면서, 주어진 조건을 만족하는 요소가 하나라도 있으면 true를 반환하고, 조건을 만족하는 요소가 없으면 false를 반환
Copy link

Choose a reason for hiding this comment

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

헉 요건 생각지도 못한 요소였던 거 같아요!!! 좋은 기능,,, 잘 알아갑니당,,,

Comment on lines +4 to +9
const today = new Date(); // 현재 날짜와 시간을 가져오는 Date 객체
const options = { month: "long", day: "numeric", weekday: "long" };
// 날짜, 요일 등 포맷 시 month와 weekday는 긴 형식으로 (9월, 목요일) day는 숫자 형식 (5, 25)

const formattedDate = today.toLocaleDateString("ko-KR", options); // options 형식의 한국어 날짜
document.querySelector(".date").textContent = formattedDate;
Copy link

Choose a reason for hiding this comment

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

저는 new Date()로 현재 날짜와 시간을 가져왔을 때,
각각
const day= today.getDate() 와 같은 메소드를 사용했었거든요!
지민재님 방식도 너무너무 좋은 방식인 거 같아요!
그치만 월, 일 정도만 필요할 때에는 getDate()와 같은 메소드를 사용하면 조금 더 간소화할 수 있을 것 같아요!


function createList() {
const newTodo = todoInput.value.trim(); // 문자열 앞 뒤 공백을 제거하는 trim을 이용, 사용자가 input에 입력한 todo를 저장
if (newTodo === ""){
Copy link

Choose a reason for hiding this comment

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

아마 빈문자열 ""는 falsy한 성질을 가지기 때문에
if (!newTodo) 와 같은 방식으로도 바꿀 수 있을 것 같습니다!

}

function displayTodo(todoText, isCompleted) {
const li = document.createElement("li"); // 새로운 <li>요소 생성. 하나의 todo를 나타냄
Copy link

Choose a reason for hiding this comment

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

그냥 기본 tag이름보다 명확한 네이밍을 해주는 게 좋을 것 같습니다

script.js Outdated
Comment on lines 91 to 93
li.appendChild(checkbox); // 새로 만든 요소 (체크박스, 텍스트, 삭제 버튼)을 <li> 요소에 추가해서 하나의 todo 항목 완성!
li.appendChild(todoSpan); // 할 일 텍스트를 담은 <span> 요소 추가
li.appendChild(deleteBtn); // 삭제 버튼 추가
Copy link

Choose a reason for hiding this comment

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

appendChild는 하나의 요소만 자식요소로 넣을 수 있다고 해요!
만약 여러 요소를 하나의 태그 안에 자식 요소로 넣고 싶다면
append를 이용하는 것도 좋은 방법일 것 같아요! 보다 코드가 간소화되니까요!

Suggested change
li.appendChild(checkbox); // 새로 만든 요소 (체크박스, 텍스트, 삭제 버튼)을 <li> 요소에 추가해서 하나의 todo 항목 완성!
li.appendChild(todoSpan); // 할 일 텍스트를 담은 <span> 요소 추가
li.appendChild(deleteBtn); // 삭제 버튼 추가
li.append(checkbox, todoSpan, deleteBtn)

Copy link
Author

Choose a reason for hiding this comment

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

헉... 저도 이거 똑같은 내용을 썼답니다! 물론 제가 수정해야 한다는 쪽으로요... ㅎㅎ 코드 리뷰를 통해 하나 더 배우게 되어 기쁘네요 감사합니다 🔥🔥

script.js Outdated

completedCountElem.textContent = completedTodos;
totalCountElem.textContent = totalTodos;
if (totalTodos > 0 && completedTodos === totalTodos) {
Copy link

Choose a reason for hiding this comment

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

너무 섬세한 거 같아요ㅎㅎㅎㅎ

Copy link
Member

@ddhelop ddhelop left a comment

Choose a reason for hiding this comment

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

너무 귀엽고 멋진 과제하시느라 고생하셨습니다!! 🙂

과제 링크와 코드를 보면서 작은 디테일들 찾는 재미로 리뷰했던것 같아요.

다음과제에서는 각 함수에 대한 설명을 주석으로 간단하게 작성하면 유지보수하기도 편하고 코드리뷰하기도 정말 편할 것 같아요! 추가적으로 할일을 모두 완료된 상태에서 완료된 할일을 삭제하면 완료되었다는 alert창이 뜨는 버그가 있는 것 같아요! 이 부분만 예외처리한다면 더 완벽할 것 같습니다!!

Comment on lines +36 to +41
const isDuplicate = todoList.some((todo) => todo.text === newTodo); //some 메서드, 배열의 각 요소를 순회하면서, 주어진 조건을 만족하는 요소가 하나라도 있으면 true를 반환하고, 조건을 만족하는 요소가 없으면 false를 반환
if (isDuplicate) {
alert("이미 동일한 투두가 있습니다!👏🏻"); // 알림창으로 중복 투두 알림
todoInput.value = "";
return; // 중복되면 함수 종료
}
Copy link
Member

Choose a reason for hiding this comment

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

중복 예외처리를 some() 메서드를 사용하여 이미 동일한 할 일이 있는지 검사 로직을 구현한점이 인상적이예요!

Comment on lines +111 to +114
li.classList.add('remove'); // 삭제될 요소의 스타일링을 위해 class name 부여
li.addEventListener('animationend', () => {
li.remove(); // 애니메이션 끝난 후 실제로 제거
});
Copy link
Member

Choose a reason for hiding this comment

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

할 일 삭제시 애니메이션 및 애니메이션이 끝난 후에 DOM에서 완전히 제거하는 방식을 적용한 디테일 멋져요오!

script.js Outdated
Comment on lines 124 to 125
if (totalTodos > 0 && completedTodos === totalTodos) {
alert('축하합니다! 모든 할 일을 완료하셨습니다! 🎉');
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

@mimizae mimizae Sep 9, 2024

Choose a reason for hiding this comment

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

ㅎㅎ 상태변수를 추가해서 할 일이 모두 완료되었고, 그 상태가 처음으로!! 발생한 경우에만 alert 표시를 하도록 수정했습니다 :)
상태변수를 처음에 false로 초기화한 후 새로운 todo가 추가될 때, todo를 완료했을 때 updateCount 함수가 호출되어 전체 todo 개수와 완료된 todo 개수를 계산하게 하고 만약

if (totalTodos > 0 && completedTodos === totalTodos && !isAllCompleted) {
      alert('축하합니다! 모든 할 일을 완료하셨습니다! 🎉');
      isAllCompleted = true; // 모든 할 일이 완료되었다고 기록
    } else if (completedTodos < totalTodos) {
      // 할 일이 삭제되거나 새로 추가되면 상태를 초기화
      isAllCompleted = false;
    } 

이럴 경우에만 alert가 띄워지도록 했습니다! 모든 할 일이 완료되지 않았을 때, false였을 테니 처음으로 모든 할 일이 완료 되면 그 상태가 toggle 되어 true가 될 때 alert가 뜨고 다 완료된 todo 중 하나를 삭제해도 완료된 todo와 전체 todo 개수가 동일해 isAllCompleted = true 일 테니 alert가 안 뜨도록... 했습니다!!

너무 장황하네요... ㅠㅠ 하지만 덕분에 이 에러를 발견했고 상태 변수를 활용해 특정 시점에만 이벤트가 발생하도록 하는 법을 공부할 수 있어 감사했습니다 👍🏻😍

Copy link
Author

Choose a reason for hiding this comment

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

헉... 새로고침 되었을 때 상태 변수가 초기화 되어 또 다시 계속 alert가 떠서 바로 localstorage에 저장해 새로 고침을 해도 이전 값을 기억하도록 수정했습니답

@@ -1 +1,129 @@
//😍CEOS 20기 프론트엔드 파이팅😍

document.addEventListener("DOMContentLoaded", function () {
Copy link
Member

Choose a reason for hiding this comment

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

DOMContentLoaded 이벤트를 사용한 것 멋져요!! 저도 잘 안쓰는 개념이였는데, 덕분에 좋은 개념 하나 챙겨갑니다!!
"HTML 문서의 DOM이 모두 로드된 후에 JavaScript 코드가 안전하게 실행되도록 보장"

// 날짜, 요일 등 포맷 시 month와 weekday는 긴 형식으로 (9월, 목요일) day는 숫자 형식 (5, 25)

const formattedDate = today.toLocaleDateString("ko-KR", options); // options 형식의 한국어 날짜
document.querySelector(".date").textContent = formattedDate;
Copy link
Member

Choose a reason for hiding this comment

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

이부분도 같은 요소를 반복적으로 사용할 때는 DOM 접근을 최소화하기 위해 캐싱하는 것이 좋을 것 같아요!

Suggested change
document.querySelector(".date").textContent = formattedDate;
const dateElement = document.querySelector(".date");```

return; // 중복되면 함수 종료
}

todoList.push({ text: newTodo, completed: false }); // 배열에 입력 값 저장
Copy link
Member

Choose a reason for hiding this comment

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

현재는 todoList의 항목이 텍스트 값만으로 관리되고 있는 것 같아요. 물론 그에 대한 중복 입력 예외처리가 되어있지만, 하지만 일부 상황에서는 같은 이름의 할 일을 여러 번 추가하고 싶을 수 있음을 고려하면 각 할 일에 고유 ID를 추가하는 방법을 사용하면 해결할 수 있어요! 텍스트 중복을 허용하면서도 할 일의 개별적인 상태(완료 여부 등)를 관리할 수 있을 것 같아요 🤩

Suggested change
todoList.push({ text: newTodo, completed: false }); // 배열에 입력 값 저장
todoList.push({ id: Date.now(), text: newTodo, completed: false });

Copy link
Author

Choose a reason for hiding this comment

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

중복 입력 예외 처리를 한 이유가 고유 ID가 아닌 text로만 todo들을 관리하다 보니 중복 todo가 생겼을 때 하나의 todo를 완료 및 삭제를 해도 다른 중복 todo까지 동일한 이벤트가 적용되어 이 버그의 해결책으로 some()메서드를 이용해 같은 text를 가진 todo가 있으면 입력을 방지하는 것을 선택했었습니다 😂😂

근데 정말 중요한 todo라서 잊지 않기 위해 여러 개 중복 입력할 수도 있을 테고, 또 좀 더 복잡한 중복 입력 예외처리 보다는 고유 ID를 추가해 더 간결하게 기존에 발생했던 버그를 해결할 수 있을 것 같아 고유 ID 추가하는 방식으로 수정하겠습니다!
알려주셔서 감사합니다 🔥👍🏻👍🏻

});
}

function createList() {
Copy link
Member

Choose a reason for hiding this comment

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

createList() 함수 내부에서 유효성 검사, 중복 체크, 할 일 추가, UI 업데이트, 카운트 업데이트 등이 모두 처리되고 있는 것 같아요. 더 유지보수 가능하게 만들기 위해 각 부분을 개별 함수로 분리하는 것도 좋은 방법이므로, 리액트를 사용할때도 고려하면서 코드를 작성해보면 좋을 것 같아요 :)

}
.PageContainer{
display: flex;
width: 100vw;
Copy link
Member

Choose a reason for hiding this comment

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

vw는 브라우저 뷰포트 크기에 맞추는 단위라 이를 사용하는 것도 좋지만 저의 경우는 전체 너비의 경우에는 상대단위 (%)를 사용하는 것이 오류 발생률도 적어서 반응형 구현할 때 자주 씁니다. 민재님도 상대단위 한번 사용해보시고 편한 방법으로 사용해보세요!!

Copy link

@s-uxun s-uxun left a comment

Choose a reason for hiding this comment

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

안녕하세요. 민재 님!
이번 코드리뷰를 맡은 송유선입니다. 전체적으로 깔끔한 UI와 사용자를 고려한 UX가 인상적이었어요😄 특히 다양한 경우를 고려하여 alert를 구현하신 점에서 세심함을 느꼈습니다 ㅎㅎ 주석도 상세하게 달아주셔서 이해하기 더욱 편했던 것 같아요. 덕분에 많이 배웠습니다. 다음 과제도 함께 파이팅해요!

Comment on lines +100 to +109
function toggleTodoCompletion(todoText) { // 배열 todoList에서 특정 할 일 항목의 완료 상태를 토글시키는 역할
todoList = todoList.map((todo) => { // map 메서드는 배열의 각 항목을 변형하여 새로운 배열을 반환한다!!
if (todo.text === todoText) { // 순회하던 배열의 특정 원소의 text와 display 함수에서 props로 전달 받은 체크 상태가 변경된 todo의 text가 동일하다면?
return { ...todo, completed: !todo.completed }; // 그 원소의 completed 상태 토글
}
return todo;
});
saveStorage(); // todoList가 업데이트 되어 새롭게 localStorage에 저장
updateCounts(); // 체크 표시 상태에 따라 완료된 todo 개수 변경
}
Copy link

Choose a reason for hiding this comment

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

toggleTodoCompletion 함수를 보니까 map 메서드를 통해 모든 todolist를 순회하면서 todo의 상태를 바꾸고 있는 구조인 것 같아요. 물론 이 방식도 유효하지만 find() 메서드를 사용하여 좀 더 효율적으로 코드를 수정할 수 있을 것 같아요. 대략적으로 아래와 같이 바꾸면 어떨까요?

Suggested change
function toggleTodoCompletion(todoText) { // 배열 todoList에서 특정 할 일 항목의 완료 상태를 토글시키는 역할
todoList = todoList.map((todo) => { // map 메서드는 배열의 각 항목을 변형하여 새로운 배열을 반환한다!!
if (todo.text === todoText) { // 순회하던 배열의 특정 원소의 text와 display 함수에서 props로 전달 받은 체크 상태가 변경된 todo의 text가 동일하다면?
return { ...todo, completed: !todo.completed }; // 그 원소의 completed 상태 토글
}
return todo;
});
saveStorage(); // todoList가 업데이트 되어 새롭게 localStorage에 저장
updateCounts(); // 체크 표시 상태에 따라 완료된 todo 개수 변경
}
function toggleTodoCompletion(todoText) {
const todo = todoList.find((todo) => todo.text === todoText);
if (todo) {
todo.completed = !todo.completed;
saveStorage();
updateCounts();
}
}

map() 메서드는 배열의 모든 요소를 순회하면서 각 요소를 변형한 새로운 배열을 반환하는데, 이번 바닐라JS 투두리스트에서 완료 상태를 토글하는 작업은 굳이 배열 전체를 변형할 필요가 없을 것 같습니다. find() 메서드는 배열을 순회하다가 첫번째로 조건을 만족하는 요소를 찾으면 그 요소를 반환하고 순회를 멈추기 때문에 하나의 요소만 업데이트해야하는 토글 함수에 더 효율적인 방향이라고 생각합니다..! 가독성 면에서도 짧고 간단해서 보기가 수월할 것 같구요..!

map()은 일반적으로 불변성을 유지하기 위해 새로운 배열을 반환하는 상황에서 많이 사용된다고 알고 있는데, (제 생각으로는) 이 부분의 코드에서는 불변성 유지가 필수적이지 않은 것 같아요. 만약 리액트와 같은 상태 관리 시스템에서 개발을 한다면 어디서든 불변성을 유지하는 것이 매우 중요하지만, (리액트는 상태변화가 있을 때 렌더링을 트리거하기 때문에 불변성을 지켜야 변경된 상태만 효율적으로 감지할 수 있기 때문) 로컬 스토리지에 업데이트하는 단순한 데이터 처리에서는 불변성을 유지하지 않아도 성능에 문제가 생길 확률이 적습니다..!

불변성 유지 여부에 따른 코드 전개

color: rgb(94, 169, 139);
}

#submitBtn {
Copy link

Choose a reason for hiding this comment

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

submitBtn과 deleteBtn은 색상 외에 거의 비슷한 스타일을 가지고 있는데, 코드 상 너비와 배치 방식이 서로 다르게 설정되어 있습니다. (제출 버튼은 absolute 포지션, 삭제 버튼은 flexbox 통한 정렬) button 등으로 공통된 요소를 맞춰주신 다음에 id명, class명으로 세부적인 요소를 조절해주시면 통일성 측면에서도, 나중에 유지 보수 측면에서도 더욱 좋을 것 같아요!

.todoBox li {
display: flex;
width: calc(100% - 2.5rem);
box-shadow: 0 0 5px rgba(0, 0, 0, 0.11), 0 5px 5px rgba(0, 0, 0, 0.178);
Copy link

Choose a reason for hiding this comment

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

현재 동일한 box-shadow 속성이 여러 곳에서 쓰이고 있네요. 저도 이번에 공부하면서 처음 알게 된 건데.. 이 경우 css 변수를 활용해봐도 좋을 것 같아요!

:root {
    --box-shadow: 0 0.875rem 1.75rem rgba(0,0,0,0.25), 0 0.625rem 0.625rem rgba(0,0,0,0.22);
}

이렇게 css 파일에서 변수를 선언하면, 해당 변수는 전역에서 사용될 수 있게 돼요. (단, 어디에도 자동으로 적용되지는 않아요!) 저렇게 변수를 선언한 이후에는

main, .todoBox li {
    box-shadow: var(--box-shadow); /* 정의한 변수를 여기서 사용 */
}

이와 같이 var 을 활용해서 간단하게 표현할 수 있답니다..!

CSS 변수 정리

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