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 YunJiYoung 직원 사진 관리 서비스 #47

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

Conversation

developer-jyyun
Copy link

@developer-jyyun developer-jyyun commented Aug 18, 2023

📷 직원 사진 관리 서비스

직원들의 사진을 관리할 수 있는 사진 관리자 서비스

https://javascript-picture.web.app/index.html

ID: [email protected]

PW: 123456

  • 비로그인시 데이터 조회만 가능합니다.

  • [email protected]으로 로그인 시 모든 데이터의 등록 / 수정 / 삭제가 가능합니다. 😊

  • 회원가입/로그인 시 자신이 사진 등록 가능하며, 작성자와 로그인 한 유저가 동일 할 경우에 등록한 사진의 수정, 삭제가 가능합니다.

[구현범위]

  • “Firebase 서비스”를 이용하여 사진을 관리할 수 있는 페이지를 구현
  • 프로필 페이지를 개발
  • 스크롤이 가능한 형태의 리스팅 페이지를 개발
  • 전체 페이지 데스크탑-모바일 반응형 페이지를 개발
  • 사진 등록, 수정, 삭제 가능
  • 로그인 / 회원가입 페이지 개발
  • CSS
    • 애니메이션 구현
    • 상대수치 사용(rem, em)
  • JavaScript
    • DOM event 조작
  • 직원을 등록, 수정, 삭제가 가능
  • infinity scroll 기능
  • LocalStorage 사용
  • 로딩페이지 구현

[흐름]

userflow

[필요한 작업]

  • 검색 / 정렬 기능
  • 데이터 유효성 검사

@developer-jyyun developer-jyyun changed the title Kdt0 yun ji young Kdt0 yun ji young 직원 사진 관리 서비스 Aug 18, 2023
@developer-jyyun developer-jyyun changed the title Kdt0 yun ji young 직원 사진 관리 서비스 Kdt0 YunJiYoung 직원 사진 관리 서비스 Aug 18, 2023
Copy link

@Gaoridang Gaoridang left a comment

Choose a reason for hiding this comment

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

전체적으로 코드가 깔끔해서 보기 편했습니다! firebase는 최신 버전 문법이 조금 더 간결한 것 같습니다ㅎㅎ 고생 많으셨어요~~

Comment on lines +19 to +20
const queryString = new URLSearchParams(window.location.search);
queryString.get('id')

Choose a reason for hiding this comment

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

1이건 처음 보는 클래스(?)네요. 검색해보니 되게 유용할 것 같아요! 다음에 참고하겠습니다ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

저도 이번 과제를 통해 알게 되었는데 엄청 유용하게 쓰였습니다!:)

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 1 to 7
window.onload = function () {
setTimeout(()=>{
loading.style.opacity = "0";
loading.style.transform = "scale(0)";
loading.style.transition='all 1s';

},1200);

Choose a reason for hiding this comment

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

여기서 loading은 무엇인가요?? 선언이 안 된 것 같아서 궁금합니다!

Copy link
Author

@developer-jyyun developer-jyyun Aug 19, 2023

Choose a reason for hiding this comment

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

엇 코드 수정 중 삭제 됐던 것 같아요! 덕분에 초기에 수정했습니다!!! 감사합니다:)

public/main.js Outdated
Comment on lines 63 to 65
else {
entry.target.classList.remove("active");
}

Choose a reason for hiding this comment

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

개인적인 사용자 경험 측면에서 이전에 보였던 것들이 지워졌다가 다시 나타나는 것보다, 나타나는 효과만 있는게 좋을 것 같았습니다!

Copy link
Author

Choose a reason for hiding this comment

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

엇 재준님 말씀 듣고 나타나는 효과만 있도록 바꿔보니 사용자 경험 측면에서도 좋고, ui측면에서도 훨씬 깔끔한 것 같습니다. 조언 감사합니다!!:)

@developer-jyyun
Copy link
Author

전체적으로 코드가 깔끔해서 보기 편했습니다! firebase는 최신 버전 문법이 조금 더 간결한 것 같습니다ㅎㅎ 고생 많으셨어요~~

firebase는 무조건 최신 버전을 쓰는 것이 좋은 것 같습니다...
구 버전은 모듈 사용 시 에러가 나네요...😭😭😭

@mysdpy30s
Copy link

로딩페이지부터 시작해서 메인 페이지 레이아웃, 폰트, 리스팅 등 각종 요소들이 너무 깔끔하고 보기 좋아서 감탄했어요! :) Infinity scroll도 부드럽게 잘 작동하는게 인상적입니다 😊 말씀하셨던것처럼 식물 관리 페이지로 진행하셨어도 굉장했을것 같아요 :D 고생 많으셨어요!

@lee-sangone
Copy link

infinity scroll 기능 구현 정말 부드럽네요 수고하셨습니다!

@im-na0
Copy link

im-na0 commented Aug 23, 2023

요구사항 웬만한건 다 구현하셨네요 너무너무 수고하셨습니다 !!~
로딩 애니메이션이랑 infinite scroll 요소들 나타날 때 애니메이션이 너무 귀여운 것 같아요 ㅎㅎㅎ
수정 완료 시에 alert창 뜨는 센스까지..👍 그리고 전체적으로 ux 흐름이 자연스러워서 처음 오는 사용자도 편안하게 사용할 수 있을 것 같습니다 !!

Copy link

@SOL-MI SOL-MI left a comment

Choose a reason for hiding this comment

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

로그인 기능이 아주 인상깊었던 귀여운 사이트였습니다 :D! 정말 많은 기능을 완성도 높게 구현하신 것 같아요ㅎㅎ 👍👍

@@ -0,0 +1,16 @@
window.onload = function () {
Copy link

Choose a reason for hiding this comment

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

window.onload라는 메소드를 지영님 덕에 처음 접해보게 됐네요 :D ㅎㅎ 감사합니다.
궁금한 점이 있는데 이 부분이 다른 내용들이 다 load되면 로딩 화면이 사라지게 하는 것에 대한 코드인건가요? 그렇다면 setTimeout으로 시간이 지정되어있는데 그 시간이 무슨 의미를 주는 것인지 궁금합니다! 다른 부분을 load하는 시간이 1.2초 보다 짧은 경우/ 긴 경우에 어떻게 되는 건지 궁금하다는 의미였습니다..!
그리고 제가 찾아봤었을 때 나머지 모든 것들 load후에 불러오는 방법으로 window.onload / window.addEventListener('load',function(){}) / (html에서) defer / (html에서) script태그를 가장 하단에 위치 이렇게 4가지 방법을 비교하는 것 같더라구요. 4개 중 하나만 쓰면 된다고 생각했는데 지영님 index.html에서 보면 defer가 쓰여있고, loading.js에는 window.onload 이렇게 2개가 다 쓰여있어서 제가 잘못 이해한 건가 하는 생각이 드네요ㅠㅠ 혹시 관련 내용을 조금만 설명해주실 수 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

setTimeout으로 시간 지정은 로드 후 + 1.2초 지난 뒤 로딩 페이지가 사라지게 만들어 주었습니다!
로드 후 곧바로 로딩 페이지가 사라지는 것이 일반적이지만 제 코드에 적용해보니 로드가 너무너무 빨라서 로딩페이지가 노출되는 것이 마치 에러같이 보이더라구요...그래서 setTimeout 통해 인위적으로 시간을 늘려 주었습니닷..

defer속성이 습관화 되기 전엔 로딩 직 후 곧바로 이루어 지는 작업들을 습관처럼 냅다 onload로 처리했었어서
defer와 onload가 불필요하게 중복된 것 같습니다! 리팩토링을 통해 수정해봐야겠네요!
솔미님 말씀처럼 (html에서) script태그를 가장 하단에 위치할 경우 defer속성 사용 유무에 따른 큰 효과가 없다고 들었으나, 지난번 멘토님께서 script 위치는 하단에 defer속성과 함께 사용하는 것을 권장하셨던 것 같아 스크립트가 하단에 위치함에도 defer속성을 작성하였습니다.(제가 잘못 이해한 것일까요?ㅠㅠ)

+)추가로 멘토님 코드 리뷰입니다! onload보단 addEventListner 활용이 좋겠네요! :)
.on* 패턴으로 이벤트 핸들러를 추가하기보단, addEventListener를 활용해보시는 게 어떨까요?
전자로 이벤트 핸들러를 추가하시면 동일한 이벤트에 여러 핸들러를 추가할 수도, 옵션을 추가할 수도 없어집니다.

</nav>
</header>

<div id="wrap">
Copy link

Choose a reason for hiding this comment

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

배포 사이트에선 제대로 된 것 같은데 여기선 div가 안 닫혀있는 것 같습니다..!

`;

const wrap = document.querySelector(".basic-container");
wrap.insertAdjacentHTML("beforeend", template);
Copy link

Choose a reason for hiding this comment

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

업로드됐을 때 이름 순 정렬 또는 시간 순 정렬이 안되니 업로드한 내용을 찾기가 조금 어렵더라구요..!
지금은 firestore내 고유 아이디 순으로 정렬되고 있는 것 같습니다..!
저는 시간 순 정렬을 채택했는데 그때 orderBy 메소드를 사용했었습니다ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

저도 느꼈던 부분입니다ㅠㅠ!! 나중에라도 정렬과 검색기능은 꼭 추가해보려구요!

.then(() => {
// intersectionObserver

const io = new IntersectionObserver((entries, observer) => {
Copy link

Choose a reason for hiding this comment

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

저는 이 부분 구현을 아직 못했는데 멋지십니다 :D 애니메이션 효과까지 최고!
저만 그런지 모르겠는데 화면을 크게 봤었을 때 중간중간 바로 로딩되지 않는 프로필이 있는 것 같습니다..! 스크롤을 다시 올렸다 내리면 그때 로딩이 됐었습니다. 모바일크기 화면에서는 문제 없었습니다!!!

Copy link
Member

@marshallku marshallku left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!
전반적으로 UI 구성도 깔끔하고 좋네요.
리뷰에 남긴 디테일한 영역들 잡아가보면 코드 완성도도 많이 올릴 수 있을 것 같습니다!

Comment on lines +7 to +9
ID: [email protected]

PW: 123456
Copy link
Member

Choose a reason for hiding this comment

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

placeholder에 이 정보를 대범하게 넣어두신 두 번째 분이십니다.
혹시나 싶어서 로그인해봤는데 되길래 놀랐습니다...ㅋㅋㅋ

Copy link
Author

Choose a reason for hiding this comment

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

앗 어드민 권한 가진 id의 테스트를 해볼 수 있도록 적어두려다 placehorder에 넣어두었습니다....
넣어두면서도 찜짐했었는데.. 아무리 과제여도...빼는 것이 좋겠죠..?😂

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
Author

Choose a reason for hiding this comment

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

커밋을 잘못하여 리셋 작업을 하였는데 그 때 추가되었습니다!

Comment on lines +9 to +12
rel="preload"
as="style"
onload="this.rel = 'stylesheet'"
href="/reset.css"
Copy link
Member

Choose a reason for hiding this comment

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

최적화도 시도해보셨네요!
혹시 이렇게 옵션 줬을 떄랑, 일반적으로 추가했을 때 퍼포먼스 비교도 해보셨나요?

<ul class="gnb">
<li><a href="/index.html">직원목록</a></li>
<li><a href="/upload/upload.html">업로드</a></li>
<li id="gnb__delete-btn"><a class="nav-delete-btn">삭제</a></li>
Copy link
Member

Choose a reason for hiding this comment

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

이 anchor는 어디 쓰이는 건가요?

Copy link
Author

Choose a reason for hiding this comment

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

이후에 메인 페이지에서도 삭제 기능이 가능하도록 하려 했는데 아직 구현하지 못해 숨김처리 해두었습니다 T_T

Comment on lines +10 to +19
<dt class="thumbnail" style="background-image: url(${result.data().img})"></dt>
</div>
<div class="col-2">
<dt class="name"><b>이름 :</b> ${result.data().name}</dt>
<dd><b>소속팀</b> : ${result.data().team}</dd>
<dd><b>직급</b> : ${result.data().rank}</dd>
<dd><b>내선번호</b> : ${result.data().ext}</dd>
<dd><b>연락처</b> : ${result.data().phone}</dd>
<dd><b>이메일</b> : ${result.data().email}</dd>
<dd><b>기타</b> : ${result.data().memo}</dd>
Copy link
Member

Choose a reason for hiding this comment

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

templateDetail 선언하기 이전에 destructuring으로 필드들 가져와보면 어떨까요?

const io = new IntersectionObserver((entries, observer) => {
entries.forEach((entry) => {
// 주시 대상이 뷰포트 안으로 들어오면 active 클래스 추가
if (entry.intersectionRatio > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

entry.isIntersecting로 상태를 확인하실 수 있습니다.
ratio로 체크하니까 가끔 스크롤이 빠르게 움직이면 화면에 출력이 안 되네요...

Comment on lines +24 to +26
.then((결과) => {
console.log(결과);
결과.forEach((doc) => {
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 +2 to +15
function readImage(input) {
// 인풋 태그에 파일이 있는 경우
if (input.files && input.files[0]) {
// FileReader 인스턴스 생성
const reader = new FileReader();
// 이미지가 로드가 된 경우
reader.onload = (e) => {
const previewImage = document.getElementById("preview-image");
previewImage.src = e.target.result;
};
// reader가 이미지 읽도록 하기
reader.readAsDataURL(input.files[0]);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

URL.createObjectURL()URL.revokeObjectURL()을 통해 이미지를 생성 / 삭제하실 수 있습니다!

public/preview.js Show resolved Hide resolved
Comment on lines +14 to +23
if (localStorage.getItem("user") != null) {
//스토리지에 이미지 저장

const file = document.querySelector("#image").files[0];
const storageRef = storage.ref();
const imgPath = storageRef.child(
"image/" + Math.round(Math.random() * 9999) + file
);

const upload = imgPath.put(file);
Copy link
Member

Choose a reason for hiding this comment

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

이런 공통되는 로직도 묶어서 관리하실 수 있을 것 같습니다!

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