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주차 기본 과제 ] 비니의 냠냠 🍰 창업🏠 손님을 모셔오자!🌈 #3

Merged
merged 20 commits into from
Jul 16, 2023

Conversation

eunbeann
Copy link
Member

✨ 구현 기능 명세

기본 과제

  • nav
    • 종류 선택시 태그를 카드 섹션 위에 하나씩 부착하기
    • 태그별 상품 리스트 보여주기
      • 전체 → 다 보여주기
      • 종류별 → 필터 기능
    • X 클릭시
      • 카드 다시 정렬ㅋ
      • 종류 선택 해제
      • 태그 삭제
  • card article
    • + 아이콘 클릭시 태그 모달이 등장합니다.
    • 카드 위에 덮어씌워지는 창 → 해시태그 목록 담은 content 보여주기
    • x 버튼을 누르면 모달이 닫힙니다.

심화 과제 …ㅜ.^

  • 목록

    • 목록 선택 시 리스트로 새 상품 추가가 나타남
      • 새 상품 추가

        href=’/addCard’

  • 새 상품 추가 페이지

    • label, input 연결시켜 구현
      • 상품명
      • 태그 종류 , 로 구분해서 담기
      • 이미지
        • 파일 선택 후 이미지 보여주기 필수
    • 추가 버튼
      • 모든 정보 입력시 localStorage에 추가 후 main 페이지로 이동
      • 메인페이지에서 추가된 상품 조회 가능
  • 카드 띄울 때 애니메이션

    • 태그 기능으로 보여지는 상품 카드들이 달라질 때 동적인 애니메이션을 가지고 변경 됨

🌼 PR Point

  • 체크 박스 구현 할 때
    • checked 값을 바꿔주면 이벤트 발생이 아니라 속성 값만 바뀌어서 이벤트를 건드리는 쪽으로 하고 싶었는데 못함1!
      • 이벤트를 건드릴 수 있으면 굳이 함수 여러 개를 넣지 않아도
        // 체크 change에 따라 상태 변경 코드에 영향을 받아서 카드랑 태그가 생기고 사라지고가 될 텐데
  • 카드 섹션 삭제필터링해서 삭제해야할 때 만들어 놓은 컴포넌트 단을 세트로 삭제해야하는 부분을 어떻게 해야할지 감이 안잡혔음
    • 현재는 클래스 이름을 동적으로 설정해서 기능 구현 시켰지만 유지보수, 리펙토링 영역에서 좋은 방법은 아닌 것 같아서 더 고민 해야할 것 같음
  • 모달 구현에서 각 태그가 선택되게 하는 부분
    • 선택한 이벤트의 부모 요소로 넘어가서 다시 DOM 접근!!! 고민을 많이 했는데 내 코드들 중에 가장 마음에 드는 함수 😎

🥺 소요 시간, 어려웠던 점

  • 몇 번씩이나 길을 잘못들어서 코드를 아주 여러번…갈아엎음
  • 초반에 한 함수 안에 기능을 죄다 때려박아서 몇 번을 재작성 함.. 과제 시작 전에 해야할 기능 확실하게 정리하고 진행하는 연습 + 습관들이기 개발 시간 줄이자!!
  • 시험 기간이랑 겹쳐서 그 무엇에도 집중하지 못함…

⇒ 전반적으로 아직 내가 부족하구나를 느꼈던 과제 🥺 배울 점이 아직 한참 남았단 건 좋은거에요 아자아자

모든 코드리뷰 열렬히 환영 제발 해주세요 제발


🌈 구현 결과물

breadStreo

@eunbeann eunbeann requested review from pinktopaz and urjimyu April 21, 2023 07:54
@eunbeann eunbeann changed the title [ n주차 기본/심화/생각 과제 ] 비니의 냠냠 🍰 창업🏠 손님을 모셔오자!🌈 [ 2주차 기본 과제 ] 비니의 냠냠 🍰 창업🏠 손님을 모셔오자!🌈 Apr 21, 2023
Copy link

@Arooming Arooming left a comment

Choose a reason for hiding this comment

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

은빈이네 냠냐미 창업 넘 귀여워여~~~>__< 이번에 요 과제 갈아엎고 다시 한 1인으로써 감동적인 창업 스토리 잘 보고 갑니다~~^^ㅠㅠㅠ,,,

this.checked?showCards(checkedId):deleteCards(checkedId)
this.checked?addCategoryTag(checkedId):deleteCategoryTag(checkedId)
})
})

Choose a reason for hiding this comment

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

이거 구냥 이렇게 중간에 써도 잘 돌아가나여? (궁금궁금)
저는 dom에 안올리면 동작을 잘 안해서 이런식으로 돌아가게 하고 싶은 함수들 돔에 올려줬어요!
document.addEventListener("DOMContentLoaded", function () { // 여기에 함수 들어감! });

Copy link
Member Author

Choose a reason for hiding this comment

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

돔에 올린다는 생각을 따로 안해봤는데.. 잘 돌아가더라구요?!

생각해보니 저도 함수에 넣을까 하다가 렌더링을 따로 해줘야돼서 그냥 냅다 바깥으로 빼서 onload에 돌아가게 것 같네여

아마 showCard 같은 함수가 DOM을 건드려서 잘 돌아가는 것 같아효....

}

.section__tag-list::before {
content: "#";
content: "#";
}

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.

후후 저번 과제 코드리뷰로 받은 의견 .!!
코드리뷰 짱~

padding-bottom: 0.5rem;
white-space: normal;
overflow: auto;
/* text-overflow: ellipsis; */

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.

우왓!! 계속 생각했는데 놓쳤군요 센스쟁이 찝어줘서 고맙습니다 움쪽~

@Arooming
Copy link

Arooming commented Apr 21, 2023

아 그리구 카테고리 태그 모달에 있는 x 버튼 눌렀을 때, 해당 카테고리의 input 체크도 풀리고 카드도 사라지는 부분..!!

저는 let으로 빈 배열(categoryData) 하나 만들어서 구현했습니당! input 클릭하면 클릭한 카테고리의 id가 배열로 들어가고, 체크를 해제하면 배열에서 해당 카테고리는 빠지고 filter로 화면에는 나머지 카테고리에 해당하는 아이템만 남아있도록..!

// 배열에 데이터 저장
categoryData.push(checkedId);
// 데이터 필터
categoryData = categoryData.filter((it) => it !== checkedId);

도움이 됐으면 좋겟네여ㅠㅠ🫡🥳😇!!!

Copy link

@pinktopaz pinktopaz left a comment

Choose a reason for hiding this comment

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

은빈이 코드 여러번 엎었다더니 함수 잘 나눠서 작성해줬네!!
시험기간에 과제하느라 넘 수고 많아써:)
리팩토링도 잘 갈겨보자-!

@@ -13,6 +13,8 @@

</head>
<body>

<script type="module" src="main.js"></script>

Choose a reason for hiding this comment

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

js를 head에 놓았는데 용량이 크다면 head를 모두 부르는데 오래걸리고, 그만큼 body도 늦게 실행되어 사용자 입장에서 백지화면를 더 오래볼 수 있어요! 그래서 최적화를 위해서 script는 body 태그 최하단에 두는 게 조아요-! <script> 태그는 어디에 위치해야 할까요?

<input type="checkbox"/>
<p>쿠키</p>
</label>
<div class="nav__category">

Choose a reason for hiding this comment

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

조금 더 시맨틱하게 nav 안에서 ul-li 태그로 카테고리 버튼을 만들어줘도 될 것 같아요!

<p>쿠키</p>
</label>
<div class="nav__category">
<input type="checkbox" class="category" value="all" id="all"/>

Choose a reason for hiding this comment

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

id는 아무래도 클래스처럼 이 태그가 어떤 역할을 하는지를 보여주는 태그이니까 이런 경우에는 dataset을 이용해봐도 좋을 것 같아!
HTML 데이터셋(Dataset, data-*) 속성의 이해

Copy link

Choose a reason for hiding this comment

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

데이터셋 저도 배워가요👍🏻

</section>
<main>

<section id="cards">

Choose a reason for hiding this comment

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

화면 상에서 태그가 상단에 위치하는데 html 문서상에서는 하단에 있는게 어색하기도 하고, html은 기본적으로 코드 순서대로 파싱을 하니까 이 section 친구는 위로 가는 게 좋지 않을까요오?!

window.onload=initialPage();

// onload
function initialPage() {

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.

처음에 다 체크를 만들어놓는 이유는 무엇일깡?

}

.section__like:hover {
color: #E97BAC;
color: #e97bac;

Choose a reason for hiding this comment

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

리팩토링할 때 전역변수 사용해서 컬러 지정해보기!


// 카테고리 태그 만들기
function addCategoryTag(addId) {
let resultList = ``

Choose a reason for hiding this comment

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

요기서 빈값으로 먼저 선언해준 이유가 있으까요-? 그냥 categorySection 하단에서 바로 선언과 동시에 값 지정해도 될 것 같은데!

const checked = document.querySelectorAll(".category");
const btn = document.getElementById(`tag-btn-${addId}`);
btn.addEventListener('click', function() {
checked.forEach(function(checkbox) {

Choose a reason for hiding this comment

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

forEach 안에서 함수를 또 써준 이유가 있을까요?! 아예 전역 함수로 빼거나 아니면 함수 빼고 코드만 넣거나! 하면 더 깔끔할 것 같은데!

function deleteTagByBtn(addId) {
const checked = document.querySelectorAll(".category");
const btn = document.getElementById(`tag-btn-${addId}`);
btn.addEventListener('click', function() {

Choose a reason for hiding this comment

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

보통은 화살표 함수를 많이 쓰는 것 같아요

btn.addEventListener("click", ()=> {})

이렇게! 더 가독성이 좋은 것 같아!

})

// 모달 구현
function openModal(event) {

Choose a reason for hiding this comment

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

isModalOpen이라는 변수를 만들고 이 값에 따라서 모달을 열고 닫는 함수 하나만 만들면 openModal, closeModal 두 개의 함수가 필요하지 않을 것 같아!
한 번 이 방법으로도 구현해보기! 이 방법이 실제로 리액트에서 state를 활용해서 많이 사용하는 방법이라 미리 연습해보면 좋을 것 같아:)

Copy link
Member

Choose a reason for hiding this comment

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

아니면! css 로 미리 속성 지정해둔다음에 classList.toggle로 모달 구현해도 덴다!

Copy link

@urjimyu urjimyu left a comment

Choose a reason for hiding this comment

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

은빈~! 함수도 깔끔하게 분리한 게 보이구 네이밍도 좋구 창의적인 아이디어도 보여서 많이 배웠다!ㅎㅎ 처음부터 다시 해야된다고 하더니 너무 멋지게 뚝딱 했잖아?! 최고다 은빈!!

<p>쿠키</p>
</label>
<div class="nav__category">
<input type="checkbox" class="category" value="all" id="all"/>
Copy link

Choose a reason for hiding this comment

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

데이터셋 저도 배워가요👍🏻

@@ -0,0 +1,122 @@
import { breads } from "./products.js";

window.onload=initialPage();
Copy link

Choose a reason for hiding this comment

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

우와 이 생각은 못했는데 너무 좋은 아이디어다!!✨

Comment on lines +16 to +20
function filterBreadsByType(type) {
const filteredBreads = breads.filter((bread) => bread.type === type);
return filteredBreads.length > 0 ? filteredBreads : breads;
}

Copy link

Choose a reason for hiding this comment

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

내가 한참 헤맸던 필터링 정말 깔끔하게 함수 짰다! 보고 배웁니다ㅎㅎ 최고최고!

//카드들 삭제하기
function deleteCards(filterType) {
const articleList = document.querySelectorAll(`.${filterType}`);
articleList.forEach(article => article.parentNode.removeChild(article));
Copy link

Choose a reason for hiding this comment

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

오 이렇게 parentNode 쓰면 깔끔하게 제거가 되는군요!👍🏻

})

// 모달 구현
function openModal(event) {
Copy link
Member

Choose a reason for hiding this comment

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

아니면! css 로 미리 속성 지정해둔다음에 classList.toggle로 모달 구현해도 덴다!

window.onload=initialPage();

// onload
function initialPage() {
Copy link
Member

Choose a reason for hiding this comment

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

처음에 다 체크를 만들어놓는 이유는 무엇일깡?

Comment on lines 62 to 66
let resultList = ``
const categorySection = document.querySelector("#section__category");
resultList = `
<div id="tag-${addId}">${addId}<button class="typeTag" id="tag-btn-${addId}" type="button"> X</button></div>
`;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let resultList = ``
const categorySection = document.querySelector("#section__category");
resultList = `
<div id="tag-${addId}">${addId}<button class="typeTag" id="tag-btn-${addId}" type="button"> X</button></div>
`;
const categorySection = document.querySelector("#section__category");
const resultList = `
<div id="tag-${addId}">${addId}<button class="typeTag" id="tag-btn-${addId}" type="button"> X</button></div>
`;

로하면 let안써도 데지않나?!! 왜냠 js도 그렇고 리액트도 그렇고 let은 변경이 쉬워서 의도대로 작동하지 않을수도 있어서 최대한 지양하는게 조하!

// 배열 필터링
function filterBreadsByType(type) {
const filteredBreads = breads.filter((bread) => bread.type === type);
return filteredBreads.length > 0 ? filteredBreads : breads;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return filteredBreads.length > 0 ? filteredBreads : breads;
return filteredBreads.length ? filteredBreads : breads;

로만 해도 될것가튼데!

Comment on lines 5 to 17
const breadsList = breads.map((bread) => {
const hashtags = bread.hashtags
.map((tag) => `<li class="section__tag-list">${tag}</li>`)
.join(" ");
return `
<article class="section__card">
<h3>${bread.name}</h3>
<ul class="section__tag">${hashtags} </ul>
<img class="section__img" src="./imgs/${bread.src}"/>
<button class="section__like" id="btn${bread.id}" type="button">♥</button>
</article>
`;
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const breadsList = breads.map((bread) => {
const hashtags = bread.hashtags
.map((tag) => `<li class="section__tag-list">${tag}</li>`)
.join(" ");
return `
<article class="section__card">
<h3>${bread.name}</h3>
<ul class="section__tag">${hashtags} </ul>
<img class="section__img" src="./imgs/${bread.src}"/>
<button class="section__like" id="btn${bread.id}" type="button"></button>
</article>
`;
});
const breadsList = breads.map(({name, src, id, hashtags }) => `
<article class="section__card">
<h3>${name}</h3>
<ul class="section__tag">
${hashtags
.map((tag) => `<li class="section__tag-list">${tag}</li>`)
.join(" ")}
</ul>
<img class="section__img" src="./imgs/${src}"/>
<button class="section__like" id="btn${id}" type="button"></button>
</article>
`;
);

이렇게도 구조분해 할당해서 쓸 수 있을고야!

type: "breads",
src: "Bread1.png",
hashtags: ["바게트", "바게뜨?", "가로로 긴 바게뜨"],
like: false,
Copy link
Member

Choose a reason for hiding this comment

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

불린형 변수명 명명하는 방법 화긴해보기!

@eunbeann eunbeann self-assigned this May 26, 2023
@eunbeann eunbeann merged commit c212b21 into main Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants