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

KDT0_JangYeongMin 직원 관리 서비스 #65

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yeongmins
Copy link

@yeongmins yeongmins commented Aug 18, 2023


userflow


🏢 직원 관리 서비스 🏢

🔗 프로젝트 URL


📌 필수 구현 사항

  • “AWS S3 / Firebase 같은 서비스”를 이용하여 사진을 관리할 수 있는 페이지를 구현
  • 스크롤이 가능한 형태의 리스팅 페이지를 개발
  • 전체 페이지 데스크탑-모바일 반응형 페이지를 개발
  • 사진을 등록, 수정, 삭제가 가능
  • 유저 플로우를 제작하여 리드미에 추가
  • css 상대수치 사용
  • JavaScript DOM event 조작

📌 선택 구현 사항

  • 직원을 등록, 수정, 삭제가 가능
  • 직원 검색 기능을 추가

🗓️ 개발 기간

2023.08.08 ~ 2023.08.18


🔨 사용 기술 스택


🎨 Design UI

logo

UI URL : Adobe XD


✏️ userflow

userflow


📝 아쉬운점 & 느낀점

  • 파이어베이스를 처음 사용했는데 연동이 너무 어려워서 DB 구현을 못했습니다.

  • 반응형을 주기 위해 css 코드를 너무 비효율 적으로 작성했습니다.

⌨️ 추후 구현사항

  • 파이어베이스 DB 연동

  • 상세 프로필 구현

  • 깔끔한 모바일 페이지 구현

@yeongmins yeongmins self-assigned this Aug 18, 2023
@HOOOO98
Copy link

HOOOO98 commented Aug 22, 2023

전체적으로 기능 구현을 잘 하셨네요!!👍😊
UI도 감탄이 나올 정도로 깔끔해서 실제로 hr에서 쓸 거 같아요!
이후에 리팩토링에는 DB랑 storage 연결하시면 되겠네요~!
저는 업로드 후 불러올때 모달과 원래 이미지를 연결시켜주기 위해 리스트 속성에 data-id를 새로 만들어서 부여했습니다.
콜렉션 하위 문서의 랜덤한 ID로 동적 부여했습니다!
영민님은 처음에 어떻게 시도하셨었는지 궁금하네요~!

@yeongmins
Copy link
Author

@HOOOO98 감사합니다! 저는 UI 게속 보니까 조금 아쉬운 것 같기도 하네요 ㅎㅎ,,
저는 input 태그에 전부 id값을 주고 입력 한 값을 .value 를 사용해서 값을 가져온 후 innerHTML 에 span 태그로 값을 넣었습니다!

@JiHongkyu
Copy link

확실히 UI랑 동작하는 게 깔끔하네요! 대단하십니다!
리팩토링 때 DB연동까지 구현하시면 완벽할 것 같습니다!

@IAMISTP
Copy link

IAMISTP commented Aug 25, 2023

UI 동작하는게 매우 부드럽고 깔끔합니다!!
사용자가 사용하는데 있어 이질감 없이 느껴질거 같아 너무 좋네요 ! 저도 이번 리팩토링때 이렇게 반응형으로 부드럽게 동작할수 있도록 수정을 해봐야겠네요!

@NamgungJongMin
Copy link

영민님 사이트의 디자인은 봐도 봐도 멋지네요. 그저 정적인 상태일 때 뿐만아니라 기능들이 동작할 때 보여지는 UI들도 너무 이뻐서 마치 디자이너를 따로 두신줄 알았어요.

@suyeonnnnnnn
Copy link

기대하고 있었는데 UI 깔끔하고 멋있어요~! 검색, 체크박스등 여러 기능들도 잘 구현하셨네요! 잘 보고 갑니다~! 👍👍

Copy link

@jungHyeonS jungHyeonS left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 영민님~
파이어베이스 활용도 적절히 해주셨고 코드도 깔끔하게 작성해주신거같습니다~
추후에는 이벤트 리스너 와 자바스크립트 내장 메소드 활용에 대해서 고민해주시면 좋을꺼같습니다~



// "mypage" 버튼 클릭 시 이벤트 리스너
mypageButton.addEventListener("click", () => {

Choose a reason for hiding this comment

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

이벤트 리스너에 함수가 조금 복잡한거 같습니다 이럴때는 함수를 분리하는부분을 한번 고민해주시면 좋습니다~
예를들어 모달닫기, 모달 열기는 별도의 함수로 분리할수있을꺼같습니다~


const mypageRegisterButton = document.getElementById("mypage_register");
// "mypage_register" 버튼 클릭 시 이벤트 리스너
mypageRegisterButton.addEventListener("click", () => {

Choose a reason for hiding this comment

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

mypageButton 버튼을 클릭할떄마다 mypageRegisterButton 에 이벤트 리스너가 추가될고있습니다
이러면 새로운 이벤트 리스너가 계속 추가되는구조라서 메모리 누수가 발생할수도있습니다 따라서 이벤트 리스너를 분리해주시거나
removeEventListener 를 한번 호출해주시는게 좋습니다

Comment on lines +75 to +76
registerModal.classList.remove("show");
body.classList.remove("modal-open");

Choose a reason for hiding this comment

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

모달 관련된 코드가 중복되어보입니다 이러한 중복된 코드들은 별도의 함수를 만드시는것을 추천드립니다~

Comment on lines +48 to +60
const itemWrap = document.createElement("button");
itemWrap.classList.add("item_wrap");
itemWrap.id = "item_wrap_button";

// 이미지를 감싸는 div 생성
const itemImg = document.createElement("div");
itemImg.classList.add("item_img");

// 이미지 요소 생성
const profileImage = document.createElement("img");
profileImage.src = imgURL;
profileImage.alt = "프로필 이미지";
itemImg.appendChild(profileImage);

Choose a reason for hiding this comment

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

특정 엘리먼트를 만드는 로직이 중복되어보입니다 이러한 요소들을 한번에 처리하는 함수를 아래처럼 만들수있을꺼같습니다~

function createElement(tag, classNames = [], attributes = {}) {
   const elem = document.createElement(tag);
   classNames.forEach(className => elem.classList.add(className));

   for (let attr in attributes) {
      elem[attr] = attributes[attr];
   }

   return elem;
}

Comment on lines +16 to +28
spans.forEach((span) => {
const text = span.textContent.toLowerCase();
if (text.includes(searchTerm)) {
shouldShow = true; // 검색어가 포함된 span이 있으면 표시할 플래그를 true로 설정
}
});

// 아이템을 표시하거나 숨김 처리
if (shouldShow) {
item.style.display = "flex"; // 표시
} else {
item.style.display = "none"; // 숨김
}

Choose a reason for hiding this comment

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

shouldShow 변수를 설정할때 some() 메소드를 사용해주시면 더 좋습니다, some 을 사용하면 아래와 같은 코드를 작성할수있습니다~

    const shouldShow = Array.from(spans).some((span) => {
      return span.textContent.toLowerCase().includes(searchTerm);
    });

    // 아이템을 표시하거나 숨김 처리
    item.style.display = shouldShow ? "flex" : "none";

Comment on lines +3 to +14
var name = document.getElementById("name_ID").value;
var email = document.getElementById("email_ID").value;
var phone = document.getElementById("phone_ID").value;
var rank = document.getElementById("rank_ID").value;

// 이미지 파일 선택
var fileInput = document.getElementById("file_input");
var imageFile = fileInput.files[0];

// Firebase Storage 참조
var storageRef = firebase.storage().ref();
var imageRef = storageRef.child("images/" + imageFile.name);

Choose a reason for hiding this comment

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

es6 이후부터는 var 보다는 let,cosnt 를 사용하시는것을 추천드립니다~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants