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주차] 김류원 미션 제출합니다. #10

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

Conversation

ryu-won
Copy link

@ryu-won ryu-won commented Sep 7, 2024

💗 느낀 점 💗

vercel 배포 링크 : https://vanilla-todo-20th-eight.vercel.app/

다른 분들은 어땠을 지 모르지만 저는 투두리스트 만드는 것도 살짝 어려웠어서 어려운 기능은 찾아서 공부하고 구현하는 과정이 힘들었던 거 같습니다. 하지만 이 과정에서 어느 부분이 부족한 지를 알 수 있었고, 그 부분에 대해서 더 공부하는 시간을 가졌습니다. 다음 스터디 전까지 2주 정도 남은 걸로 알고 있는데, 이 기간 동안 자바스크립트를 탄탄하게 더 공부 해야겠습니다. vercel 배포시 이미지 하나가 안 보였는데, 이 현상의 이유를 몰라서 이미지 src url를 깃헙이미지 url로 변경하였습니다. 혹시나 아시는 분이 있으면 저에게 알려주시면 감사하겠습니다! 아직 실력이 많이 부족하여 이번 과제를 잘 했는지 잘 모르겠지만 코멘트 많이 달아주시면 반영해서 공부하겠습니다!

💗[Key Questions]💗

1. DOM은 무엇인가요?

  • 문서 객체 모델(DOM)은 HTML 문서 구조를 객체로 표현한 것이다. HTML 문서에는 body, p 등 여러 태그가 문서의 구조를 이루고 있는데, 이런 여러 HTML 요소의 계층을 반영해서 만든 객체가 DOM이다. HTML 문서를 객체로 표현하면 JavaScript와 같은 스크립트 및 프로그래밍 언어가 웹페이지에 접근할 수 있다. 즉, DOM은 HTML로 구성된 웹 페이지와 스크립트 및 프로그래밍 언어를 연결시켜주는 역할을 한다. JavaScript에서는 대표적으로 HTML 요소의 id 선택자를 사용하는 document.getElementByld 메서드가 있다. DOM은 문서를 노드와 객체의 트리구조로 표현한다.
  • DOM 생성 순서
    HTML 웹 페이지는 HTML 파서에 의해 DOM으로 변환된다. 하지만 여기서 파서가 <script> 태그를 만나면 파서는 DOM 생성을 중지하고 자바스크립트 엔진이 script에 정의된 파일 및 코드를 실행한다. 스크립트 실행이 완료되면 다시 HTML 파서가 DOM 생성을 다시 시작한다. 즉, 브라우저는 동기적으로 HTML과 JavaScript를 처리하기 때문에 <script> 태그의 위치에 따라 DOM 생성이 느려질 수 있다. 그렇기 때문에 <script> 태그는 HTML 문서 맨 하단에 넣는 것이 좋다. DOM 생성이 지연되지 않고, 스크립트에 DOM을 접근하는 코드가 있다면 제대로 실행되지 않기 때문이다.

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

  • 이벤트 흐름 제어는 DOM에서 이벤트가 전파되는 방식을 말한다. 두 가지 방식이 있다:

    1. 버블링(Bubbling): 가장 안쪽의 요소에서 시작해 바깥쪽으로 이벤트가 전파된다. 기본적인 이벤트 전파 방식이다.
    2. 캡처링(Capturing): 가장 바깥쪽 요소에서 시작해 안쪽으로 이벤트가 전파된다. 보통 명시적으로 설정해야 사용할 수 있다.
      어떤 전파 방향을 사용할 것이냐는 자바스크립트 설정을 통해 제어할수 있으며 둘을 동시에도 사용이 가능하다. 자바스크립트 addEventListener() 함수의 3번째 매개변수로 true 값을 주면 해당 이벤트 타겟은 캡처링을 통해 이벤트를 전파받아 호출되게 된다.
  • 이벤트 전파흐름
    스크린샷 2024-09-07 오후 8 52 53

    이처럼 브라우저는 사용자로부터 이벤트가 발생하면 가장 상단의 요소부터 하위의 요소까지 내려오고 다시 거슬러 올라가는 식으로 이벤트를 전달하여 발생하도록 한다. 만일 타깃 요소까지 이벤트를 전파하는 과정에서 그의 부모, 조상에도 이벤트 리스너가 등록되어 있다면 실행되게 된다.

  • 장점
    이벤트 위임: 상위 요소에 하나의 이벤트 리스너만 추가하여 여러 하위 요소를 효율적으로 관리할 수 있다.
    코드 최적화: 이벤트 리스너 수를 줄여 메모리 사용을 최적화할 수 있다.

  • 버블링 + 캡처링 동시 설정하기
    다음과 같이 이벤트 리스너 함수를 두개 설정해주면 된다.

  // 버블링 호출
child.addEventListener("click", (e) => {
  console.log('child clicked');
})

// 캡처링 호출
child.addEventListener("click", (e) => {
  console.log('child clicked');
}, true)
  • 버블링 & 캡처링 문제점
    만일 부모와 자식 둘다 이벤트를 등록한 상태에서, 자식 요소만 클릭했을때만 이벤트를 발생하고 부모 요소는 이벤트를 발생시키고 싶지 않은 상황이 있을 것이다. 브라우저의 이벤트 구조를 바꿀수는 없고, 엘리먼트의 이벤트 전파를 방지 처리 하는 식으로 해결하여야 한다.

  • 이벤트 전파 방지 방법

    1. e.stopPropagation()
      상위, 하위로 가는 이벤트 전파를 막을 수 있다.
ancestor.addEventListener("click", (e) => {
e.stopPropagation() // 이벤트 전파 방지
print('ancestor')
})
  1. e.stopImmediatePropagation()
    이벤트 전파와 더불어 형제 이벤트 실행을 중지한다. 예를 들어 아래와 같이 동일한 child 요소의 이벤트 리스너가 2개 등록 되어 있을때, 어떠한 조건에서 클릭 이벤트를 두번 실행하지 않고 한번만 실행토록 하길 원한다면 유용하다.
child.addEventListener("click", (e) => {
    
    if(조건)
    	e.stopImmediatePropagation()
        
    print('child')
})

child.addEventListener("click", (e) => {
    print('child 2')
})

  1. e.target 으로 조건 걸어 방지
    복잡하지만 좀더 세심하게 이벤트 핸들러를 컨트롤하고 싶다면, 직접 조건 분기를 통해 일일히 지정해 줄 수 있다. 콜백 함수의 매개변수 e (event) 를 이용해 현재 호출된 타겟의 정보를 가져와 비교함으로써 제어가 가능한 원리이다.
document.body.addEventListener('click', (e) => {

    if (e.target.id === "ancestor") {
        print('ancestor')
    }

    if (e.target.id === "parent") {
        print('parent')
    }

    if (e.target.id === "child") {
        print('child')
    }
    
});

  1. e.preventDefault()
    e.preventDefault() 는  기본 이벤트 동작 자체를 취소한다. 예를들어 a href="url" 의 링크 기능이나, form 태그의 submit 이벤트를 취소할때 유용하다.
  • 이벤트 전파 방지 주의점
    버블링 이벤트 전파를 막는 e.stopPropagation() 메서드는 추후에 문제가 될 수 있는 상황을 만들어낼 수 있다는 점을 유의해야 한다.
    예를들어 내 서비스에서 사람들이 페이지에 어디를 클릭했는지 등의 행동 패턴을 분석하기 위해, window 내에서 발생하는 클릭 이벤트 전부를 감지는 분석 시스템을 사용할때, 이런 분석 시스템의 코드는 클릭 이벤트를 감지하기 위해 보통 document.addEventListener('click') 와 같이 사용하는데, stopPropagation로 버블링을 막아놓은 영역에선 '죽은 영역'이 되어리기 때문에 분석이 제대로 되지 않을 수 있다. 따라서 꼭 필요한 경우를 제외하곤 버블링을 막지 않는 것이 좋다. 아키텍처를 잘 고려해 진짜 막아야 하는 상황에서만 버블링을 막아야 한다. 만일 버블링을 막아야 해결될 것 같은 문제라면 커스텀 이벤트를 사용해 문제를 해결할 수도 있다는 데 이부분은 더 공부해봐야 할 거 같다.
  1. 클로저와 스코프가 무엇인가요?

    1. 스코프
      Scope를 우리말로 번역하면 ‘범위’라는 뜻을 가지고 있다. 즉, 스코프(Scope)란 ‘변수에 접근할 수 있는 범위’라고 할 수 있다.

      1. 스코프의 종류

        • 전역 스코프: 코드 어디에서든지 참조할 수 있다.
        • 지역 스코프 : 함수 코드 블록이 만든 스코프로 함수 자신과 하위 함수에서만 참조할 수 있다.
        1. 함수 스코프 - var
          자바스크립트는 함수 스코프를 기본으로 사용한다. 함수 스코프는 함수에서 선언한 변수는 해당 함수 내에서만 접근 가능하다는 걸 의미합다.
        2. 블록 스코프 - let, const
          블록 스코프는 블록 {}이 생성될 때마다 새로운 스코프가 형성되는 것을 의미한다. 원래 자바스크립트는 함수 스코프를 따르지만,
          let과 const 키워드의 등장으로 블록 스코프를 사용하는 것도 가능해졌다.
          ​ *스코프는 함수를 호출할 때가 아니라 함수를 어디에 선언하였는지에 따라 결정된다. 이를 렉시컬 스코핑(Lexical scoping)라 한다.
      2. 스코프 체인 방식
        변수나 함수 참조 시, JavaScript 엔진은 먼저 현재 스코프에서 찾기 시작 ->
        찾지 못하면, 바로 바깥쪽 스코프로 이동하여 검색한다. ->
        이 과정을 반복하여 전역 스코프까지 도달한다. ->
        전역 스코프에서도 찾지 못하면 ReferenceError를 발생시킨다.

        스크린샷 2024-09-07 오후 9 51 51

      정답 : apple
      apple

  2. 클로저 = 함수
    상위 스코프의 변수를 기억하는 함수. 클로저가 만들어진 환경을 기억한다.(렉시컬스코프)

스크린샷 2024-09-07 오후 10 19 45 정답 : 우리밋/ undefined/ woorimIT/ undefined �

김류원 added 7 commits September 7, 2024 00:53
- HTML 구조 설정
- JavaScript를 이용한 Todo 항목 추가 기능 구현
- Todo 항목 완료 상태 토글 기능 추가
- Todo 항목 삭제 기능 추가
- 한국어 날짜 표시 기능 구현
- todoForm 선택자 변경
Comment on lines +27 to +34
if (Todo) {
createTodoElement(Todo, false);
todos.push({ text: Todo, completed: false });
saveTodos();
TodoInput.value = '';
} else {
alert('To Do를 입력하세요');
}

Choose a reason for hiding this comment

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

이렇게 항목이 존재하지 않을 때, 에러 핸들링를 해주신건 사용자입장에서 정말 좋을거같아요!
저는 해당 부분을 놓쳤는데, 배워갑니다 ㅎㅎ

Comment on lines +69 to +70
toggleIcon.src = 'https://raw.githubusercontent.com/ryu-won/vanilla-todo-20th/40e5a4dcd0113eadd85034cc953aa3fa97de4527/icon/NotCheck.svg';
toggleIcon.alt = 'Toggle unComplete';

Choose a reason for hiding this comment

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

이미지 소스를 가져올 때 주소가 하드코딩 되어있는데 나중에 쉽게 관리할 수 있도록 아래와 같이 상수로 분리하는 방법도 있을 것 같습니다!

const  NOT_CHECK = 'https://raw.githubusercontent.com/.../NotCheck.svg';

이렇게 하면 해당 이미지를 중복적으로 사용할 때, 이미지 경로를 변경 시 코드 전체에서 한 번만 수정하면 되고, 에러 찾기도 더 쉽지 않을까 생각합니다 😊

Comment on lines +101 to +110
function formatDateKorean(date) {
const days = ['일요일', '월요일', '화요일', '수요일', '목요일', '금요일', '토요일'];
const months = ['1월', '2월', '3월', '4월', '5월', '6월', '7월', '8월', '9월', '10월', '11월', '12월'];

const month = months[date.getMonth()];
const day = date.getDate();
const dayOfWeek = days[date.getDay()];

return `${month} ${day}일 ${dayOfWeek}`;
}

Choose a reason for hiding this comment

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

month표현도 day처럼 표현한다면, months라는 변수를 사용하지 않아도 되니까 더 간결하고 일관된 코드가 될 수 있을 것 같습니다!

Copy link

@psst54 psst54 left a comment

Choose a reason for hiding this comment

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

첫 과제 너무 고생하셨습니다!!

Key Question도 정말 꼼꼼히 조사하셨네요👍👍👍 저도 많이 배워갑니다!!

이미지 깨짐은 파일명이 NotCheck.svg인데 ./icon/notCheck.svg로 불러오셔서 그렇습니다. 임시방편으로 ./icon/NotCheck.svg로 불러오시거나 파일 이름을 notCheck.svg로 변경하시면 될 거에여! 파일 이름을 변경할 때 아마 git이 대소문자를 구분하지 않아 문제가 발생할 수 있는데, https://s0ojin.tistory.com/47 같은 링크를 참고하시면 좋을 것 같아요!

<p id="todayDate">9월 6일 금요일</p>

<!-- input container -->
<div class="WriteForm">
Copy link

Choose a reason for hiding this comment

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

WriteForm이나 TodoInput, check_icon에서 네이밍 컨벤션이 다른 부분이 보이는데, 혹시 의도하신걸까요?!

@@ -1 +1,119 @@
//😍CEOS 20기 프론트엔드 파이팅😍
const addBtn = document.querySelector('.addBtn');
const TodoInput = document.querySelector('.TodoInput');
Copy link

Choose a reason for hiding this comment

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

여기서도 TodoInput이나 Todo처럼 camelCase를 사용하지 않는 부분이 있어서, 따로 기준을 가지고 계신지 궁금해요!

}
}

todoForm.addEventListener('submit', addTodo);
Copy link

Choose a reason for hiding this comment

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

submit 이벤트를 이용해주셔서 enter로도 todo를 추가할 수 있어서 편리했어요👍👍👍

// 완료 토글 아이콘
const toggleIcon = document.createElement('img');
toggleIcon.src = isCompleted ? './icon/checkComplete.svg' : 'https://raw.githubusercontent.com/ryu-won/vanilla-todo-20th/40e5a4dcd0113eadd85034cc953aa3fa97de4527/icon/NotCheck.svg';
toggleIcon.alt = isCompleted ? 'Toggle Complete' : 'Toggle unComplete';
Copy link

Choose a reason for hiding this comment

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

alt까지 챙겨주시는 꼼꼼함 너무 좋아요🫶🫶 저는 까먹기도 하거든요....😅

Comment on lines +102 to +103
const days = ['일요일', '월요일', '화요일', '수요일', '목요일', '금요일', '토요일'];
const months = ['1월', '2월', '3월', '4월', '5월', '6월', '7월', '8월', '9월', '10월', '11월', '12월'];
Copy link

Choose a reason for hiding this comment

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

이렇게 상수를 선언해 주신 점 좋습니다👍
그런데 JS에서 이런 상수를 선언할 때는 UPPER_SNAKE_CASE로 선언하는 방법이 더 널리 쓰인다고 알고 있어요! 한번 찾아보시면 좋을 것 같아요!!

Comment on lines +94 to +96
listItem.appendChild(toggleIcon);
listItem.appendChild(todoText);
listItem.appendChild(deleteBtn);
Copy link

Choose a reason for hiding this comment

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

혹시 listItem.append(toggleIcon, todoText, deleteBtn);처럼 element를 한번에 추가할 수 있다는 걸 알고 계신가용?! 저도 이번 과제에서 처음 알았는데 유용한 것 같아서 공유드립니다!
https://developer.mozilla.org/en-US/docs/Web/API/Element/append 에서 더 자세히 확인하실 수 있어요🫶🫶


// 투두 텍스트
const todoText = document.createElement('span');
todoText.textContent = Todo;
Copy link

Choose a reason for hiding this comment

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

여기에서 textContent를 써주신 부분이 좋았어요👍👍 저는 innerText를 썼는데, 검색해보니 innerText는 스타일 재계산이 일어나지만, textContent는 스타일 재계산이 일어나지 않으므로 간단한 텍스트 업데이트에는 textContent가 더 성능이 좋다고 합니다! 저두 textContent로 바꿔야겠어요🫶🫶

/*투두 시작*/
.todoContainer {
background-color: #f1f3ff;
width: 50%;
Copy link

Choose a reason for hiding this comment

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

저는 모바일로 확인했을 때 너비가 너무 줄어든다는 느낌을 받았어요! min-width 등을 지정해보시는건 어떨까요?

transition: background-color .3s, color .3s;
}

.writeTodoForm > .addBtn:hover, .delete-btn:hover{
Copy link

Choose a reason for hiding this comment

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

hover까지 처리해주신거 너무 좋아요👍👍

flex-grow: 1;
}

/* 할일 추가 애니메이션*/
Copy link

Choose a reason for hiding this comment

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

애니메이션이 있어서 굉장히 부드럽게 추가/삭제가 작동한다는 느낌을 받았어요!! 너무 고생하셨습니다💖💖

Copy link
Collaborator

@jinnyleeis jinnyleeis left a comment

Choose a reason for hiding this comment

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

류원님!! 예쁜 투두 리스트 과제 구현하시느라 수고 많으셨어요!!!

DOMContentLoaded 이벤트를 사용하여 DOM이 완전히 로드된 후에 안정적으로 데이터를 초기화할 수 있게 작성하신 점도 정말 인상깊은 것 같아요!!

현재 텍스트를 기준으로 항목을 구분하고 있어, 내용이 중복되는 요소들이 존재할 경우에는 버그가 발생하는 것 같아요! 이 부분만 수정하시면 더욱 완벽한 코드가 될 수 있을 것 같아요!

그리고 현재는 커밋명과 - 커밋 내용이 잘 대응이 되지 있는 부분이 있는 것 같은데 (ex. css 수정 -> 내에 js 함수 수정이 있는 경우 등 ) 커밋도 조금 기능별로 명확히 나누어서 해주시면 나중에 수정이 필요하거나 코드 리뷰할 때 더 좋을 것 같아요!

다양한 배열 메서드도 활용해주시고 이번 과제 정말 수고 많으셨어요! ><

toggleIcon.src = './icon/checkComplete.svg';
toggleIcon.alt = 'Toggle Complete';
} else {
toggleIcon.src = './icon/notCheck.svg';
toggleIcon.alt = 'Toggle unComplete';
}
// localStorage 업데이트
const index = todos.findIndex(item => item.text === Todo);
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 류원님의 투두 리스트에서 동일한 내용을 가진 항목을 추가하고, 완료하면 첫번째 항목의 completed 값만 true로 변경되고 두번째 항목의 completed 값은 변경되지 않는 것을 확인할 수 있어요.
image

지금 코드를 보면, 항목을 고유하게 식별할 수 있는 값이 없고, 텍스트만을 기준으로 항목을 찾고 업데이트하는 것으로 보여요. 때문에, 텍스트가 동일한 두 항목이 있을 경우 제대로 구분되지 않고 한 항목만 업데이트되는 문제가 발생하는 것 같아요.
배열 메서드 findIndex()에 대한 공식 문서 설명을 보시면, '주어진 판별 함수를 만족하는 배열의 첫 번째 요소에 대한 인덱스를 반환'이라고 되어 있어요. 때문에, 첫 번째 요소 이후의 항목은 무시가 되는 것 같아요.
https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex

이러한 문제를 해결하려면, 각각의 항목을 텍스트 내용에 상관없이 고유하게 식별할 수 있어야 하는데, text를 구분 기준으로 삼기 보다, 고유한 id를 추가해서 Id를 기준으로 식별하는 것이 좋아보여요!

    todos.push({ text: Todo, completed: false });

->
todos.push({ id: Date.now(), text: Todo, completed: false });

...
const index = todos.findIndex(item => item.id === todoId);

이런식으로요!

https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex

@@ -30,6 +29,7 @@ html{
border-bottom: 1px solid #7e8dea;

}
/**/

/*투두 시작*/
.todoContainer {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pasted Graphic 9

류원님의 투투리스트에서 항목이 height을 초과하는 경우에, 컨테이너 밖으로 삐져나오게 되는 것 같아요.

이때 css의 overflow 속성을 사용하면, 컨텐츠가 컨테이너를 초과할 때 어떻게 처리할지 설정해줄 수 있을 것 같아요!!
https://developer.mozilla.org/ko/docs/Web/CSS/overflow
류원님 코드에 overflow-y: auto; 속성 하면, 항목이 넘치면 아래와 같이 세로로 스크롤 되도록 처리할 수 있네요!!
image

setTimeout(() => {
todoList.removeChild(listItem);
// localStorage에서 삭제
todos = todos.filter(item => item.text !== Todo);
Copy link
Collaborator

Choose a reason for hiding this comment

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

할일 항목을 삭제할때도, 텍스트 기준으로 비교되어서
예를 들어 ㅇㅇㅇㅇㅇ라는 항목이 5개가 있는데 하나를 삭제했을 경우, 새로고침하면 5개의 항목이 다 삭제되는 것 같아요!!
이 경우에도 text 기준이 아닌, 항목별로 고유한 Id를 통해 필터링해서, 로직을 처리해주는 것이 적합할 것 같아요!
아까 배열 메서드 중 findIndex는 값과 일치하는 첫 번째 요소를 반환했는데,
filter 메서드는 주어진 배열에서 제공된 함수에 의해 구현된 테스트를 통과한 요소들을 담은 새로운 배열을 반환한다는 차이점도 알아두면 좋을 것 같아요!
https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/Array/filter

script.js Outdated
todoList.removeChild(listItem);
});

// HTML 구조에 맞게 요소 추가
Copy link
Collaborator

Choose a reason for hiding this comment

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

다혜님이 말씀해주신것처럼, appendchild 메서드로 요소를 하나씩 추가하는 것보다 한번에 추가하는 방법을 사용하는게 최적화 측면에서도 좋아보여요!
appendChild가 호출될 때마다 브라우저가 DOM을 업데이트하고, 리플로우와 리페인팅 발생 가능성이 있어요. 때문에, 성능이 저하가 될 수 있어 가급적 dom 조작을 최소화하는 방식을 취하는 것이 좋아보여요.

가빈께 남긴 코드리뷰중에, 리플로우와 리페인팅에 대한 설명을 참고해보시면 좋을 것 같아요!
#4 (comment)

toggleIcon.alt = 'Toggle unComplete';
toggleIcon.classList.add('toggle-icon');

// 투두 텍스트
const todoText = document.createElement('span');
todoText.textContent = Todo;
Copy link
Collaborator

Choose a reason for hiding this comment

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

변수명을 쓸 때, Todo보다 lowercamelcase 컨벤션에 맞추어 todo로 작성하는것이 좋을 것 같아요!

@@ -19,37 +37,57 @@ function addTodo(e) {
todoForm.addEventListener('submit', addTodo);

// 투두 추가 함수
function createTodoElement(Todo) {
function createTodoElement(Todo, isCompleted) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

createTodoElement() 함수 내에
할 일 추가, 완료 토글, 삭제 로직이 모여 있어서 , 나중에 수정할 부분이 생기면, 복잡할 수 있겠다는 생각이 들어요!
때문에 각각의 로직별로 함수로 적절히 분리하면 가독성과 유지보수에 더 좋을 것 같아요!

투두 텍스트 생성 / 토글 생성 / 삭제 버튼 생성 로직을 별도의 함수로 빼낸 후,
createTodoElement() 함수 내에서 이들을 호출하는 식으로 수정해보시는 것을 추천드려요!!

Copy link

@Shunamo Shunamo left a comment

Choose a reason for hiding this comment

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

안녕하세요 류원님! 첫주차 과제 너무 고생많으셨습니다 😍
애니메이션 디테일이 인상 깊었어요 ㅎㅎ 디자인도 넘 예버서 감탄하면서 봤다는 ..😳


.todoList{
width:100%;
}
Copy link

Choose a reason for hiding this comment

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

스크린샷 2024-09-09 오후 3 12 08
overflow 처리가 안되어 있어서 todolist 목록이 많아졌을 때,
컨테이너 외부로 넘치고 있어요!
max-height, overflow 속성 적용하면 더 좋을 것 같아요 🤩

Suggested change
}
max-height: 70vh;
overflow-y: auto;
}

overflow docs
😃😃

}

.todoList li {
width: 100%;
Copy link

Choose a reason for hiding this comment

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

.todoList 자체에 이미 width: 100%가 설정되어 있어서,
li에 적용된 width: 100%는 지워주셔도 좋을 것 같아요 😇👋

padding: 0 15px 0 15px;
color: #788bff;
font-weight: 700;
font-size: 0.8rem;
Copy link

Choose a reason for hiding this comment

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

다른 값들은 px 단위로, font-size에는 rem 단위로 작성하신 것 같아요!
사실 이번 과제에서는 큰 차이가 없겠지만.. 폰트 외의 다른 값도 rem으로 단위를 통일시켜 주시는게 좋아요! 프로젝트 볼륨이 커질수록 반응형 레이아웃 관리가 더 수월할 거예요! 😸
rem velog

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.

5 participants