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.

Sign up for GitHub

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주차] 최지원 미션 제출합니다. #3

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

[1주차] 최지원 미션 제출합니다. #3

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2174809
feat: 기본 UI 구현
jiwonnchoi Sep 5, 2024
3556734
feat: 입력창 토글 기능 구현
jiwonnchoi Sep 6, 2024
1434121
feat: 할 일 추가 함수 구현
jiwonnchoi Sep 6, 2024
8ec025d
feat: 토글, 삭제 함수 구현
jiwonnchoi Sep 6, 2024
e3313a3
feat: 날짜 및 현재시각 표기
jiwonnchoi Sep 6, 2024
a254567
feat: 로컬스토리지 데이터 처리 구현
jiwonnchoi Sep 6, 2024
3152ab0
feat: 반응형 구현
jiwonnchoi Sep 6, 2024
4de67aa
style: 클래스명 수정 및 rem 단위 변환
jiwonnchoi Sep 6, 2024
8f7e29b
feat: 공통 기능 함수화
jiwonnchoi Sep 6, 2024
c08b6fa
feat: 전체, 완료된 항목 개수 표기 추가
jiwonnchoi Sep 6, 2024
86260ea
feat: 입력창 애니메이션 추가
jiwonnchoi Sep 6, 2024
3d0989b
fix: 스타일 수정 및 주석 추가
jiwonnchoi Sep 7, 2024
ca4e76e
chore: 버셀 배포
jiwonnchoi Sep 7, 2024
3d044fe
refactor: 아이콘 png에서 svg로 변경
jiwonnchoi Sep 10, 2024
28d46e6
refactor: 현재시각 1초지연 방지
jiwonnchoi Sep 10, 2024
970222b
refactor: did 네이밍 done으로 변경
jiwonnchoi Sep 10, 2024
cd12e36
refactor: addTodoItem의 input값 캐싱
jiwonnchoi Sep 10, 2024
780ce6d
refactor: deleteItem 중복호출 수정
jiwonnchoi Sep 11, 2024
a8f23ca
refactor: 전역 스타일 추가
jiwonnchoi Sep 11, 2024
eb9f93e
refactor: toggleForm 로직 분리 및 불리언 추가
jiwonnchoi Sep 11, 2024
9ef7e0e
refactor: 버튼생성 이벤트를 상위에 위임하도록 수정
jiwonnchoi Sep 11, 2024
f6d8336
fix: 배열 각 항목에 고유id 추가
jiwonnchoi Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,26 @@ const showMessage = document.querySelector(".show-input"); // 입력창 열고

// 입력창 열고 닫기 함수
const toggleForm = () => {
Copy link
Member

Choose a reason for hiding this comment

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

이런식으로 스타일을 통해서 관리하는 것도 좋지만 입력창의 상태(열림/닫힘)를 명확하게 추적하기 위해 boolean 변수로 상태를 관리하여 여닫는것도 유지보수하기엔 더 편하니까 한번 사용해봐도 좋을 것 같아요 :)_

Copy link
Author

Choose a reason for hiding this comment

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

클래스명은 워낙 지정하기 나름이라 혼동할 수도 있고, html/css에서 변경될 가능성도 있어 확실히 클래스 변경에 의존하는 것보다는 말씀해주신 것이 훨씬 직관적이고 혼란을 줄일 수 있겠다는 생각이 듭니다! 좋은 의견 주셔서 감사합니다☺️

form.style.display = form.style.display === "none" ? "flex" : "none";
showMessage.innerHTML =
form.style.display === "none" ? "입력창 불러오기" : "입력창 다시닫기";
if (form.classList.contains("show")) {
form.classList.remove("show");
form.classList.add("hide");

form.addEventListener(
Copy link
Collaborator

Choose a reason for hiding this comment

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

toggleForm 함수가 실행될 때마다 animationend 이벤트 핸들러가 새로 추가되도록 코드가 작성되어있는 부분을 수정하면 좋을 것 같아요!

ddEventListener()의 3번째 파라미터로 { once : true } 전달해 주어서, 이벤트가 발생할 때 한 번만 동작하게 되긴 하지만,
toggleForm이 다시 실행될 때마다 새로운 이벤트 리스너가 추가되어 한 번 실행 -> { once : true } 로 인해 삭제 -> 또 다시 토글 발생 -> 새로운 이벤트 리스너 추가 -> { once : true } 로 인해 삭제… 가 반복되는 불필요한 동작이 반복되는 것 같아요!!

때문에 이 방식보다 { once: true } 옵션을 쓰지 않고,
toggleForm 함수 외부에서 이벤트 리스너를 한 번만 등록하고
toggleForm 함수 내부에서는 클래스를 토글하는 동작만 수행하도록 로직을 분리해주는 것이 최적화와 유지보수 측면에서 더 깔끔할 것 같아요!

`
// 애니메이션 종료 후 처리하는 리스너 (한 번만 추가)
const handleAnimationEnd = (e) => {
if (form.classList.contains('hide')) {
form.style.display = 'none';
form.classList.remove('hide');
}
};

// 단순히 클래스만 토글하도록!!

const toggleForm = () => {
if (form.classList.contains('show')) {
form.classList.remove('show');
form.classList.add('hide');
showMessage.innerHTML = '입력창 불러오기';
} else {
form.style.display = 'flex';
form.classList.remove('hide');
form.classList.add('show');
showMessage.innerHTML = '입력창 다시닫기';
}
};

// 이벤트 리스너 초기에 1번만 등록!!
form.addEventListener('animationend', handleAnimationEnd);
`

Copy link
Author

Choose a reason for hiding this comment

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

수정방향 코드까지 너무 감사드립니다!! 처음에 클래스 토글까지 구현한 상태에서 애니메이션을 추가하여 한번에 쓰려하다 보니 불필요한 동작이 일어나게 되는 것까지 고려하지 못한 것 같습니다. ㅜㅜ 한 함수에서 담당하는 기능이 최소화되도록 분리하여 쓰는 습관을 길러야겠다는 생각이 많이 듭니다.

"animationend",
() => {
form.style.display = "none";
form.classList.remove("hide");
},
{ once: true }
);
Copy link
Member

Choose a reason for hiding this comment

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

입력창을 열고 닫을 때 애니메이션을 적용한 디테일 챙기는 부분 정말 좋네요!

showMessage.innerHTML = "입력창 불러오기";
} else {
form.style.display = "flex";
form.classList.remove("hide");
form.classList.add("show");

showMessage.innerHTML = "입력창 다시닫기";
Copy link

Choose a reason for hiding this comment

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

animationend라는 이벤트 리스너 타입을 활용하여 UI를 신경 쓴 부분이 인상깊습니다. 저는 처음 본 방법이라 신기하고, JS로 어느정도 수준의 UI까지 활용가능할 수 있는지 호기심이 생겼습니다.

}
};

// 이벤트 리스너 등록 및 기존 데이터 불러오기 함수
Expand Down
34 changes: 33 additions & 1 deletion style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,29 @@ main {
justify-content: space-between;
}

@keyframes slideDownFadeIn {
from {
opacity: 0;
transform: translateY(-20px); /* 위에서 아래로 */
}
to {
opacity: 1;
transform: translateY(0); /* 제자리 */
}
}

@keyframes slideUpFadeOut {
from {
opacity: 1;
transform: translateY(0);
}

to {
opacity: 0;
transform: translateY(-20px);
}
}

.input-box {
width: 50%;
height: 1.875rem;
Expand All @@ -65,6 +88,15 @@ main {
padding: 0 0.625rem;
}

.input-box.show {
display: flex;
Copy link
Member

Choose a reason for hiding this comment

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

.input-box에서 display: flex를 설정한 후, .input-box.show에도 다시 display: flex를 설정되고 있어요. 이부분은 없애도 좋을 것 같아요

Suggested change
display: flex;

animation: slideDownFadeIn 0.3s ease-out forwards;
}

.input-box.hide {
animation: slideUpFadeOut 0.3s ease-out forwards;
}

input {
width: 70%;
border: transparent;
Expand Down Expand Up @@ -116,7 +148,7 @@ ul {
}

footer {
width: 12.5rem;
width: 14rem;
display: flex;
flex-direction: row;
justify-content: space-between;
Expand Down