Kdt0 jang ho jin #61

LeHiHo · 2023-08-18T14:59:09Z

직원 관리 시스템

배포주소 :https://frolicking-meerkat-ab332e.netlify.app/

직원등록

직원삭제

직원수정

userflow

netlify · 2023-08-18T14:59:19Z

✅ Deploy Preview for frolicking-meerkat-ab332e ready!

Name	Link
🔨 Latest commit	`f19169e`
🔍 Latest deploy log	https://app.netlify.com/sites/frolicking-meerkat-ab332e/deploys/64e5725d0fbc4f00088a6c80
😎 Deploy Preview	https://deploy-preview-61--frolicking-meerkat-ab332e.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

noSPkeepgoing

중간에 firebase로 다시 짜느라 힘드셨을텐데,, 필수 기능 모두 구현하신 거 대단해요! 고생 많이 하셨습니당✨👏

noSPkeepgoing · 2023-08-23T08:01:06Z

js/delete.js

+function toggleClassOnElement(element, className) {
+  element?.classList.toggle(className);
+}


오 엄청 다양한 곳에서 활용할 수 있게 함수로 묶으셨네요. 호진님의 세심함이 잘 느껴지는 부분인 거 같아요.✨

noSPkeepgoing · 2023-08-23T08:09:53Z

js/delete.js

+});
+
+document.querySelector(".staff-box__delete").addEventListener("click", async function () {
+  const selectedElements = document.querySelectorAll(".staff-box__item.active.select");


저라면 선택됐다는 것을 알 수 있는 클래스는 select니까, active는 제외하고 .staff-box_item.select이런 식으로 작성했을 거 같아요!

하핫.. 저도 그렇게 구현하려했는데 사실 수정모드일때 staff-box_item.modify가 되고 거기다가 선택했을땐 staff-box_item.modify.select로 했습니다.
멍청이 처럼 코드짜서
staff-box_item.select로 하면 수정모드랑 제거모드를 구별할수 없게되버립니다.ㅠㅠ

noSPkeepgoing · 2023-08-23T08:42:20Z

js/modify.js

+  modalBox.querySelector("input[name='employeeNumber']").value = data.employeeNumber;
+  modalBox.querySelector("input[name='userName']").value = data.userName;
+  modalBox.querySelector("select[name='dept']").value = data.dept;
+  modalBox.querySelector("select[name='position']").value = data.position;
+  modalBox.querySelector("input[name='userNumber']").value = data.userNumber;
+  modalBox.querySelector(".modal-box__inputImg img").src = data.imageUrl;


const {employeeNumber, userName, dept, position, userNumber, imageUrl } = data; 이런식으로 변수를 가져오거나,

modalBox.querySelectorAll('input') .forEach( el => { const name = el.name; el.value = data[name]; });

요런식으로하면 코드를 작성해보면 어떨까요?! 뭔가 충분히 코드를 줄일 수 있는 방법이 있을 거 같은데 좋은 생각이 딱히 안 떠오르네요,,🥲

noSPkeepgoing · 2023-08-23T08:52:04Z

js/modify.js

+      previouslySelected.classList.remove("select");
+    }
+
+    clickedItem.classList.add("select");


여기 부분인지는 확실치 않긴한데, 직원 관리에서 select했던 클래스가 유지되면서 직원삭제에서도 반영되더라구요!
사실 제가 선택된지 모르고 삭제해버리는 참사를 일으켜 버렸어요..ㅎ👀💦

예외처리 리팩토링 할때 좀더 세심히 해보겠습니다..

noSPkeepgoing · 2023-08-23T08:56:27Z

js/register.js

+  let file = document.querySelector("#image").files[0];
+  let storageRef = storage.ref();
+  let savePath = storageRef.child("image/" + file.name);
+  let uploadImage = savePath.put(file);


사용자가 빈칸으로 제출해서 한 번 오류가 나면 다시 재입력해도 아무런 동작을 하지 않더라구요! 값이 있는지 없는지 미리 체크하는 로직이 추가되면 더 좋을 거 같습니당

kyungkyuBae · 2023-08-23T09:15:50Z

저는 항상 사용자의 편의성에서 더 좋다거나 웹의 성능을 비교 했을때 드라마틱한 차이가 없을것같다면
항상 자신과 타협해서 쉬운 쪽으로만 코드를 짜려고하는데
호진님의 코드를 보면 난이도를 먼저 보지않고,
사용자의 편의성,코드의 가독성,코드의 성능 모두를 고려하셔서 어렵더라도 도전하시고 코드를 짜시는게 배워가고싶어요
이번 aws도 스터디원 모두 쉬운 firebase로 돌아갈때 aws로도 해보시고 firebase로도 하시는걸 보고 정말 대단하다고 느꼈습니다.
코드를 봐도 저는 의식의 흐름대로 코드르 짜고 냅두는 경우가 많은데 호진님 코드를 보면 가독성을 위해서
여러 문법들을 사용하셔서 코드의 숫자를 최대한 줄이고 가독성 좋게 하려고 하시는게 보이는 것 같아서 배워가겠습니다.

suehub

코드의 가독성에 항상 신경을 많이 쓰시는 것 같습니다. UI도 고민을 많이 하신 것이 보이고, 파이어베이스로 바꾸시느라 고생 많으셨습니다~👍
현재 웹에서 모달창의 높이가 맞지 않는데 이 부분과 반응형만 추가한다면 더 퀼리티있는 관리 페이지가 될 것 같아요!

suehub · 2023-08-23T12:52:47Z

js/delete.js

+      if (employeeData && employeeData.imageUrl) {
+        const imageUrl = employeeData.imageUrl;
+        const cleanUrl = imageUrl.split("?")[0];
+        const imageKey = cleanUrl.split("%2F").pop();
+        const imageKeyDecoded = decodeURIComponent(imageKey);
+
+        try {
+          await storage.child(`image/${imageKeyDecoded}`).delete();
+        } catch (error) {
+          console.error("Error processing imageUrl for employee:", id, error);
+        }


이 부분이 storage에 업로드된 이미지까지 삭제하기 위한 코드로 이해했는데 맞을까요??

suehub · 2023-08-23T12:58:39Z

js/modify.js

+}
+
+const toggleEmployeeModify = () => {
+  [...document.querySelectorAll(".staff-box__item")].forEach((item) => toggleClassOnElement(item, "modify"));


querySelectorAll 로 선택한 엘리멘트들을 spread operator로 바로 사용할 수 있게 작성하셨군요! 생각하지 못했는데 배워갑니다! 👍

suehub · 2023-08-23T13:09:52Z

js/modify.js

+    const snapshot = await db.collection("user").get();
+    snapshot.forEach((doc) => {
+      const staffElement = createStaffTemplate(doc.data(), doc);
+      staffBox.insertAdjacentHTML("beforeend", staffElement);


insertAdjacentHTML() 메소드를 처음 봤는데, '텍스트를 파싱하여 결과 노드들을 지정된 위치의 DOM 트리에 삽입'하는 메소드면 append()와 같은 역할을 하는 것으로 이해하면 될까요?

humnyenye · 2023-08-23T13:41:05Z

늘 다양한 시도를 해보시는 것 같아서 멋집니다. 고민하면서 코드를 짜는게 항상 보이니까 저도 열심히 해야지 하는 힘을 얻습니다.

marshallku

고생하셨습니다!

반복되는 로직을 묶고, 모듈화를 진행해보면 더욱 좋을 것 같습니다!

UX적으로는

직원 삭제 버튼을 누르면 다시 직원 삭제 버튼으로 취소할 수 있는 기능
직원 삭제 이전 확인
직원 클릭 시 해당 직원 정보만 확인 (모달 재사용)
정도가 있었으면 더 좋았을 것 같아요.

marshallku · 2023-08-26T10:28:11Z

index.html

+
+</head>
+
+<body masked>


class="masked"를 의도하셨던 건가요?

넵 저렇게 작성해도 동작하는거 같아서 넘어갔던것 같습니다.
해당 부분 수정했습니다!

marshallku · 2023-08-26T10:28:48Z

index.html

+      <div class="navbar__logo">
+        <i class="fa-solid fa-users-rectangle"></i>
+        <a href="">직원 관리 시스템</a>
+        <a href="#" class="navbar__tooglebtn">


여기는 <button>을 사용하는 게 좀 더 정확해 보입니다!

marshallku · 2023-08-26T10:29:21Z

index.html

+
+          </div>
+          <div class="modal-box__inputImg">
+            <img src="" alt="">


<img>를 모달이 보일 때 동적으로 만드는 게 낫지 않을까요?

marshallku · 2023-08-26T10:30:13Z

index.html

+            <div>
+              <div>
+                <label for="userName">이름</label>
+                <input type="text" name="userName" id="userName" placeholder="홍길동">


class 등 속성과 마찬가지로 id에도 kebab-case를 사용하는 게 일반적입니다.

marshallku · 2023-08-26T10:32:00Z

index.html

+              <div>
+                <label for="dept">부서</label>
+                <select name="dept" id="dept">
+                  <option value="인사">인사</option>


If no value attribute is included, the value defaults to the text contained inside the element

MDN을 참고해보시면, value 값은 기본적으로 element 내부 텍스트로 잡혀있습니다. 이 상황에는 value를 작성하지 않았어도 되지 않을까요?

앗 해당부분 수정하겠습니다!

marshallku · 2023-08-26T10:51:06Z

js/main.js

+function toggleClassOnElement(element, className) {
+  element?.classList.toggle(className);
+}


toggleClassOnElement를 여러 파일에서 구현해뒀는데, 한 군데만 구현하고 import하는 게 낫지 않을까요?

marshallku · 2023-08-26T10:54:39Z

js/modify.js

+  const clickedItem = event.target.closest(".staff-box__item.modify");
+
+  if (clickedItem && clickedItem.classList.contains("modify")) {
+    const previouslySelected = document.querySelector(".staff-box__item.modify.select");


clickedItem.classList.contains("select") 처럼도 작성할 수 있지 않을까요?

marshallku · 2023-08-26T10:56:47Z

js/modify.js

+  const updatedData = {};
+  formData.forEach((value, key) => (updatedData[key] = value));
+
+  let file = document.querySelector("#image").files[0];


재할당 되지 않는 변수로 보이네요~
추가로 getElementById를 사용해도 될 것 같습니다.

marshallku · 2023-08-26T10:57:44Z

js/modify.js

+        const imageUrl = employeeData.imageUrl;
+        const cleanUrl = imageUrl.split("?")[0];
+        const imageKey = cleanUrl.split("%2F").pop();
+        const imageKeyDecoded = decodeURIComponent(imageKey);
+        console.log(imageKeyDecoded);


이런 로직도 공통화했으면 좋았을 것 같네요!

marshallku · 2023-08-26T10:58:19Z

js/register.js

+const db = firebase.firestore();
+const storage = firebase.storage();
+
+$(".submitButton").click(function (event) {


이 함수에서만 갑자기 jQuery가 사용된 이유가 있을까요...?

LeHiHo added 3 commits August 18, 2023 09:48

first commit

335b596

first commit

a54d09c

add ReadMe.md

5bd5ea6

LeHiHo added 3 commits August 21, 2023 19:59

first commit

9b4b162

remove key

0ce25d6

add .env

f15b86c

LeHiHo force-pushed the KDT0_JangHoJin branch from 8571ef6 to f15b86c Compare August 21, 2023 12:24

LeHiHo added 4 commits August 21, 2023 21:29

add require

79e3945

fix bug

f1ad678

fix bug

3c7436a

fix bug

0c5d370

LeHiHo force-pushed the KDT0_JangHoJin branch from 49ccbba to 0c5d370 Compare August 23, 2023 01:14

LeHiHo added 2 commits August 23, 2023 11:38

fix bug

63caa6e

fix readme.md

f19169e

noSPkeepgoing reviewed Aug 23, 2023

View reviewed changes

suehub reviewed Aug 23, 2023

View reviewed changes

marshallku reviewed Aug 26, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kdt0 jang ho jin #61

Kdt0 jang ho jin #61

LeHiHo commented Aug 18, 2023 •

edited

Loading

netlify bot commented Aug 18, 2023 •

edited

Loading

noSPkeepgoing left a comment

noSPkeepgoing Aug 23, 2023

noSPkeepgoing Aug 23, 2023

LeHiHo Aug 23, 2023

noSPkeepgoing Aug 23, 2023 •

edited

Loading

noSPkeepgoing Aug 23, 2023

LeHiHo Aug 23, 2023

noSPkeepgoing Aug 23, 2023

kyungkyuBae commented Aug 23, 2023

suehub left a comment

suehub Aug 23, 2023

suehub Aug 23, 2023

suehub Aug 23, 2023

humnyenye commented Aug 23, 2023

marshallku left a comment

marshallku Aug 26, 2023

LeHiHo Aug 28, 2023

marshallku Aug 26, 2023

marshallku Aug 26, 2023

marshallku Aug 26, 2023

marshallku Aug 26, 2023

LeHiHo Aug 28, 2023

marshallku Aug 26, 2023

marshallku Aug 26, 2023

marshallku Aug 26, 2023

marshallku Aug 26, 2023

marshallku Aug 26, 2023


		</head>

		<body masked>

Kdt0 jang ho jin #61

Are you sure you want to change the base?

Kdt0 jang ho jin #61

Conversation

LeHiHo commented Aug 18, 2023 • edited Loading

직원 관리 시스템

직원등록

직원삭제

직원수정

userflow

netlify bot commented Aug 18, 2023 • edited Loading

✅ Deploy Preview for frolicking-meerkat-ab332e ready!

noSPkeepgoing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noSPkeepgoing Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyungkyuBae commented Aug 23, 2023

suehub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humnyenye commented Aug 23, 2023

marshallku left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeHiHo commented Aug 18, 2023 •

edited

Loading

netlify bot commented Aug 18, 2023 •

edited

Loading

noSPkeepgoing Aug 23, 2023 •

edited

Loading