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주차] 이은비 미션 제출합니다 #4

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

Conversation

silverain02
Copy link

배포링크

Vanilla Todo

Key Questions

  • DOM은 무엇인가요?

    DOM 은 Document Object Model (문서 객체 모델)로 HTML이나 XML문서를 구조화 시킨 것이다. Javascipts와 같은 스크립팅 언어로 이 DOM을 수정할 수 있다.

  • HTML (tag) Element를 JavaScript로 생성하는 방법은 어떤 것이 있고, 어떤 방법이 가장 적합할까요?

    1. createElement()

      var divElement = document.createElement('div');

      createElement 메서드를 통해 html 요소를 생성하고 appendChild, prepend, insertBefore 같은 함수를 통해 HTML 의 특정 위치에 요소를 삽입할 수 있습니다

    2. insertAdjacentHTML

      element.insertAdjacentHTML(position, text);

      insertAdjacentHTML메서드를 통해 특정 위치(position)에 html요소(text)를 추가할 수 있다.

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

    HTML태그 중 특정한 의미를 가진 태그를 ‘Semantic tag’라고 하며 웹사이트에서는 주요하게 header, nav, footer, main, aside, article, section가 있습니다.
    그냥 div를 사용하는 것보다 이런 의미있는 태그를 사용하는 것이 코드 공유시 타 개발자의 이해를 도울 수 있고 효율적으로 리팩토링할 수 있다.

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

    flexbox는 요소의 사이즈가 불명확하거나 동적 변화 시에도 유연하게 레이아웃을 실현하게 해주는 방식으로, display:flex 를 통해 적용가능하다.

    flex-direction: row, column은 각각 수평,수직으로 자식 요소를 배치시킨다. justify-content, align-items로 자식요소를 정렬시킬 수 있다.

  • JavaScript가 다른 언어들에 비해 주목할 만한 점에는 어떤 것들이 있나요?

    JavaScript는 웹 브라우저 내에서 실행되어 웹의 동작 구현 부터 node.js와 같은 프레임워크를 사용해 서버 기능까지 구현 할 수 있습니다.

    또한 타입을 명시할 필요가 없는 인터프리터 언어이기 때문에 컴파일 과정없이 웹 내에서 소스코드를 바로 실행할 수 있다는 장점이 있습니다.

  • 코드에서 주석을 다는 바람직한 방법은 무엇일까요?

    기능 혹은 함수 단위로 주석을 달아 의도한 기능을 설명하려고 사용하고 있습니다. 다른 팀원이 보고도 기능을 이해하고 수정할 수 있도록 해야 되기 때문에 가능한 팀내에서 주석에 대한 컨벤션을 통일하는 게 좋다고 생각합니다.

후기

React로 컴포넌트 단위로만 작업하다 오랜만에 쌩 html, css, js를 다루니 간단한 기능인데도 다소 시간이 오래걸렸습니다 ㅎㅎ
피그마 작업없이 바로 CSS로 만들다보니 디자인적으로 아쉬움이 있었습니다 디자이너의 중요성도 많이 느꼈네요ㅎㅎ…
배열에 요소가 추가될 때마다 자동적으로 리렌더링이 되지 않다보니 render함수로 매번 요소가 변화할 때마다 수동으로 리렌더링 처리를 해주었는데 리액트 내부 기능을 구현하는 느낌이라 작동방식을 이해하는데 도움이 된 것 같습니다

Copy link
Member

@leejin-rho leejin-rho left a comment

Choose a reason for hiding this comment

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

과제 수고하셨습니다~! 주석때문에 꼼꼼히 써주셔서 읽기 좋았어요!

const addTodo = () => {
//입력값 유효성 검사
if (input.value.trim() === '') {
return;
Copy link
Member

Choose a reason for hiding this comment

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

alert 창으로 명시해주면 더 좋을 것 같아요~

@@ -110,7 +108,7 @@ const renderDone = () => {
li.appendChild(span);
li.appendChild(deleteBtn);

Copy link
Member

Choose a reason for hiding this comment

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

주석을 자세하게 써주셔서 알아보기 더 좋은 것 같아요!

const addDone = (elem) => {
doneList.push(elem); //done 배열에 추가

localStorage.setItem('dones', JSON.stringify(doneList)); //로컬스토리지 배열 업데이트
Copy link
Member

Choose a reason for hiding this comment

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

저는 한번에 모아서 localStorage에 저장했는데 은비님처럼 바로바로 하는게 더 좋은 것 같아요~

style.css Outdated
}

li {
width: 18vw;
Copy link
Member

Choose a reason for hiding this comment

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

다 반응형으로 하신거 좋은것 같아요~! 저도 다음에는 반응형으로 해보겠습니다


//로컬스토리지 내 done 목록 띄우기
const renderDone = () => {
doneList = JSON.parse(localStorage.getItem('dones')) || [];
Copy link
Member

Choose a reason for hiding this comment

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

저는 그냥 페이지를 load할 때만 local storage에서 가져오는 느낌으로 구현했는데 은비님처럼 아예 render하는 방법으로 사용하신게 훨씬 좋은 것 같아요~ 참고하겠습니다!

width: 2rem;
}

.doneContent {
Copy link
Member

Choose a reason for hiding this comment

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

todo랑 done 둘 다 사이즈를 줘서 내용을 추가할 때마다 움직이지 않고 고정돼도 좋을 것 같아요!

Copy link
Member

@moong23 moong23 left a comment

Choose a reason for hiding this comment

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

안녕하세요
프론트 파트장 김문기입니다~!

투두 리스트에 추가 구현도 많이 해주셨고, render함수를 통해 강제 렌더링을 통해 작업하시면서
react의 렌더링 작동방식도 이해하는데 도움이 되셨다고 하니 의미 있는 과제가 된 것 같아서 뿌듯합니다~!

이번주 과제 너무 수고 많으셨구 잠시 후 스터디에서 뵙겠습니다~!!!

Comment on lines +38 to +40
if (input.value.trim() === '') {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

trim을 통해 input 예외처리해주신점 좋습니다 👍

Comment on lines +74 to +76
li.appendChild(checkbox);
li.appendChild(span);
li.appendChild(deleteBtn);
Copy link
Member

Choose a reason for hiding this comment

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

여러개의 child를 append해야하는 경우 appendChild보다는 append를 사용해도 좋을 것 같아요~!
그리고, append를 위해 잠시 사용하는 li지만 가독성을 위해 tmpList와 같이 "임시로 사용할 리스트"임을 명시해주어도 좋을 것 같습니다!

Suggested change
li.appendChild(checkbox);
li.appendChild(span);
li.appendChild(deleteBtn);
li.append(checkbox, span, deleteBtn);


//로컬스토리지 내 done 목록 띄우기
const renderDone = () => {
doneList = JSON.parse(localStorage.getItem('dones')) || [];
Copy link
Member

Choose a reason for hiding this comment

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

ts쓸 때는 localStorage.getItem('dones')가 null일 경우를 대비하여

Suggested change
doneList = JSON.parse(localStorage.getItem('dones')) || [];
doneList = JSON.parse(localStorage.getItem('dones'), '[]')

로 작성해도 좋을 것 같네요~!

const li = document.createElement('li');

const checkbox = document.createElement('input');
checkbox.type = 'checkbox';
Copy link
Member

Choose a reason for hiding this comment

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

체크리스트가 종료된 경우에는

Suggested change
checkbox.type = 'checkbox';
checkbox.type = 'checkbox';
checkbox.checked = true;

를 통해 체크 됐음을 표시해도 좋을 것 같습니다.

Comment on lines +66 to +76
const span = document.createElement('span');
span.innerText = todo;

const deleteBtn = document.createElement('button');
deleteBtn.innerText = 'X';
deleteBtn.addEventListener('click', () => removeTodo(index));

// li 요소에 체크박스, 제목, 삭제 버튼을 추가
li.appendChild(checkbox);
li.appendChild(span);
li.appendChild(deleteBtn);
Copy link
Member

Choose a reason for hiding this comment

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

단순히 todo text를 넣기 위핸 span을 선언하기보다

Suggested change
const span = document.createElement('span');
span.innerText = todo;
const deleteBtn = document.createElement('button');
deleteBtn.innerText = 'X';
deleteBtn.addEventListener('click', () => removeTodo(index));
// li 요소에 체크박스, 제목, 삭제 버튼을 추가
li.appendChild(checkbox);
li.appendChild(span);
li.appendChild(deleteBtn);
const deleteBtn = document.createElement('button');
deleteBtn.innerText = 'X';
deleteBtn.addEventListener('click', () => removeTodo(index));
// li 요소에 체크박스, 제목, 삭제 버튼을 추가
li.appendChild(checkbox);
li.appendChild('todo');
li.appendChild(deleteBtn);

와 같이도 적어줄 수 있을 것 같습니다

Comment on lines +117 to +137
const removeTodo = (index) => {
//filter메소드로 삭제한 배열로 업데이트
todoList = todoList.filter((data) => {
return data !== todoList[index];
});
//localStorage다시 저장
localStorage.setItem('todos', JSON.stringify(todoList));

renderTodo(); //리렌더링
};

//doneList삭제함수
const removeDone = (index) => {
//filter메소드로 삭제한 배열로 업데이트
doneList = doneList.filter((data) => {
return data !== doneList[index];
});
//localStorage다시 저장
localStorage.setItem('dones', JSON.stringify(doneList));
renderDone(); //doneList 리렌더링
};
Copy link
Member

Choose a reason for hiding this comment

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

이 부분 로직이 겹치는 부분이 많은 것 같은데, 'todos' 혹은 'dones'을 매개변수로 받아서 아래와 같이 함수를 합칠수도있을것같네요

Suggested change
const removeTodo = (index) => {
//filter메소드로 삭제한 배열로 업데이트
todoList = todoList.filter((data) => {
return data !== todoList[index];
});
//localStorage다시 저장
localStorage.setItem('todos', JSON.stringify(todoList));
renderTodo(); //리렌더링
};
//doneList삭제함수
const removeDone = (index) => {
//filter메소드로 삭제한 배열로 업데이트
doneList = doneList.filter((data) => {
return data !== doneList[index];
});
//localStorage다시 저장
localStorage.setItem('dones', JSON.stringify(doneList));
renderDone(); //doneList 리렌더링
};
const removeFunc = (query, index) => {
if(query === 'todos'){
[targetList, targetFunc] = todoList, renderTodo;
} else if (query === 'dones'){
[targetList, targetFunc] = doneList, renderDone;
}
targetList = targetList.filter((data) => {
return data !== targetList[index];
});
localStorage.setItem(query, JSON.stringify(targetList));
targetFunc();

Comment on lines +12 to +19
.inputBox {
width: 30vw;
height: 3vw;

background-color: #f7f8f8;
border: none;
border-radius: 10px;
}
Copy link
Member

Choose a reason for hiding this comment

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

inputbox에 text가 바로 왼쪽에 붙어있는데, 미관상 padding값을 좌우로 조금 주는 것이 더 나을때도 있더라구요 참고하시면 좋을 것같아요~!

const clock = () => {
let current = new Date(); //현재 날짜 와 시간
let year = current.getFullYear(); //연도
let month = current.getMonth(); //월
Copy link
Member

Choose a reason for hiding this comment

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

아래에서 month +1을 너무 잘 해주셨지만, 애초에 선언할 때

Suggested change
let month = current.getMonth(); //월
let month = current.getMonth() + 1; //월

과 같이 선언해주시면 아래에서 재사용을 할 일이 있을 때 바로 사용하기 좋을 것 같습니다.

Comment on lines +27 to +36
.todoContent {
list-style: none;

display: flex;
flex-direction: column;
align-items: center;
justify-content: space-between;

gap: 1vw;
}
Copy link
Member

Choose a reason for hiding this comment

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

image 이미지와 같이 todo가 많을 때 페이지 전체가 늘어지는데, 위 부분에 height max값을 정해준 후 `overflow-y : scroll` 과 같은 속성을 달아주어서 조금 더 나은 UI를 만들 수 있지 않을 까 싶습니다~!

@mod-siw
Copy link

mod-siw commented Sep 17, 2023

안녕하세요 1주차 코드 리뷰 파트너 변지혜입니다😄
localstorage와 삭제 아이콘 hover에 주석까지, 신경써주신 코드 잘 봤습니다.
제가 생각해보지 않았던 새로운 방식의 함수 적용을 덕분에 배워가네요.
1주차 과제 수고 많으셨습니다! 스터디 시간에 만나요❗

Comment on lines +51 to +61
.doneContent input {
width: 2rem;
}

li {
width: 20vw;
display: flex;
justify-content: space-between;

font-size: 2vw;
}
Copy link

Choose a reason for hiding this comment

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

단순히 width 뿐만 아니라 font-size, gap까지 px을 쓰지 않으시고 반응형이 가능한 단위들로 쓰셨네요! 저는 아직 rem과 vw에 익숙치 않아서 이렇게도 쓸 수 있구나 배워갑니다. 코드 리뷰 하며 단위들이 헷갈려서 찾아본 링크 공유할게요!
언제 rem을 쓰면 좋은지, 언제 vw를 쓰면 좋은지에 대한 내용도 공부해보고 싶어집니다..!
사이즈 단위별 차이

checkbox.addEventListener('change', () => {
addDone(todo);
removeTodo(index);
});
Copy link

Choose a reason for hiding this comment

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

렌더링 함수 만들어서 쓰는 방식 생각을 못했었네요! item 상태 바뀔 때마다 로드되어야 하는 함수 하나하나 호출하지 않아도 되는 좋은 방법인 것 같아요.

<form class="inputWrapper">
<input class="inputBox" placeholder=" 할일을 입력해보세요" />
</form>
<div class="contentWrapper">
Copy link

Choose a reason for hiding this comment

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

1주차 Key Questions 에 Semantic tag에 대한 내용이 있었던 만큼, Semantic tag를 활용해서 코드를 짜면 해당 tag의 장점들을 이용할 수 있을 것 같아요! 저도 습관적으로 div가 제일 먼저 나오긴 하지만...ㅎㅎ 이번에 배운 만큼 한 번 사용해보면 좋을 것 같습니다!

<span id="todoNum">0</span>
</h2>
<ul class="todoContent"></ul>
</div>
Copy link

Choose a reason for hiding this comment

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

span 태그로 나타내는 숫자는 id로 처리한 이유가 궁금해요!
저는 헷갈리는 경우가 있어 클래스로 몰빵하는 편인데, 너무 클래스만 쓰는 듯해 기준을 갖고 나눠볼까 싶어서요😅 어떤 식으로 기준 나누셨는지 궁금합니다!

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