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 15 commits into
base: master
Choose a base branch
from

Conversation

YooSeonHo
Copy link
Member

@YooSeonHo YooSeonHo commented Sep 16, 2022

vanilla js를 정말 오랜만에 해봐서 많이 방황했습니다.
중복되는 부분들을 많이 제거하여 코드를 간결히 짜고 싶었습니다.
하지만 항상 그렇듯 하나를 바꾸면 다른 게 안되어버리는 일이 있어서 많이 어려웠고 아쉬움이 많이 남습니다.
팀 빌딩 후 팀원들을 위해 주석을 다는 습관을 들이기 위해 노력하겠습니다~!
간만에 하니까 재미있었어요!

배포 링크: https://vanilla-todolist-16th.netlify.app

[Key Questions]

  1. DOM은 무엇인가요?

DOM은 문서 객체 모델입니다. 말 그대로 HTML문서의 요소들을 객체로 모델링한 것입니다.
HTML문서에 접근할 수 있는 인터페이스이기도 합니다.
자바스크립트는 이 객체를 통해 HTML 문서에 접근하여 내용을 열람, 수정할 수 있습니다.
Document 객체를 이용하면 접근할 수 있습니다.

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

ocument.createElement(HTML 요소)를 통해 html element 생성이 가능합니다.
생성 후 html문서에 추가하려면document.body.appendChild(~)를 이용하면 만든 요소를 바디에 넣어줄 수 있습니다.
또 방법으로 insertAdjacentHTML 가 있는 것을 알 수 있었습니다.
element.insertAdjacentHTML(position, text);와 같이 사용합니다.
Target Html으로부터 어디에 요소를 추가할 건지 정할 수 있어서 appendChild를 사용해야하는 createElement보다 한 단계 간단히 사용할 수 있습니다.
inserAdjacentHTML은 처음으로 알게 되었는데 어디 추가할 지 직관적으로 알 수 있고 명령어 한번만으로 html 요소 추가까지 할 수 있다는 점이 굉장히 매력적이라고 느껴졌습니다. 앞으로는 이 방법을 사용하는 것도 좋을 것 같다는 생각을 했습니다.

  1. Semantic tag에는 어떤 것이 있으며, 이를 사용하는 이유는 무엇일까요?
, , 등등이 있습니다. 시맨틱 태그를 사용하는 이유는 구역 속 코드가 어떤 내용인지 명확히 하여 컴퓨터 뿐만 아니라 사용자도 이름만 보고도 태그의 기능을 이해할 수 있습니다. 또 구조를 정의하기 위해 사용하고 코드의 가독성을 높일 수도 있습니다.
  1. Flexbox Layout은 무엇이며, 어떻게 사용하나요?

크기가 불분명해도 특별한 계산없이 정렬을 쉽게할 수 있는 레이아웃 방식입니다.
Flex-direction으로 가로(row) 혹은 세로(column) 중 하나의 방향을 기준으로 정하여 properties를 정렬할 수 있습니다.

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

HTML을 동적으로 작성할 수 있습니다. 컴파일 과정을 거치지 않기 때문에 상대적으로 자료형의 압박을 덜 받을 수 있습니다.

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

주석을 목적에 따라 다르게 작성해야 한다고 생각합니다. 개인적인 코드에는 본인이 보기 쉽게 작성하고, 정보 전달을 위해선 모든 이의 수준을 고려하여 최대한 상세하게, 그리고 협업용 코드에서는 간결하지만 명확하게 작성해야 한다고 생각합니다.

Copy link
Member

@jhj2713 jhj2713 left a comment

Choose a reason for hiding this comment

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

안녕하세요! 프론트 파트장 주효정입니다👏

우선 한 주 동안 수고많으셨어요! 깃 로그를 영어로 작성해주신 점이 인상깊네요. 그리고 주석도 적절하게 달아주셔서 이해하기 편했던 것 같아요! 협업 프로젝트를 하실 때의 활약이 기대가 됩니다~👍 몇 가지 제안 사항이나 의견을 남겨뒀습니다!

그럼 스터디 시간에 뵐게요~!!

<link rel="stylesheet" href="style.css" />
</head>

<body>
<div class="container"></div>
<div class="container">
Copy link
Member

Choose a reason for hiding this comment

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

Semantic Tag에 대해 학습한 만큼 여기에서도 div말고 적절한 Semantic tag를 사용해봐도 좋을 것 같아요~

index.html Outdated
Comment on lines 22 to 24
<div class="doingContainer">
<span id="doingText">Doing (0)</span>
<ul id="doing"></ul>
Copy link
Member

Choose a reason for hiding this comment

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

idclass를 혼용하시는데, 사용하는 데에 어떤 기준이 있는지 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

className으로 받으면 요소가 아닌 HTMLcollection으로 받아온 다는 점을 알았습니다.
이걸 알기 전에 모두 class로 작성해놨어서 그때그때 필요한 걸 class에서 id로 바꿔서 사용했던 기억이 납니다 ㅠ.ㅠ
앞으로 조금 더 신경 써 보겠습니다 감사합니다~!

index.html Outdated
Comment on lines 27 to 29
<div class="doneContainer">
<span id="doneText">Done (0)</span>
<ul id="done"></ul>
Copy link
Member

Choose a reason for hiding this comment

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

idclass 모두 camelCase를 사용해서 네이밍하시네요! 보통 class 이름은 호환성이나 사용성 문제로 camelCase보다는 kebab-case 네이밍을 더 많이 사용하는 것 같아요. 추가로 CSS 네이밍에 대한 방법론도 여러 가지 공부해보면 좋을 것 같아요! 참고자료 남기고 갑니다.

Copy link
Member Author

@YooSeonHo YooSeonHo Sep 17, 2022

Choose a reason for hiding this comment

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

우와 감사합니다! 정독해보겠습니다~!

Copy link
Member Author

Choose a reason for hiding this comment

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

우와 정말 어려운데 정말 유용할 거 같아요

script.js Outdated
Comment on lines 12 to 13
doingText.innerText = `Doing (${toDoList.childElementCount})`;
doneText.innerText = `Done (${doneList.childElementCount})`;
Copy link
Member

Choose a reason for hiding this comment

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

doingTextdoneText에 데이터 개수를 넣어주고 계신데, “Doing”과 “Done”은 다소 불필요한 텍스트로 보여요! 숫자 부분만 따로 태그를 지정해준 후 해당 element의 innerText를 변경해주는 방법이 더 명확한 작동이 될 수 있지 않을까요~

Copy link
Member Author

Choose a reason for hiding this comment

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

innerText 변경만 할 줄 알고 더 추가할 방법은 생각 못했는데 검색해보니 appendChild를 통해서 변경할 수도 있네용. 감사합니다~~

Comment on lines +38 to +39
content.appendChild(checkBtn);
content.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.

appendChild()를 반복해서 사용하고 계신데, 한 번에 여러 Element를 추가할 수 있는 append()는 어떨까요? 두 함수의 차이점을 비교해보는 것도 좋을 것 같아요~

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
Member Author

Choose a reason for hiding this comment

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

오 방법 감사합니다!! 일일이 쓰기 추가하기 힘들었어욥 ㅠ

script.js Outdated
Comment on lines 47 to 49
if (saveToDo) {
toDoInput.value = '';
Doing(saveToDo);
Copy link
Member

Choose a reason for hiding this comment

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

입력값 체크도 따로 해주셨네요!👍 그런데 지금은 공백이 입력되는 경우에는 예외처리가 되지 않고 있어요. 공백값도 처리할 수 있도록 정규식(\s)이나 함수(trim())를 사용해봐도 좋을 것 같아요~!

Copy link
Member Author

Choose a reason for hiding this comment

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

미처 생각하지 못했던 부분이네요!!! 다음번엔 꼭 예외처리도 추가해볼게요 흐흐

style.css Outdated
Comment on lines 65 to 77
#doingText,
#doneText {
padding-left: 30px;
}

li {
list-style-type: none;
}

#check,
#delete {
margin-left: 10px;
}
Copy link
Member

Choose a reason for hiding this comment

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

공통된 스타일 선언이 몇 가지 보이는 것 같아요! 클래스나 id가 많아지게되면 유지보수가 어려울 수 있어 해당 Element를 포괄할 수 있는 클래스 명을 선언하고 하나의 클래스에 스타일을 지정하는 것도 좋을 것 같아요~!

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 저도 중복 코드에 대해서 생각을 많이 했었어요 ㅠ 다음번엔 꼭 적용해보이겠습니다 흐

Copy link

@chaaerim chaaerim left a comment

Choose a reason for hiding this comment

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

선호님 안녕하세요! 프론트 운영진 김채림입니다 😊
과제 너무 잘 봤습니다! 코드를 깔끔하게 작성해주셔서 리뷰하기에도 정말 수월했어요!
앞으로 제출해주실 과제들이 기대되는 1주차 과제였습니다~
완성도 있게 과제를 제출해주셔서 크게 고치실 점은 없고 리뷰 몇 개 남겨봤습니다 ㅎㅎ

과제하시느라 수고 많으셨고 스터디 시간에 뵙겠습니다!

Comment on lines +59 to +67
feat : 새로운 기능에 대한 커밋
fix ! 버그 수정에 대한 커밋
build : 빌드 관련 파일 수정에 대한 커밋
chore : 그 외 자잘한 수정에 대한 커밋
ci : CI관련 설정 수정에 대한 커밋
docs : 문서 수정에 대한 커밋
style : 코드 스타일 혹은 포맷 등에 관한 커밋
refactor: 코드 리팩토링에 대한 커밋
test : 테스트 코드 수정에 대한 커밋

Choose a reason for hiding this comment

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

헐 이걸 옮겨 적으셨다니 감동 그 잡채..

Copy link
Member

Choose a reason for hiding this comment

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

감동 그 잡채 22..

Copy link
Member Author

Choose a reason for hiding this comment

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

ㅋㄷㅋㄷ 좋은 걸 전수해주셔서 감사합니다

Comment on lines +57 to +64
@font-face {
font-family: 'SEBANG_Gothic_Bold';
src: url('https://cdn.jsdelivr.net/gh/projectnoonnu/noonfonts_2104@1.0/SEBANG_Gothic_Bold.woff')
format('woff');
font-weight: normal;
font-style: normal;
}

Choose a reason for hiding this comment

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

폰트까지 적용해주셨네요! 폰트 예쁜 것 같아요~

style.css Outdated
Comment on lines 7 to 23
position: absolute;
display: flex;
flex-direction: column;
flex-wrap: wrap;
align-items: stretch;
top: 0;
bottom: 0;
left: 0;
right: 0;
width: 500px;
height: 700px;
border: 10px black;
border-radius: 7%;
background-color: white;
box-shadow: 1px 1px 3px 1px #dadce0;
line-height: 1;
margin: auto;

Choose a reason for hiding this comment

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

position: absolute를 사용하고 top, bottom, left, right를 지정해주시는 것 보다 margin-top을 사용하시면 더 간단하게 css 적용할 수 있을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

오 box 가운데로 옮기려고 애 썼는데 간단히도 할 수 있군요. 다음번엔 꼭 참고할게요 감사합니다~

Copy link
Member

@SeieunYoo SeieunYoo left a comment

Choose a reason for hiding this comment

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

안녕하세요 선호님 !

전반적으로 코드가 깔끔하고 커밋도 깔끔하게 날려주셔서 리뷰어의 입장에서 불편함 없이 리뷰할 수 있었습니다 :)
다른 분들께서 코멘트를 잘 달아주셔서 저는 몇가지 suggestion 만 드리겠습니다~!! 추가적인 토글 기능도 고민해보시면 좋을 것 같아요 👍

스터디 시간때 뵙겠습니다 ~! 수고 많으셨어요 🎉

Comment on lines +59 to +67
feat : 새로운 기능에 대한 커밋
fix ! 버그 수정에 대한 커밋
build : 빌드 관련 파일 수정에 대한 커밋
chore : 그 외 자잘한 수정에 대한 커밋
ci : CI관련 설정 수정에 대한 커밋
docs : 문서 수정에 대한 커밋
style : 코드 스타일 혹은 포맷 등에 관한 커밋
refactor: 코드 리팩토링에 대한 커밋
test : 테스트 코드 수정에 대한 커밋
Copy link
Member

Choose a reason for hiding this comment

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

감동 그 잡채 22..

Comment on lines +38 to +39
content.appendChild(checkBtn);
content.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.

참고자료 남겨두고 갑니다~!

index.html Outdated
<span id="doingText">Doing (0)</span>
<ul id="doing"></ul>
</div>
<hr />
Copy link
Member

Choose a reason for hiding this comment

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

<hr/> 태그 아주 좋아요~! 저도 자주 쓰고 있거든요 ㅎㅎ

Copy link
Member

Choose a reason for hiding this comment

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

우와 이 태그 좋네요... 📝

script.js Outdated
Comment on lines 16 to 23
function checkToDo(event) {
const checkItem = event.target.parentElement;
event.target.remove();
//Done에 들어오면 체크박스 지우기
doneList.appendChild(checkItem);
doingText.innerText = `Doing (${toDoList.childElementCount})`;
doneText.innerText = `Done (${doneList.childElementCount})`;
}
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
Member Author

Choose a reason for hiding this comment

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

아아 ㅠ.ㅠ 그 부분을 생각못했네요 꼭 추가할게요 감사합니다~!!!

Copy link
Member

@kongnayeon kongnayeon left a comment

Choose a reason for hiding this comment

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

안녕하세요 선호님! 이번 주 코드 리뷰 파트너 강나연이라고 합니다 😸

코드를 엄청 깔끔하게 짜셔서 이해하기 편했어요!! 공백 입력 시 알림창이나 토글 시 아이콘 다르게 주시는 것들... 짱이에요 배워 갑니다!

제가 JS 지식이 아직 부족해 엄청난 도움이 되지는 못하겠지만... 중복되는 부분을 제거하기 위해 많이 노력하셨다고 하셔서...! 한 가지 제안 남겨 뒀습니다!

그럼 스터디 시간에 봬요! 📚

index.html Outdated
<span id="doingText">Doing (0)</span>
<ul id="doing"></ul>
</div>
<hr />
Copy link
Member

Choose a reason for hiding this comment

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

우와 이 태그 좋네요... 📝

doneText.innerText = `Done (${doneList.childElementCount})`;
}
}

Copy link
Member

Choose a reason for hiding this comment

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

아이콘 다르게 주신 것도 좋아요!!!

doneText.innerText = `Done (${doneList.childElementCount})`;
}
}

Copy link
Member

Choose a reason for hiding this comment

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

doingText.innerText = `Doing (${toDoList.childElementCount})`;
doneText.innerText = `Done (${doneList.childElementCount})`;

요 부분은 조건문 밖으로 빼서 한 번만 써도 좋을 것 같아요! 밑의 deleteToDo 함수에서 또 사용하던데 두 줄이긴 하지만 함수로 만들어도 좋을 것 같아요...!

toDoInput.value = '';
Doing(saveToDo);
} else {
alert('Check Content plz :)');
Copy link
Member

Choose a reason for hiding this comment

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

알림창 좋네요...!! 배워 갑니당....

const checkItem = event.target.parentElement;
const toDoing = event.target;
toDoing.innerText = '⬆️';
doneList.appendChild(checkItem);
Copy link
Member

Choose a reason for hiding this comment

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

appendChild 를 사용하셨네요! 저는 appendChild의 개념이 조금 헷갈려서 많이 찾아봤었는데, 선호님 코드 참고하니 더 이해가 되는 것 같네요 ㅎㅎ

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.

6 participants