-
Notifications
You must be signed in to change notification settings - Fork 28
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
[Mission5/홍창기] Project_Notion_VanillaJs 과제 #42
base: 4/#5_sscoderati
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.eslintrc.js
Outdated
"browser": true, | ||
"es2021": true | ||
}, | ||
"extends": "eslint:recommended", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prettier 규칙도 같이 적용되게 해보시죠~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9ab6c73 적용 완료했습니다!
index.html
Outdated
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meta 설정에 있어 어떤 값들이 들어갈 수 있는지 이참에 찾아보면 좋을것 같네요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
viewport의 개념과 content 속성에 들어갈 수 있는 width=device-width, initial-scale, minimum-scale, maximum-scale에 대해 알게 되어 적용하였습니다! 88855af 다음 과제부터는 기본적으로 적용해보겠습니다!
src/routes/PostChanger.js
Outdated
@@ -0,0 +1,12 @@ | |||
import { PostRouter } from "./PostRouter"; | |||
|
|||
export const PostChanger = (appEl) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메서드 이름에서 뭔가 어떤 역할이다라는게 딱 떠오르지는 않는것 같은데..이름을 바꿔주면 좋을것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9ab6c73 editorViewSwitcher
로 이름 수정하였습니다!
src/routes/PostRouter.js
Outdated
@@ -0,0 +1,12 @@ | |||
import Editor from "../components/Editor"; | |||
|
|||
export const PostRouter = (appEl, introEl) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기도 컴포넌트라기보다는 함수의 역할을 하고 있는것 같은데 함수 명명으로 넣는건 어떤가여?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9ab6c73 editorViewRouter
로 명명하였습니다!
src/core/Component.js
Outdated
this.render(); | ||
} | ||
render() { | ||
// 자식 클래스에서 Override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저번 리뷰때 남긴것처럼 자식 클래스에서 해당 메서드를 강제하고 싶으면 throw를 해주는게 좋아보여요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
33a4387 적용해보았습니다!
src/components/DocumentItem.js
Outdated
const res = request("", { | ||
method: "POST", | ||
body: JSON.stringify({ | ||
title: "제목 없음", | ||
parent: `${this.state.id}`, | ||
}), | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요청을 보낼때 메서드나 직렬화를 컴퍼넌트 내에서 하지말고 별도 서비스를 만들어서 사용하는게 좋아보여요
src/api/DocumentAPI.js
Outdated
@@ -0,0 +1,20 @@ | |||
const API_END_POINT = "https://kdt-frontend.programmers.co.kr/documents"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전반적으로 폴더 명을 대문자로 지은것 같은데 컴포넌트 명이 아닌 파일들은 소문자로 짓는게 일반적인 규칙으로 적합할거같아요
src/components/Document.js
Outdated
for (const key in docu) { | ||
const { id, title, documents } = docu[key]; | ||
const parentDocu = new DocumentItem(); | ||
parentDocu.setState({ id, title, isFolded: true }); | ||
container.appendChild(parentDocu.el); | ||
if (container.getAttribute("class") !== "container") { | ||
parentDocu.el.setAttribute("style", "display: none"); | ||
parentDocu.el.setAttribute("class", "child"); | ||
} | ||
if (documents.length !== 0) { | ||
renderDocuments(parentDocu.el, documents); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for -in, for of 대신 map, forEach, reduce등의 메서드를 사용해보려고 노력해보세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 노력해야겠습니다.
src/components/Document.js
Outdated
if (container.getAttribute("class") !== "container") { | ||
parentDocu.el.setAttribute("style", "display: none"); | ||
parentDocu.el.setAttribute("class", "child"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반복적으로 DOM API 를 사용한다면 element의 존재 여부도 확인이 되면 좋으니 utils/dom.js 경로에 classList contains를 확인하는 유틸이나 attribute를 지정하는 메서드를 선언해서 사용하시면 훨씬 코드도 간략해지고 반복적으로 쓰기도 좋을겁니다
src/components/Document.js
Outdated
this.el.setAttribute("id", "document-app"); | ||
this.el.setAttribute("style", "background-color: rgb(251,251,250)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
스타일 지정은 클래스로 작성하시죠~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
적용하기 어려운 구조인 것 같아서 css로 접근해보았습니다.. 6676c27 혹시 이런 구조에서 클래스로 스타일 적용을 하려면 어떻게 하는게 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
창기님 코드 잘보고 오늘도 많이 배우고 갑니다!
src/app.js
Outdated
window.addEventListener("404-not-found", () => { | ||
// TODO: 404 페이지 렌더링 | ||
console.log("404 Not Found"); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
404 렌더링... 결국 하셨군요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗 아직 하지는 않았는데... 구현하면 알려드리겠습니다! 지금은 없는 글을 불러오려고 하면 콘솔에 찍는 단계까지만 구현했어요😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@doggopawer e894536 구현하였습니다!
(테스트 링크 : https://sscoderati-notion-clone.netlify.app/documents/12345)
src/components/Document.js
Outdated
for (const key in docu) { | ||
const { id, title, documents } = docu[key]; | ||
const parentDocu = new DocumentItem(); | ||
parentDocu.setState({ id, title, isFolded: true }); | ||
container.appendChild(parentDocu.el); | ||
if (container.getAttribute("class") !== "container") { | ||
parentDocu.el.setAttribute("style", "display: none"); | ||
parentDocu.el.setAttribute("class", "child"); | ||
} | ||
if (documents.length !== 0) { | ||
renderDocuments(parentDocu.el, documents); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 노력해야겠습니다.
src/components/Editor.js
Outdated
const inputCategory = target.getAttribute("class"); | ||
if (timer !== null) { | ||
clearTimeout(timer); | ||
} | ||
timer = setTimeout(() => { | ||
const currentWriting = { | ||
[inputCategory]: target.value, | ||
updatedAt: new Date(), | ||
}; | ||
setItem(this.state.id, currentWriting); | ||
savePostOnServer(this.state.id, currentWriting); | ||
}, 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 동일한 부분에서 많이 부족한것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스형 컴포넌트로 구현된 코드를 보는 게 생각보다 어려워서 사실 어떻게 리뷰를 남겨야할까 고민이 많이 됐지만, 404 처리, 제목 수정 시 좌측 documentList에 바로 반영되는 기능, 반복되는 함수 유틸화 등 과제에 많은 공을 들이신 게 느껴지는 코드였습니다 ! 앞으로 다른 과제들이 나올 수록 창기님께 더 여쭤볼 게 많을 것 같네요 ㅎㅎㅎ 과제 너무 고생 많으셨고 앞으로도 화이팅합시다 💪🏻🤗
const { status } = res; | ||
if (status === HTTP_STATUS_RESPONSE_OK) { | ||
return res.json(); | ||
} else if (status === HTTP_STATUS_RESPONSE_NOT_FOUND) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
status 값에 따라서 분기처리,,, 꼼꼼하시네요 👍🏻
@@ -0,0 +1,7 @@ | |||
let timer = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 디바운스 유틸화 시키니까 동작을 잘 안하던데 ㅠ 창기님 코드 참고해봐야겠습니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
창기님 과제 하시느라 고생 많으셨습니다✨
매사 열심히 하시고 또 잘 하셔서 저에게 많은 자극이 되는 것 같아요!
코드 보고 많이 배워갑니다~~👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스형을 이용해 다른 컴포넌트들에서 'Component'를 상속받아 중복된 코드들을 처리할 수 있다는 점이 좋은 것 같습니다. 창기님께서 이번 과제는 함수형으로 구현한다고 하셨는데 어떻게 처리하셨을지 기대가 됩니다😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 부분에서 서비스 완성도가 높아지는 것 같습니다!👍
📌 과제 설명
Notion을 클론!! 해보는 프로젝트지만, 마감 직전에 살펴보니 조금 발전된 형태의 TodoList 같습니다...
배포는 했지만, 발견되거나 발견되지 않은 에러가 많은 듯 하네요..
https://sscoderati-notion-clone.netlify.app/
👩💻 요구 사항과 구현 내용
기본 요구 사항별 구현 상황
바닐라 JS만을 이용해 노션을 클로닝합니다.
기본적인 레이아웃은 노션과 같으며, 스타일링, 컬러값 등은 원하는대로 커스텀합니다.
화면 좌측에 Root Documents를 불러오는 API를 통해 루트 Documents를 렌더링합니다.
Root Document를 클릭하면 오른쪽 편집기 영역에 해당 Document의 Content를 렌더링합니다.
해당 Root Document에 하위 Document가 있는 경우, 해당 Document 아래에 트리 형태로 렌더링 합니다.
Document Tree에서 각 Document 우측에는 + 버튼이 있습니다. 해당 버튼을 클릭하면, 클릭한 Document의 하위 Document로 새 Document를 생성하고 편집화면으로 넘깁니다.
편집기에는 기본적으로 저장 버튼이 없습니다. Document Save API를 이용해 지속적으로 서버에 저장되도록 합니다.
History API를 이용해 SPA 형태로 만듭니다.
루트 URL 접속 시엔 별다른 편집기 선택이 안 된 상태입니다.
/documents/{documentId} 로 접속시, 해당 Document 의 content를 불러와 편집기에 로딩합니다.
내부에서 문서를 선택해서 각 문서별 내용을 불러올 수는 있지만, 외부에서 해당 URL로 접속을 시도하면 404 에러가 납니다..추가 구현 사항
✅ 피드백 반영사항
render()
메소드 호출 강제화✅ PR 포인트 & 궁금한 점
각 컴포넌트의 render 부에서 너무 많은 것들을 담당하고 있어서 외부 함수로 분리하고 싶은데, 상태 처리나 비동기 함수의 호출 관계가 복잡해져서 끝내 분리하지 못했습니다. 혹시 관련하여 참고하면서 공부할 만한 글이 있을까요..?