[14기 양아름] step1 돔 조작과 이벤트 핸들링으로 메뉴 관리하기 #268
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dhrod0325
reviewed
Jul 16, 2022
Comment on lines
+54
to
+58
| if (deleteFlag) { | ||
| const deleteMenu = currentTarget.closest('.menu-list-item'); | ||
| deleteMenu.remove(); | ||
| } | ||
| updateEspressoMenuCount(-1); |
There was a problem hiding this comment.
Suggested change
| if (deleteFlag) { | |
| const deleteMenu = currentTarget.closest('.menu-list-item'); | |
| deleteMenu.remove(); | |
| } | |
| updateEspressoMenuCount(-1); | |
| if (!deleteFlag) { | |
| return; | |
| } | |
| const deleteMenu = currentTarget.closest('.menu-list-item'); | |
| deleteMenu.remove(); | |
| updateEspressoMenuCount(-1); |
지금은 취소를 눌러도 updateEspressoMenuCount가 호출되네요 early return 이 이런 오류를 예방하기 좋은 것 같아요! 😀
| @@ -0,0 +1,5 @@ | |||
| export function $(selector) { | |||
| const element = document.querySelector(selector); | |||
| if (element === null) throw new Error('element is null'); | |||
There was a problem hiding this comment.
selector 이름을 만약에 잘못 입력 했을 때 예외처리를 안 해주게 되면 다음 스크립트가 동작을 안하게 되겠네요.!
엄격하게 체크하려고 의도 하신 건지 궁금해요~!😮
|
|
||
| espressoMenuList.addEventListener('click', (event) => { | ||
| const currentTarget = event.target; | ||
| switch (currentTarget?.dataset.action) { |
There was a problem hiding this comment.
옵셔널체이닝 좋네요! 👍 data로 이벤트를 분기 하는 것 도 좋은 것 같아요!
There was a problem hiding this comment.
dataset 을 사용해서 switch 쓰도록 한 방식 좋네요! 저도 사용해봐야겠네용 ㅎㅎ
| event.preventDefault(); | ||
| const menuName = espressoMenuInput.value; | ||
|
|
||
| if (menuName) { |
There was a problem hiding this comment.
early return 해주면 뒤 코드의 흐름을 보기 더 쉬울 것 같아요~!
Comment on lines
+3
to
+7
| let orderedMenuCount = 0; | ||
| const espressoMenuCount = $('.menu-count'); | ||
| const espressoMenuForm = $('#espresso-menu-form'); | ||
| const espressoMenuInput = $('#espresso-menu-name'); | ||
| const espressoMenuList = $('#espresso-menu-list'); |
| const currentTarget = event.target; | ||
| switch (currentTarget?.dataset.action) { | ||
| case 'edit': | ||
| const newMenuName = prompt('메뉴명을 수정하세요.'); |
yooveloper
reviewed
Jul 17, 2022
| </button> | ||
| <button | ||
| type="button" | ||
| data-action="delete" |
There was a problem hiding this comment.
저는 name attribute 를 추가하여 이벤트를 분기했는데 data-action을 사용하셨네요 !👍
yooveloper
reviewed
Jul 17, 2022
| const espressoMenuInput = $('#espresso-menu-name'); | ||
| const espressoMenuList = $('#espresso-menu-list'); | ||
|
|
||
| const updateEspressoMenuCount = (number) => { |
There was a problem hiding this comment.
+1, -1 를 인자로 넘겨서 처리 하셨군요!
menu list의 length를 활용하는 방법도 고려해보면 좋을것 같아요 !
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎯 step1 요구사항 - 돔 조작과 이벤트 핸들링으로 메뉴 관리하기
prompt인터페이스를 활용한다.confirm인터페이스를 활용한다.<ul id="espresso-menu-list" class="mt-3 pl-0"></ul>안에 삽입해야 한다.✨ 상세 설명
dom.js
index.js
onSubmit이벤트를 추가하여 input에 메뉴 이름이 입력되어 있을 시 메뉴 리스트에 메뉴를 추가한다.reset()을 활용하여 input을 빈 값으로 초기화한다.updateEspressoMenuCount()함수를 호출한다.#espresso-menu-list)에 이벤트를 추가하여 이벤트 위임을 통해 자식에게 전달되도록 한다.data-action으로 어떤 버튼이 클릭되었는지 확인한다.updateEspressoMenuCount()함수를 호출한다.🤔 더 고민해보고 싶은 것
index.js에 모든 걸 다 맡기는 식으로 개발을 하였습니다...🥲 이후에 메뉴별로 상태를 관리해야 할 경우를 대비하여 남은 시간동안 더 고민해볼게요!