Skip to content

Comments

[1주차] 이채연 과제 제출합니다.#2

Open
codus02 wants to merge 2 commits intoCEOS-Developers:mainfrom
codus02:codus02
Open

[1주차] 이채연 과제 제출합니다.#2
codus02 wants to merge 2 commits intoCEOS-Developers:mainfrom
codus02:codus02

Conversation

@codus02
Copy link

@codus02 codus02 commented Sep 12, 2025

https://vanilla-todo-22nd-ru5r.vercel.app/




깃허브 포크 클론해서 사용하는거 처음해봐서 깃허브 사용이랑 배포하는거 한참 애먹음..

언어들도 오랜만에 써보고 js는 진짜 기억 잘 안나서 한참걸렷음 다시 공부해야할듯

- DOM은 무엇인가요? Document Object Model 로, 문서의 각 요소를 각 개체로 만들고 이를 트리구조로 구성한것이다. 자바스크립트로 동적 변경할수있음

- 이벤트 흐름 제어(버블링 & 캡처링)이 무엇인가요? 이벤트가 발생했을때 DOM 트리를 따라 이벤트가 전파되는데 이때 상위->하위 요소로 전파되는게 캡처링이고 하위-> 상위 요소로 전파되는걸 버블링이라고함

- 클로저와 스코프가 무엇인가요? 스코프는 변수가 유효한 범위로 전역, 지역으로 나뉜다. 클로저는 변수가 선언된 스코프를 기억하고 스코프 밖에서 그 변수를 사용할수있게 하는것

Comment on lines +134 to +139
const newTodo = {
id: nextId++,
text: text,
completed: false,
date: getCurrentDateString(),
};

Choose a reason for hiding this comment

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

newTodo로 한번에 push할 수 있게 한 부분이 인상깊었습니다

Comment on lines +199 to +208
li.innerHTML = `
<label class="todoLabel">
<input type="checkbox" class="todoCheckbox" ${
todo.completed ? 'checked' : ''
}
onchange="toggleTodo(${todo.id})">
<span class="todoText">${todo.text}</span>
</label>
<button class="deleteBtn" onclick="deleteTodo(${todo.id})">🗑️</button>
`;

Choose a reason for hiding this comment

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

제가 찾아본 바로는 이 부분이 XSS 공격 위험이 있다고 합니다
DOM 방식을 사용해보면 좋을 것 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

innerHTML 사용은 위험할수잇으니까 최소화해야함

id="todoInput"
placeholder="할일을 입력하세요"
maxlength="100"
required

Choose a reason for hiding this comment

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

required 가 경고 parameter라는 것을 알게 됐습니다
js에서 공백 exception 처리보다 간결한 것 같습니다

Comment on lines +245 to +260
try {
const saved = localStorage.getItem('todosByDate');
const savedId = localStorage.getItem('nextId');

if (saved) {
todosByDate = JSON.parse(saved);
}

if (savedId) {
nextId = parseInt(savedId);
}
} catch (error) {
console.error('불러오기 오류:', error);
todosByDate = {};
nextId = 1;
}

Choose a reason for hiding this comment

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

불러오는 과정에서의 exception 처리는 좋은 습관인것 같습니다

Comment on lines +68 to +73
function moveToDate(direction) {
currentDate.setDate(currentDate.getDate() + direction);
updateDateDisplay();
renderTodoList();
updateProgress();
}

Choose a reason for hiding this comment

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

eventListener를 통해 moveToDate 코드를 이전,이후에 삽입하는 방식 대신 하나의 함수화를 통해 더 깔끔한 코드인 것 같습니다

Copy link
Member

@KWONDU KWONDU left a comment

Choose a reason for hiding this comment

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

1주차 과제 고생하셨습니다 -!

Copy link
Member

Choose a reason for hiding this comment

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

prettier 사용하신 거 좋은 거 같아요. 제가 사용하는 세팅도 공유드릴테니 해당 부분도 보시면 좋을 거 같아요

{
  "arrowParens": "always",
  "bracketSpacing": true,
  "endOfLine": "lf",
  "printWidth": 120,
  "semi": true,
  "singleQuote": true,
  "tabWidth": 2,
  "trailingComma": "all",
  "useTabs": false,
  "plugins": ["prettier-plugin-tailwindcss"] // 이건 무시하셔도 됨
}

<input type="date" class="dateInput" id="dateInput" />
<p class="dateInfo" id="dateInfo">2024년 12월 19일 목요일</p>
</div>
<button class="dateNavBtn" id="nextDay">▶</button>
Copy link
Member

Choose a reason for hiding this comment

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

css property 중 cursor: pointer;라는 속성이 있는데요, 해당 속성 (마우스 hover 시 포인터가 손 모양으로 바뀜) 잘 사용하시면 좀 더 사용자 친화적인 UI가 될 것 같아요

Copy link
Member

Choose a reason for hiding this comment

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

지금 일부 버튼에만 적용되어 있네요 (e.g., deleteBtn)

const dateStr = getCurrentDateString();
const today = new Date().toISOString().split('T')[0];
const isToday = dateStr === today;
const prefix = isToday ? '오늘' : dateStr;
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 오늘 케이스 구분해 놓은 거 좋은 거 같아요

localStorage.removeItem('todoList');

initializeApp();
this.getSelection;
Copy link
Member

Choose a reason for hiding this comment

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

이 코드도 현재로써는 동작하지 않는 코드인 거 같아요

// 페이지 로드 완료 시 실행
window.addEventListener('load', function () {
// 기존 데이터 삭제 (새 구조로 변경)
localStorage.removeItem('todoList');
Copy link
Member

Choose a reason for hiding this comment

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

이 부분은 지우셔도 될 것 같아요

// 전역 변수
let todosByDate = {};
let currentDate = new Date();
let nextId = 1;
Copy link
Member

Choose a reason for hiding this comment

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

지금 nextId라는 변수를 선언하셔서 unique key로 사용하신 것으로 보이는데요, 동일한 날짜/텍스트여도 데이터를 구분한다는 점에서 좋은 선택인 것 같습니다. 다만 이렇게 하시는 것보다는 현재 Date()을 uuid 인코딩해서 사용하신다던지, 각 날짜별로 unique index를 구분한다던지 하시면 더 좋을 거 같아요. 지금 하시는 건 단순 index 처리여서 해당 key가 진정 의미가 있는가? 했을 때 더 좋은 방향으로 나아갈 여지가 있는 것 같습니다

input.value = '';
} else {
alert('할일을 입력해주세요!');
}
Copy link
Member

Choose a reason for hiding this comment

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

지금 input tag에 이미 required 속성이 존재하여 alert가 추가적으로 동작하지 않는 것 같아요. 해당 부분 확인해 주시면 좋을 거 같아요

saveTodosToStorage();
renderTodoList();
updateProgress();
}
Copy link
Member

Choose a reason for hiding this comment

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

현재 날짜의 todo가 없을 때 empty list로 유지하는 대신 아예 삭제하는 것도 데이터 관리/최적화 측면에서 고려해볼만한 선택인 거 같아요

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.

3 participants