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

[2주차 기본/생각 과제] Todolist, Velog #3

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

Conversation

borimong
Copy link
Collaborator

@borimong borimong commented Oct 14, 2022

✨ 구현 기능 명세

  • 기본 과제
    1. Todo List
  • 버튼 클릭에 따라 뷰 전환하기
  • 리스트에 목록 추가 및 삭제하기
    1. Velog
  • Dropdown 구현하기
  • 태그 작성 및 삭제 구현하기
  • 심화 과제
  • 생각 과제

🎁 PR Point

  • 이벤트 구현할 때 부모 요소에 이벤트리스너를 부착하고, e.target 을 이용해 이벤트를 add 했습니다!
  • 이 과정에서 이벤트 버블링이 발생하지 않게끔 stopPropagation 함수를 모든 이벤트 리스너 함수의 최상단에 작성해 주었는데, 이걸 이렇게 쓰는 게 맞나.. 잘 모르겠네요ㅠㅠ 아시는 분 계시다면 피드백해주시면 감사하겠습니다!1
  • innerHTML 을 사용해서 html 요소를 변경했는데 지난 세미나에서 팟쨩님이 보안 이슈가 있다고 하셔서.. 대체할 수 있는 방법이 있다면 배우고 싶습니다!!(궁금궁금)

😭 소요 시간, 어려웠던 점

  • 10h
  • 이벤트 함수와 add 처럼 기능을 구현하는 함수를 분리하지 않아 코드가 복잡해져 머리가 아팠습니다..ㅎㅎ 1함수 1기능은 클린 코드의 기본인데.. 반성했습니댜..!

😎 구현 결과물

화면_기록_2022-10-15_오후_1_01_56_AdobeExpress

화면_기록_2022-10-15_오후_1_00_33_AdobeExpress

Copy link

@NaveOWO NaveOWO left a comment

Choose a reason for hiding this comment

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

현수 너무 수고했다🌟🌟🌟
세미나때 배운 이벤트 버블링을 잘 이해하고 있는 것 같아서 멋져!!!
로직에서 예외처리도 깔꼼하게 잘해줬다!!!! 멋져~~~~~~~~~~~~

Comment on lines +20 to +40
const target = e.target;
switch (target.id) {
case 'today_button' :
$('.right').classList.add('hidden');
$('.left').classList.add('full');
$('.left').classList.remove('hidden');
$('.right').classList.remove('full');
break;
case 'tomorrow_button' :
$('.left').classList.add('hidden');
$('.right').classList.add('full');
$('.right').classList.remove('hidden');
$('.left').classList.remove('full');
break;
case 'both_button' :
$('.right').classList.remove('hidden');
$('.left').classList.remove('full');
$('.left').classList.remove('hidden');
$('.right').classList.remove('full');
break;
}
Copy link

Choose a reason for hiding this comment

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

swich문을 이용해서 상태 변경하는거 너무 멋지다!!
근데 case 안에 매개변수를 제외한 같은 로직들이 반복되기 때문에, classList를 변경하는 함수를 만들어서 사용해도 좋을 것 같아요!!

Choose a reason for hiding this comment

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

switch문으로 쓸 수 있다는 생각은 안해봤는데,
이렇게 제이쿼리로도 사용할 수 있겠다!

Comment on lines +46 to +47
left_form_button.addEventListener("click", (e) => {
e.preventDefault();
Copy link

Choose a reason for hiding this comment

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

form 에서 이벤트가 일어날 때 새로고침을 방지해주는 로직 멋져!
근데 input 태그를 사용하면 e.preventDefault() 사용하지 않아도 되는데 form태그를 쓴 이유가 궁금해요!

Comment on lines +80 to +93
function onAddRight(right_input) {
const currentLi = document.createElement('li');
currentLi.innerHTML = right_input.value;

//delete 아이콘 만들어서 리스트에 추가하기
const currentDeleteBtn = document.createElement('span');
currentDeleteBtn.innerHTML = "delete";
currentDeleteBtn.classList.add('material-symbols-outlined');
currentLi.appendChild(currentDeleteBtn);

currentLi.classList.add('grid');

return currentLi;
}
Copy link

Choose a reason for hiding this comment

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

이 함수도 onAddLeft 함수랑 같은 로직을 갖고있어서, 함수를 하나로 합칠 수 있지 않을까요!?
매개변수를 left, right로 다르게 주면 될 것 같아요🌟

Comment on lines +97 to +108
left_list.addEventListener("click", (e) => {
e.stopPropagation();
const target = e.target;
switch (target.nodeName) {
case 'SPAN':
const n = e.target.parentNode;
left_list.removeChild(n);
break;
}

}
)
Copy link

Choose a reason for hiding this comment

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

전체 section 안에서 모든 click 이벤트를 감지한 뒤에, 그게 삭제버튼인지 아닌지 판단하는건 불필요하게 이벤트를 감지할 수 있을 것 같아요!!
delete 버튼을 생성할 때, 바로 그 버튼에 click이벤트를 달아주면 불필요한 로직을 없앨 수 있어요!!

const tag_list = $('.tagList');
const lists = $$('li');

input.addEventListener("keypress", (e) => {
Copy link

Choose a reason for hiding this comment

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

keypress이벤트는 키보드를 누르는 시간동안(내내) 이벤트를 감지하기 때문에, 한번 엔터를 누르는 이벤트를 감지하려면 keydown이나 keyup 이 더 적절하지 않을까요!?
그리고 keypress의 사용이 지양되는것 같아요 헤헤
https://developer.mozilla.org/en-US/docs/Web/API/Element/keypress_event

Comment on lines +15 to +18
for (let node of tag_list.childNodes){
if (node.innerText === currentLi.innerText){
count++;
break;
Copy link

Choose a reason for hiding this comment

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

현재 있는 태그를 for문으로 돌면서 태그가 중복되었는지 확인하는 것도 좋지만 태그를 생성할때마다 태그를 담아두는 배열에 추가하고 그 배열에서 중복을 확인해보는 방법도 있어요!!

Copy link

@talkingOrange talkingOrange left a comment

Choose a reason for hiding this comment

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

velog에 우리 단체사진 넣었구나ㅎㅎ 스윗해!!

Comment on lines +20 to +40
const target = e.target;
switch (target.id) {
case 'today_button' :
$('.right').classList.add('hidden');
$('.left').classList.add('full');
$('.left').classList.remove('hidden');
$('.right').classList.remove('full');
break;
case 'tomorrow_button' :
$('.left').classList.add('hidden');
$('.right').classList.add('full');
$('.right').classList.remove('hidden');
$('.left').classList.remove('full');
break;
case 'both_button' :
$('.right').classList.remove('hidden');
$('.left').classList.remove('full');
$('.left').classList.remove('hidden');
$('.right').classList.remove('full');
break;
}

Choose a reason for hiding this comment

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

switch문으로 쓸 수 있다는 생각은 안해봤는데,
이렇게 제이쿼리로도 사용할 수 있겠다!

@borimong borimong changed the title [2주차 기본 과제] Todolist, Velog [2주차 기본/생각 과제] Todolist, Velog Dec 2, 2022
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