-
Notifications
You must be signed in to change notification settings - Fork 0
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주차 기본 과제] 웨비들의 냠냠 🍰 창업🏠 손님을 모셔오자!🌈 #2
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와 디자인 바꾼거랑 소주잔 아이콘 넣은거 너무 귀엽다 ㅋㅋㅋㅋㅋㅋㅋ 혹시 마우스 포인터는 노트북 설정으로 바꾼거야? 아니면 이번 과제에서 따로 코드를 넣은거야...? 😲
전체적으로 코드가 굉장히 깔끔하고 가독성이 좋아서 놀랐어!! 이렇게 짤 수 있는 걸 난 왜 뒤죽박죽 스파게티로 짠걸까 싶은... 모달 작동이나 필터링 관련 부분도 함수로 만들어줬으면 조금 더 좋았을 것 같아ㅎㅎ
고생 많았어 짱!!
var btnClose=document.querySelectorAll('.selected-ctgr .cancel-btn') | ||
var targetID, targetClass; | ||
const selectedCtgr=[]; | ||
var ctgrAllList=[".ctgr-dish",".ctgr-tang",".ctgr-side",".ctgr-alcohol"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 카테고리 목록 리스트 같은거, 먼가 이왕 더미데이터 만든김에 하드코딩보다는 데이터 만들어놓은 거에서 가져와도 좋을듯!! ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오오 좋은 생각 !!
if (targetID==="#ctgr-all"){ | ||
for (var t=0;t<ctgrAllList.length;t++){ | ||
for (var j=0;j<document.querySelectorAll(ctgrAllList[t]).length;j++){ | ||
document.querySelectorAll(ctgrAllList[t])[j].style.display='block'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오!! 카테고리 선택에 따라 해당하는 목록의 style을 block/none으로 바꿔서 필터링을 구현했구나!!!! 나는 왜 이 생각을 못한걸까...
} | ||
} | ||
else{ | ||
targetClass=targetID.replace('#','.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 요거 신박하군 ㅋㅋㅋ 같은 명칭을 class로도 쓰고 id로도 쓴건가? querySelectorAll()
말고 selectElementsByClassName('명칭')
이랑 selectElementById('명칭')
를 써도 될거같다는 생각?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아무래도 그게 좀 더 가독성 있는 코드긴 하겠다!
피드백 고마워 ㅎㅎ
<div class="nav-items">전체</div> | ||
</div> | ||
</label> --> | ||
<a href="#ctgr-all" class="nav-container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 카테고리 부분에 a태그로 href를 준게 신박해!! 혹시 이렇게 해놓으면 따로 장점이 있을까? 카테고리 클릭할 때마다 url주소가 ~.com/#ctgr-all 이런식으로 바뀔거같은데 맞나?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞아 !
장점이라고 하면 카테고리에 대한 링크 구분이 확실히 된다는 거??
근데 클릭이벤트 구현할 때 a태그를 쓸지 말자라는 아티클도 있더라고 ~.. 좀 더 고민해봐야할 것 같긴행
</nav> | ||
<div id="ctg-cards-wrap"> | ||
<div class="selected-ctgr"> | ||
<div id="ctgr-all" style="display:none;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default로 display:none을 주어야해서 인라인 속성을 준거같은데 그래두 인라인 속성은 지양하는게 좋으니 css파일에서 지정해놨어도 좋았겠다 !!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오키오키 인라인 지양 확인!
this.parentNode.parentNode.style.display='none' | ||
targetClass="."+this.parentNode.parentNode.getAttribute('id'); | ||
if (targetClass===".ctgr-all"){ | ||
nonSelectedCtgr = ctgrAllList.filter(x=> !selectedCtgr.includes(x)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나 이부분 구현하는거 진짜 애먹었는데 ~~ 현재 선택되어 있는 카테고리 목록을 selectedCtgr
에 항상 저장해주고 모든 카테고리 리스트(ctgrAllList
)에서 이를 빼줌으로써 구현한거구나? 엄청 심플해..
근데 궁금한게, 지금 이 길다란 for문이 아마 카테고리 해제하는 부분을 담당하는 것 같은데, 그러면 해제해준 뒤에 selectedCtgr
변수에서 해당 카테고리를 제거해야하지 않아? 근데 그부분이 안보이는 것 같아서 궁금해 내가 못찾는건가
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어머 맞아 .
지금 확인해보니까
전체, 다른카테고리 있을 때 다른카테고리를 해제하면 바로 없어지네.(원래 전체에 의해 있어야하는데)
그 부분이 말해준 부분인 것 같아 !! 으악
다시 기능 수정해야겠다 고마워 ㅎㅎ
return "ctgr-alcohol"; | ||
} | ||
} | ||
function postTitle(post){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
element만드는 부분을 각 구성요소별로 함수화한게 무척 깔끔해서 가독성이 좋아!! b
font-family: "establishRetrosansOTF"; | ||
} | ||
|
||
/* -----------------헤더에 대한 css ---------------------- */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우왕 깔끔한 주석 구분 너무좋아유
filter: invert(32%) sepia(41%) saturate(1448%) hue-rotate(118deg) brightness(88%) contrast(86%); | ||
} | ||
.card .icon:hover { | ||
filter: invert(51%) sepia(8%) saturate(4316%) hue-rotate(333deg) brightness(89%) contrast(81%); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
호오..!? icon이 svg파일이라서 색이 바뀌는걸 filter로 구현한걸까? 이거 진짜 신박한데? 나는 다른 사진으로 교체할 생각밖에 안햇어
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
svg 색상 변경하는 거 찾다가 알게 되었어!
근데 이건 색상 별로 필터값을 찾아야하는 단점이 있긴 하다..
덕분에 위 사이트도 알게 됨 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넘넘 수고 많았어!!! 담주나 다담주 정도에 금잔디조 어셈블하쟈 ㅎㅎ
img.setAttribute('alt','메뉴이미지') | ||
return img | ||
} | ||
for (var i=0;i<data.length;i++){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 var 는 let 으로 변경해주면 좋을 것 같아!!! :)
/** | ||
* nav에서 선택된 카테고리에 따라 카드, 태그 생성/삭제함 | ||
*/ | ||
var target=document.querySelectorAll('.nav-container'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보니까 전체적으로 var 을 이용했는데 const 를 지향해보는 것도 좋을 것 같아!! 혹시 var 를 쓴 특별한 이유가 있을까???? :)
function ctgrMatch(post){ | ||
if (post.category==="안주"){ | ||
return "ctgr-dish"; | ||
} | ||
else if (post.category==="탕"){ | ||
return "ctgr-tang"; | ||
} | ||
else if (post.category==="사이드"){ | ||
return "ctgr-side"; | ||
} | ||
else if (post.category==="술"){ | ||
return "ctgr-alcohol"; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분 switch case 문으로 작성해보는 건 어떨까??? :)
h2.textContent=post.menuName | ||
return h2 | ||
} | ||
function postTag(post){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수명도 잘 지었구 기능별로 함수를 잘 분리한 것 같아!! 최고다!! :)
var modalOpenTarget=document.querySelectorAll('.card-tag .modal-open-icon'); | ||
// var modalCloseTarget=document.querySelectorAll('.modal-close-icon'); | ||
for(var i=0;i<modalOpenTarget.length;i++){ | ||
modalOpenTarget[i].addEventListener('click',function(){ | ||
this.parentNode.parentNode.querySelector('.tag-modal').style.display='block'; | ||
}); | ||
} | ||
var modalCloseTarget=document.querySelectorAll('.modal-close-icon'); | ||
for(var i=0;i<modalCloseTarget.length;i++){ | ||
modalCloseTarget[i].addEventListener('click',function(){ | ||
this.parentNode.style.display='none'; | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 부분 함수로 분리해주면 더 가독성 좋아지겠따!!!!! :)
export default [ | ||
{ | ||
"menuName":"국물닭발", | ||
"menuTag":["#칼칼","#닭발엔소주", "#국물","#냠냠"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기서 # 은 before 로 붙여봐도 좋을 것 같댜!!! :)
<a href="#ctgr-dish" class="nav-container"> | ||
<div class="nav-items">안주</div> | ||
</a> | ||
<a href="#ctgr-tang" class="nav-container"> | ||
<div class="nav-items">탕</div> | ||
</a> | ||
<a href="#ctgr-side" class="nav-container"> | ||
<div class="nav-items">사이드</div> | ||
</a> | ||
<a href="#ctgr-alcohol" class="nav-container"> | ||
<div class="nav-items">술</div> | ||
</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 부분은 구조가 전체적으로 반복되니까 map 을 사용해 봐도 좋을 것 같아!! :)
<div class="category-container"> | ||
<h2> 안주</h2> | ||
<h3 class="cancel-btn">X</h3> | ||
</div> | ||
</div> | ||
<div id="ctgr-tang" style="display:none;"> | ||
<div class="category-container"> | ||
<h2> 탕</h2> | ||
<h3 class="cancel-btn">X</h3> | ||
</div> | ||
</div> | ||
<div id="ctgr-side" style="display:none;"> | ||
<div class="category-container"> | ||
<h2> 사이드</h2> | ||
<h3 class="cancel-btn">X</h3> | ||
</div> | ||
</div> | ||
<div id="ctgr-alcohol" style="display:none;"> | ||
<div class="category-container"> | ||
<h2> 술</h2> | ||
<h3 class="cancel-btn">X</h3> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 부분도 map 으로 처리해 주면 좋을 것 같아요잉 :)
var nonSelectedCtgr=[]; | ||
// 생성 | ||
for(var i=0;i<target.length;i++){ | ||
target[i].addEventListener('click',function(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기서 function 대신에 화살표 함수 써 보는 것두 좋을 것 같댜!! :)
var ctgrAllList=[".ctgr-dish",".ctgr-tang",".ctgr-side",".ctgr-alcohol"] | ||
var nonSelectedCtgr=[]; | ||
// 생성 | ||
for(var i=0;i<target.length;i++){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리턴 값이 없다면 forEach 를 써봐도 좋을 것 같아!! 그런데 여기서는 생성이니까 map 을 써서 새 배열을 만들어봐두 좋을 것 같다!!!! :)
else{ | ||
targetClass=targetID.replace('#','.'); | ||
for (var j=0;j<document.querySelectorAll(targetClass).length;j++){ | ||
document.querySelectorAll(targetClass)[j].style.display='block'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스명으로 dom 요소를 선택하구 싶은 거라면 getElementsByClassName 요거 사용해봐도 좋을 것 같아용!!
* nav에서 선택된 카테고리에 따라 카드, 태그 생성/삭제함 | ||
*/ | ||
var target=document.querySelectorAll('.nav-container'); | ||
var btnClose=document.querySelectorAll('.selected-ctgr .cancel-btn') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기서는 dom 요소를 저장하는 변수 네임이니까, 동사보다는 명사형으로 해 보면 어떨까??? 예를 들어 closeButton 같은??? :)
|
||
for(var j=0;j<target.length;j++){ | ||
btnClose[j].addEventListener('click',function(){ | ||
this.parentNode.parentNode.style.display='none' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기서는 parentNode 로 접근하는 것도 좋지만, 버튼의 parent parent 에 eventListener 를 붙이고, 버튼 이벤트를 감지하면 어떨까??? :)
const div=document.createElement('div') | ||
div.setAttribute('class','card-tag') | ||
const h3=document.createElement('p') | ||
h3.innerText=post.menuTag | ||
const icon=document.createElement('img') | ||
icon.setAttribute('src','./images/square-caret-down-solid.svg') | ||
icon.setAttribute('class','modal-open-icon') | ||
icon.setAttribute('alt','더보기아이콘') | ||
div.append(h3) | ||
div.append(icon) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 부분 mapa으로 새로운 배열 만들어서 innerHTML 로 넣어주면 좀 더 직관적일 수 잇을 것 같댜!!!! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금 코드 보니까 보미 언니는 post 로 data[i]를 받아와서,
컨테이너인 div 를 createElement로 만들고, 그 div 에 class 속성으로 card_tag 를 만들고 있자나??
근데 이렇게 하면, html 태그 요소가 한 눈에 보이지 않아서 가독성이 조금 떨어지는 것 같다고 생각햇어!!
그리고 나는 태그가 복수개일 것이라고 생각해서, 그 태그를 분리하는 작업을 map 을 이용해서 해주면 좋겠다는 뜻이어써!! 만약 태그가 복수개가 아니라면 그냥 innerHTML만 사용해주면 돼!! :)
✨ 구현 기능 명세
기본과제
✅
nav
✅
card article
+
아이콘 클릭시 태그 모달이 등장합니다.심화 과제
✅
목록
✅
새 상품 추가 페이지
1.
label, input
을 연결시켜서 구현합니다.2. 추가 버튼
→ 이미지 미리보기까지만 구현…! 메인페이지의 이미지는 임의!⁄로
✅
카드 띄울때 애니메이션
🌼 PR Point
1. 더미 데이터에서 가져오기
dummy.js
파일을 만들어서,script.js
파일에서 import해서 더미 데이터를 활용했습니다. 직접 js로 data를 활용해 dom을 만들어보는 경험이 처음이었어서 해맸는데, dom 선택자인querySelectorAll, createElement, this.parentNode
등을 이용해서 js단에서 요소를 만들 수 있었습니다.2. 태그 클릭 시 카테고리를 띄우고, 카드 연동하기
크게 태그를 클릭해서 1) 카테고리/카드를
생성
하는 부분과 2) 카테고리/카드를삭제
하는 부분으로 나눠서 구현했습니다.클릭되는 요소의 id값을 받아와 해당 id의 카테고리(카드 위에 생성되는 부분)를 만들었으며,
해당 카테고리를 dummy.js의 카드마다 다 지정해주어서
위처럼 카테고리에 해당하는 카드를 나타나게 지정해주었습니다 :)
이와 비슷하게 삭제 버튼을 눌렀을 때 display의 값을 none으로 해줌으로써 삭제 기능을 구현했습니다.
또한
전체
카테고리를 선택했을 경우에는 다른 필터를 지워도 카테고리가 남게 하고,전체
카테고리를 지워도 다른 필터가 남은 경우에 나머지 카테고리가 남게 하기 위해서,ctgrAllList
와selectedCtgr
를 비교해주는 부분을 만들었습니다.3. 카드마다 태그 모달 만들기
또한 이부분도 위의 카테고리/카드 생성 및 삭제와 마찬가지로 js의 dom에 대한 접근 및 style 변경을 이용한 기능으로 구현했습니다.
🥺 소요 시간, 어려웠던 점
8h
🌈 구현 결과물
카테고리 별로 태그,카드가 생성 및 삭제되는 부분입니다.
각 카드 별로 태그를 모달로 확인할 수 있습니다.