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

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

Conversation

chaeneey
Copy link
Contributor

@chaeneey chaeneey commented Oct 29, 2024

✨ 구현 기능 명세

💡 기본 과제

  • 바닐라 JavaScript 사용. (React 등 라이브러리 사용 X)
  1. 데이터

    • 초기 데이터는 데이터 파일을 별도로 생성해서 저장
    • 이 데이터를 불러와서 localStorage에 저장(이미 localStorage에 값이 있으면 안 불러옴).
    • localStorage 데이터를 이용해서 테이블을 렌더링(HTML에 데이터를 직접 넣는 하드코딩 x)
  2. 헤더

    • 왼쪽엔 아이콘 1개 (종류는 상관 x)
    • 가운데에는 타이틀(아무거나)
    • 오른쪽에는 긴 컨텐츠(텍스트, 아이콘 등 아무거나)
    • 좌측/우측 컨텐츠의 길이가 달라도 헤더가 정확히 3등분 되어 정렬되어야 함.
  3. 필터

    • 7개의 검색필터, 2개의 버튼으로 구성
    • 검색필터를 배치할때는 반드시 display: grid 속성 사용
    • 속성이 4줄에 각각 1개, 1개, 3개, 2개 배치 되어야 함(속성의 내용은 상관 x)
    • 필터링 할 값을 input에 입력
      (기본과제에서는 7개 모두 input으로 구현해도 괜찮음)
    • 7개의 속성 모두 필터링이 동작해야 함(포함하는 값으로 필터링)
    • (기본) 1번에 1개씩 입력하고 검색 필터 적용
    • 우측 상단에는 선택삭제, 추가 버튼이 위치
    • 상단 필터에서 필터링 된 값만 나타남
    • 반드시 HTML의 table 태그를 사용해서 구현
    • 체크박스 기능
    • 선택삭제 버튼 클릭 시, 체크된 항목들 삭제
    • 추가 버튼 클릭 시, 항목 추가 모달 등장
  4. 모달

    • 필터링, 테이블에 사용되는 7개의 항목을 입력 가능
    • 우측 상단에 모달 닫기 버튼
    • 하단에 추가 버튼 클릭 시, 모달 닫히면 데이터 추가
    • 1개라도 비어있는 상태로 추가 시, alert 창 띄워주기

🔥 심화 과제

  1. 필터

    • 필터에 dropdown을 1개 이상 사용
    • 초기화 버튼을 누르면 모든 필드가 비워짐
    • (심화) 여러 조건들을 모두 만족하도록(and) 검색 필터 적용
    • 한 개 이상의 열에는, 눌렀을 때 해당 칸의 값을 이용해서 링크 이동
    • 전체 체크박스 기능 (전체가 체크되면 맨위 체크박스도 자동으로 체크, 하나라도 해제되면 같이 해제)
  2. 모달

    • 백드롭(모달 주변 어두운 배경 부분) 클릭 시 모달 닫힘

🌐 공유과제


❗️ 내가 새로 알게 된 점

  • 모달의 기능을 쉽게 구현할 수 있는 태그가 있다는 걸 처음 알았습니다. 처음엔 div를 이용해서 모달을 부르는 버튼을 누르면 display 스타일을 flex로, 모달 닫는 버튼을 누르면 display 스타일을 none으로.. 구현했었는데요, 모달 백드롭 관련된 부분을 검색하다가 태그를 발견했는데 !!.. showModal()과 close()로 아주 간편하게 모달 형식을 만들 수 있다는 점이 정말 매력적이었습니다 ... 심지어 position fixed .. top: 1rem ~~ 뭐 이런 다른 스타일링을 주지 않았는데 알아서 가운데에 우뚝 있는 모습이 참 좋더라구요 .. 근데 또 리액트를 사용할 땐 이 태그보다 useState 쓰는 게 더 좋다고 하는 글을 봐서, 리액트 땐 기존대로 하는 방식을 써야겠어욥

  • 체크박스 전체 선택은 그래도 몇 번 해봤다고 ,, 괜찮았는데 전체 체크박스 말고 아래의 다른 체크박스를 클릭했을 때 맨 위의 전체 체크박스가 변경되어야 하는 움직임은 고민을 많이 한 것 같습니다!.. every 배열 메소드를 이용해서 모든 체크박스의 체크 상태를 살펴보고, 그 결과에 따라 전체 체크박스가 반영되도록 구현했습니다!

  // 체크박스 개별선택
  filterCheckboxAll.forEach((checkbox) => {
    checkbox.addEventListener("click", () => {
      headerCheckbox.checked = [...filterCheckboxAll].every(
        (checkbox) => checkbox.checked
      );
    });
  });
  • 모달 백드롭 기능도 이번에 새롭게 알게 되었습니다! 모달을 기준으로 그 모달의 양 옆 위 아래를 감지해서, 모달을 제외한 배경을 클릭할 경우 close 시키는 과정이 신기했습니다! 이 과정에서 사용한 친구가 바로 getBoundingClientRect()인데, 엘리먼트의 크기와 뷰포트에 상대적인 위치를 제공하는 메서드라는 점을 새롭게 알게 되었습니다!
    https://developer.mozilla.org/ko/docs/Web/API/Element/getBoundingClientRect
  // 모달 백드롭
  addMemberModal.addEventListener("click", (event) => {
    const target = event.target;
    const rect = target.getBoundingClientRect();
    if (
      rect.left > event.clientX ||
      rect.right < event.clientX ||
      rect.top > event.clientY ||
      rect.bottom < event.clientY
    ) {
      closeModal();
    }
  });
};

❓ 구현 과정에서의 어려웠던/고민했던 부분

1️⃣ 멤버를 삭제하는 과정에서 어려웠던 부분이 있습니다! 제가 생각했던 흐름은 checked 상태인 요소들의 id값을 담아서, 그 id값과 같지 않은 친구들, 즉, 체크되지 않은 요소들만 updateMemberList로 담아 다시 화면에 보여주는 것이죠! 그리고 updateMemberList를 이용해 테이블 화면을 렌더해주고, 로컬스토리지에 해당 값을 반영하는 형식으로 생각했습니다.

// 멤버 삭제
export const deleteMember = () => {
  const deleteMemberIds = [];
  const prevMemberList = getLocalStorageMembersData();
  const filterCheckboxAll = document.querySelectorAll(".filter-checkbox");

  // checed된 요소들의 id 담기
  filterCheckboxAll.forEach((checkbox) => {
    if (checkbox.checked) {
      deleteMemberIds.push(Number(checkbox.parentNode.parentNode.id));
    }
  });

  // 같은 id 값을 가지지 않은 친구들만 filter
  const updateMemberList = prevMemberList.filter(
    (member) => !deleteMemberIds.includes(member.id)
  );

  return updateMemberList;
};

근데 여기서 고민되었던 부분은 checked된 요소들을 어떻게 알 수 있을지? 였습니다. 테이블의 한 행을 삭제해야 하기 때문에, checked된 해당 체크박스가 들어있는 행을 찾아야 했고, 그 과정에서 부모 노드를 선택할 수 있는 parentNode를 이용해 그 행의 id값을 가져오는 형식으로 구현했습니다. 잘 작동하긴 하지만, parentNode를 두 번 사용하기도 하고, 뭔가 더 간단한 방법이 있을 것 같기도 한 그런 느낌 ... 이 듭니다! 다른 분들은 이런 삭제 과정을 어떻게 고민하고 구현하셨는지 궁금합니다!

2️⃣ 처음에 로컬스토리지에서 membersData를 가져와 const getLocalStorageMembersData 에 담고, 이 getLocalStorageMembersData를 이용해 filter를 구현했더니, 멤버가 삭제되거나 추가된 후에도 새롭게 반영된 로컬스토리지의 membersData를 가져오지 않고 맨 처음에 가져온 membersData로 필터링이 되고 있음을 확인했습니다. (바보이슈죠 히히) 이 부분을 해결하기 위해, 변수가 아닌 함수의 형태로 변경하는 방식으로 수정하고 보니, 처음에 이 데이터 렌더링을 다들 어떻게 관리하셨을지 갑자기 궁금해졌습니다,, 데이터도 그렇고 처음 표로 쫘악 보여야 하는 것도 그렇고 신경쓸 부분이 많더라고요 ..! 이 부분들을 깔끔하게 잘 표현하는 방법을 모르겠어서, 다른 분들의 코드를 많이 참고하고 싶습니다!

// 기존 데이터 렌더링
const getLocalStorageMembersData = JSON.parse(localStorage.getItem("membersData"));


// 수정된 데이터 렌더링
export const getLocalStorageMembersData = () => {
  return JSON.parse(localStorage.getItem("membersData"));
};

🥲 소요 시간

  • 3일

🖼️ 구현 결과물

과제구현영상

스크린샷 2024-10-29 오후 4 51 09

Comment on lines +1 to +14
import {
deleteMember,
addMember,
validateInput,
} from "./utils/editMemberList.js";
import { getLocalStorageMembersData, renderMemberList } from "./utils/renderMemberList.js";
import {
initializeFilterValue,
initializeCheckbox,
} from "./utils/initialize.js";
import { filterMemberList } from "./utils/filterMemberList.js";
import { setupCheckbox } from "./utils/setupCheckbox.js";
import { members } from "./data/memberData.js";
import { closeModal, setupModal } from "./modal.js";
Copy link
Member

Choose a reason for hiding this comment

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

코드 전체가 utils 폴더 안에서 각 기능별로 세부적으로 모듈화되어 있는 게 너무 좋은 거 같아요! 저는 아무 생각 없이 한 페이지에 기능을 몰아 넣었는데 그랬더니 가독성도 떨어지고 코드가 너무 지저분해지더라구요...!!!

특히 deleteMember, addMember, filterMemberList 같은 주요 기능들이 각각의 모듈에 깔끔하게 정의되어 있으니까 코드를 이해하는 데도 훨씬 도움이 돼요! 제 코드에도 이런 식으로 모듈화를 적용해서 깔끔하게도 만들고 유지보수 측면에서도 좋게끔 이런 구조로 개선해야겠네요...ㅎㅎ

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 +24 to +35
// 모달 백드롭
addMemberModal.addEventListener("click", (event) => {
const target = event.target;
const rect = target.getBoundingClientRect();
if (
rect.left > event.clientX ||
rect.right < event.clientX ||
rect.top > event.clientY ||
rect.bottom < event.clientY
) {
closeModal();
}
Copy link
Member

Choose a reason for hiding this comment

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

저는 그냥 단순하게 클릭된 요소가 백드롭일 때 closeModal()을 호출하게만 했는데 이런 방식이 있는 건 생각치도 못했어요!
getBoundingClientRect() 이 함수를 통해 요소의 위치를 알아낼 수가 있네요.... 이렇게 하면 모달 외부와 내부 클릭을 더 세밀하게 구분할 수 있겠어요! 새로운 거 또 알아갑니다!

Comment on lines +3 to +19
// 멤버 삭제
export const deleteMember = () => {
const deleteMemberIds = [];
const prevMemberList = getLocalStorageMembersData();
const filterCheckboxAll = document.querySelectorAll(".filter-checkbox");

// checed된 요소들의 id 담기
filterCheckboxAll.forEach((checkbox) => {
if (checkbox.checked) {
deleteMemberIds.push(Number(checkbox.parentNode.parentNode.id));
}
});

// 같은 id 값을 가지지 않은 친구들만 filter
const updateMemberList = prevMemberList.filter(
(member) => !deleteMemberIds.includes(member.id)
);
Copy link
Member

Choose a reason for hiding this comment

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

체크된 멤버의 id를 배열로 모아두고 필터링하는 방식이 있었네요 저는 이렇게 따로 배열에 담아둔 다음에 필터링을 거치는 생각을 못 했었는데 deleteMemberIds처럼 배열에 먼저 담아두면 코드가 훨씬 정돈되고 이해하기 쉬워지는 것 같아요! 이것도 참고해서 적용해봐야겠어요 ㅎㅎ...

Comment on lines +45 to +63
export const addMember = () => {
const prevMemberList = getLocalStorageMembersData();

const newMember = {
id: prevMemberList.length + 1,
name: modalInputName.value,
englishName: modalInputEngName.value,
github: modalInputGithub.value,
gender: modalInputGender.value,
role: modalInputRole.value,
firstWeekGroup: Number(modalInputWeek1.value),
secondWeekGroup: Number(modalInputWeek2.value),
};

const newMemberList = [...prevMemberList];
newMemberList.push(newMember);

return newMemberList;
};
Copy link
Member

Choose a reason for hiding this comment

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

기존 멤버 리스트를 불러와서 스프레드 연산자로 새로운 배열을 만드는 방식도 너무 좋은 것 같아요! 사실 저는 이 코드 보면서 ... 이 연선자에 대한 개념을 다시 배웠어요 ㅎㅎ 거기에 새 멤버를 추가하는 방식이 너무 깔끔해요! 저는 그냥 기존 배열에 추가해버렸었는데 이렇게 새로운 배열을 만들어서 해도 되겠네요... 최고!!!

Copy link

@look-back-luca look-back-luca left a comment

Choose a reason for hiding this comment

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

안녕하세요 채연님
이번에 자바스크립트와 DOM을 처음 배워보는 입장에서 봤을 때
채연님 코드와 파일 정리하는 부분이 정말 본받고 싶었어요

각각 폴더를 만들고 전체 코드에서는 함수를 불러오는 게 정말 전체적인 관점에서 봤을 때는 너무 편리하고 재사용성도 높은 것 같아요. 하나의 파일 안에다가 js 코드로 모든 기능을 구현하려고 하니까 계속 위에 부분을 다시 읽어서 내려오는 일을 많이 했는데, 각각의 기능별로 따로따로 파일을 만들어서 정리하면 확실히 작업할 때도 편리할 것 같아요. 새로운 메서드도 덕분에 알게 되었어요 ! 감사해요

그리고 초콜릿도 너무 잘 먹었어요 !!! 진짜 맛있었어요.
과제 하시느라 고생많으셨어요

</section>
</main>

<dialog class="add-member-modal">

Choose a reason for hiding this comment

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

이런 태그는 처음 봤어요 모달 구현하기 위해 사용하는군요
근데 찾아보니까 리액트에서는 div 태그를 더 추천한다고 하네요..??
그래도 자바스크립트 코드 작성할 때 쓰기 좋은 것 같아요 덕분에 새롭게 알아갑니다!!!

<i class="fa-solid fa-x modal-close-button"></i>
</section>

<from class="modal-content-wrapper">

Choose a reason for hiding this comment

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

from 태그라는 것도 있는건가요..?? 찾아봐도 안 나와서
어떤 기능은 하는건지 궁금해요

Choose a reason for hiding this comment

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

from 태그가 어떤걸까요?? form을 잘못입력하신걸까요..?

Choose a reason for hiding this comment

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

  • form 태그 사용 이유도 궁금합니다!

@@ -0,0 +1,63 @@
import { getLocalStorageMembersData } from "./renderMemberList.js";

Choose a reason for hiding this comment

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

이렇게 자바스크립트 파일만 따로 폴더로 나눠놓기도 하는군요 ..!
역시 OB는 다르네요 짱.

: true;

const isFirstWeekGroupFiltered = filterInputWeek1.value
? firstWeekGroup === Number(filterInputWeek1.value)

Choose a reason for hiding this comment

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

이렇게 변수 타입도 다 신경쓰시는 것 멋있네요.

justify-content: center;
align-items: center;
width: 100%;
background-color: var(--lightgray);
Copy link

Choose a reason for hiding this comment

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

CSS 색상 적용을 이렇게도 할 수 있다는 걸 처음 알게됐어요..! 미리 :root에 변수를 정의해두고, 모든 요소에서 --lightgray 변수를 사용할 수 있도록 하는 방법이네요! 다음엔 저도 이렇게 한 번 해봐야겠어요 😲

padding: 1rem;
border-radius: 5px;
border: none;
width: calc(100% - 7rem);
Copy link

Choose a reason for hiding this comment

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

처음보는 형태라 찾아보니 CSS의 calc() 함수를 사용하여 동적으로 너비를 계산하는 구문이더라구요. 고정된 사이드바나 여백을 제외한 영역의 너비를 설정할 때 유용할 것 같아요!! 코드 볼 때 마다 처음 보는 구문들이 많아 계속 놀라는 중입니다..저도 얼른 다양한 구문을 익힐 수 있길,,✨

Copy link

@Rinakk Rinakk 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 +8 to +35
// 멤버 리스트 렌더링
export const renderMemberList = (data) => {
const memberList = document.querySelector(".table-body");
memberList.textContent = "";
const memberListData = data.map((member) => {
return `
<tr class="tr" id="${member.id}">
<td><input type="checkbox" class="filter-checkbox" /></td>
<td>${member.name}</td>
<td>${member.englishName}</td>
<td>
<a
href="https://github.com/${member.github}"
target="_blank">
${member.github}
</a>
</td>
<td>${member.gender === "male" ? "남자" : "여자"}</td>
<td>${member.role}</td>
<td>${member.firstWeekGroup}</td>
<td>${member.secondWeekGroup}</td>
</tr>
`;
});

memberList.innerHTML += memberListData.join("");
setupCheckbox();
};

Choose a reason for hiding this comment

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

멤버 리스트를 렌더링하는 함수가 과연 유틸에 분리되는 것이 맞을지에 대한 고민이 필요해보입니다 ㅎㅎ!

Comment on lines +21 to +23
renderMemberList(getLocalStorageMembersData());
setupCheckbox();
setupModal();

Choose a reason for hiding this comment

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

renderMemberList라는 함수에서 이미 setupCheckbox를 호출하고 있는데, 여기서 또 호출하신 이유가 있으신가요?

Comment on lines +1 to +8
// 검색필터 초기화
const filterInputName = document.querySelector(".filter-input-name");
const filterInputEngName = document.querySelector(".filter-input-engname");
const filterInputGithub = document.querySelector(".filter-input-github");
const filterInputGender = document.querySelector(".filter-input-gender");
const filterInputRole = document.querySelector(".filter-input-role");
const filterInputWeek1 = document.querySelector(".filter-input-week1");
const filterInputWeek2 = document.querySelector(".filter-input-week2");

Choose a reason for hiding this comment

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

export function qs(selector, scope = document) {
  if (!selector) throw "no selector";

  return scope.querySelector(selector);
}

이렇게 한 번 정의해놓고 가져다 쓰면 더 편리하겠죠?

Comment on lines +8 to +16
export const openModal = () => {
initializeModalValue();
addMemberModal.showModal();
};

export const closeModal = () => {
addMemberModal.close();
initializeModalValue();
};

Choose a reason for hiding this comment

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

과제 요구사항이 정확히 뭔지는 모르겠으나, 모달을 열 때와 닫을 때 초기화해주는 이유가 궁금합니다!

Comment on lines +25 to +36
// 데이터 필터링
const filterSearchBtn = document.querySelector(".filter-search-button");
filterSearchBtn.addEventListener("click", () => {
renderMemberList(filterMemberList());
});

// 필터 초기화
const filterResetBtn = document.querySelector(".filter-reset-button");
filterResetBtn.addEventListener("click", () => {
initializeFilterValue();
renderMemberList(getLocalStorageMembersData());
});

Choose a reason for hiding this comment

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

이런 친구들 따로따로 전부 함수로 만든다면 가독성이 더 좋아질 수 있겠네용
쿼리셀렉터들도 이 기능만을 위해 사용된다면 안으로 집어넣어도 될 것 같구여!

export const deleteMember = () => {
const deleteMemberIds = [];
const prevMemberList = getLocalStorageMembersData();
const filterCheckboxAll = document.querySelectorAll(".filter-checkbox");

Choose a reason for hiding this comment

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

이 querySelectorAll도 위에 리뷰처럼 따로 헬퍼함수로 만든다면 어떨지 고민해보시길 ㅎㅎ

Comment on lines +3 to +22
// 멤버 삭제
export const deleteMember = () => {
const deleteMemberIds = [];
const prevMemberList = getLocalStorageMembersData();
const filterCheckboxAll = document.querySelectorAll(".filter-checkbox");

// checed된 요소들의 id 담기
filterCheckboxAll.forEach((checkbox) => {
if (checkbox.checked) {
deleteMemberIds.push(Number(checkbox.parentNode.parentNode.id));
}
});

// 같은 id 값을 가지지 않은 친구들만 filter
const updateMemberList = prevMemberList.filter(
(member) => !deleteMemberIds.includes(member.id)
);

return updateMemberList;
};

Choose a reason for hiding this comment

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

한 함수에서 너무 많은 역할을 하고 있어요!
하ㅏㅁ수 이름이 deleteMember면 멤버를 삭제하는 역할만 하는 것이 어떨까요?
현재는 주석처럼(주석에도 오타가 있네용)
check된 요소들의 id를 담고 + id가 없는 애들만 filter한다.
두 가지 역할을 하는 것처럼 보입니다!
어떻게 분리하면 좋을지 고민해보시길!

Comment on lines +24 to +31
// 멤버 추가
const modalInputName = document.querySelector(".modal-input-name");
const modalInputEngName = document.querySelector(".modal-input-engname");
const modalInputGithub = document.querySelector(".modal-input-github");
const modalInputGender = document.querySelector(".modal-input-gender");
const modalInputRole = document.querySelector(".modal-input-role");
const modalInputWeek1 = document.querySelector(".modal-input-week1");
const modalInputWeek2 = document.querySelector(".modal-input-week2");

Choose a reason for hiding this comment

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

반복되는 querySelector를 작성하면서,
어라? 더 편한 방법은 없을까?
고민해보셨다면 더 좋았을 것 같슴다 ㅎㅎ!

Comment on lines +33 to +57
export const validateInput = () => {
return modalInputName.value &&
modalInputEngName.value &&
modalInputGithub.value &&
modalInputGender.value &&
modalInputRole.value &&
modalInputWeek1.value &&
modalInputWeek2.value
? true
: false;
};

export const addMember = () => {
const prevMemberList = getLocalStorageMembersData();

const newMember = {
id: prevMemberList.length + 1,
name: modalInputName.value,
englishName: modalInputEngName.value,
github: modalInputGithub.value,
gender: modalInputGender.value,
role: modalInputRole.value,
firstWeekGroup: Number(modalInputWeek1.value),
secondWeekGroup: Number(modalInputWeek2.value),
};

Choose a reason for hiding this comment

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

value를 가져오는 구조가 반복되고 있네요!

const getInputValues = () => ({
  name: document.querySelector(".modal-input-name").value,
  englishName: document.querySelector(".modal-input-engname").value,
  github: document.querySelector(".modal-input-github").value,
  gender: document.querySelector(".modal-input-gender").value,
  role: document.querySelector(".modal-input-role").value,
  firstWeekGroup: document.querySelector(".modal-input-week1").value,
  secondWeekGroup: document.querySelector(".modal-input-week2").value,
});

이렇게 한 번에 가져온다면 어떨까요?
그렇다면 validateInput도 구조가 바뀌겠네요.
Object.values() 를 사용해서 유효성 검증하는 부분 리팩토링 해보시길!

Comment on lines +39 to +58
const deleteMemberBtn = document.querySelector(".member-delete-button");
deleteMemberBtn.addEventListener("click", () => {
const updateMemberList = deleteMember();
localStorage.setItem("membersData", JSON.stringify(updateMemberList));
renderMemberList(updateMemberList);
initializeCheckbox();
});

// 멤버 추가
const addMemberBtn = document.querySelector(".add-member-button");
addMemberBtn.addEventListener("click", () => {
if (validateInput()) {
const newMemberList = addMember();
localStorage.setItem("membersData", JSON.stringify(newMemberList));
renderMemberList(newMemberList);
closeModal();
} else {
alert("모든 값을 입력하세요.");
}
});

Choose a reason for hiding this comment

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

memberList는 어떤 역할을 하는 파일인가요?
초기설정도 하고 있고, 각각 이벤트들도 달아주고 있네요!
내부 로직은 전부 util로 분리하셔서 추상화 레벨을 맞추신 점 아주 좋아요!
그러나, 이벤트를 등록하는 부분도 전부 따로 분리하신다면 더욱 깔끔하지 않을까하는 아쉬움이 있어요 ㅠ
파일명도 아쉬운 점이 있습니다!
memberList 라면, 멤버리스트를 렌더링하겠다는 뜻으로 받아들여지는데, 그렇다면 renderMemberList는 어떨지!
또한 이 memberList에서 초기설정까지 하는게 맞을지! 고민해보시길 ㅎㅎ

Comment on lines +1 to +20
export const setupCheckbox = () => {
const headerCheckbox = document.querySelector(".table-header-checkbox");
const filterCheckboxAll = document.querySelectorAll(".filter-checkbox");

// 체크박스 전체선택
headerCheckbox.addEventListener("click", () => {
filterCheckboxAll.forEach((checkbox) => {
checkbox.checked = headerCheckbox.checked;
});
});

// 체크박스 개별선택
filterCheckboxAll.forEach((checkbox) => {
checkbox.addEventListener("click", () => {
headerCheckbox.checked = [...filterCheckboxAll].every(
(checkbox) => checkbox.checked
);
});
});
};

Choose a reason for hiding this comment

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

여기서도 체크박스 전체선택과 개별선택을 따로 분리한 다음, setupCheckbox 안에 분리된 함수를 담아준다면
그리고 기깔난 네이밍을 지어준다면, 함수도 깔끔해질 뿐 아니라 주석으로 설명하지 않으셔도 되는 이점도 있을 것 같습니다!

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