[Mission5/주다현] Project_Notion_VanillaJs 과제 #37

judahhh · 2023-07-06T14:53:29Z

📌 과제 설명

노션의 기본적인 기능을 구현하는 노션 클로닝 과제를 진행하였습니다.

👩‍💻 요구 사항과 구현 내용

기본 요구사항

글 단위를 Document라고 합니다. Document는 Document 여러개를 포함할 수 있습니다.
화면 좌측에 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 접속 시엔 별다른 편집기 선택이 안 된 상태입니다.
/{documentId} 로 접속시, 해당 Document 의 content를 불러와 편집기에 로딩합니다.

파일 구조

📦src
┣ 📂api
┃ ┗ 📜api.js
┣ 📂components
┃ ┣ 📂editContainer
┃ ┣ 📂sidebar
┃ ┣ 📜DocumentEditPage.js
┃ ┣ 📜DocumentList.js
┃ ┣ 📜DocumentPage.js
┃ ┣ 📜Editor.js
┃ ┗ 📜Header.js
┣ 📂pages
┃ ┗ 📜MainPage.js
┣ 📂style
┃ ┗ 📜style.css
┣ 📂utils
┃ ┣ 📜ApiKey.js
┃ ┣ 📜displayDocumentList.js
┃ ┣ 📜removeDocumentList.js
┃ ┣ 📜router.js
┃ ┗ 📜storage.js
┣ 📜App.js
┗ 📜index.js

스크린샷

✅ 피드백 반영사항

최대한 함수분리를 하여 가독성 있는 코드를 작성하려고 노력하였습니다.

✅ PR 포인트 & 궁금한 점

컴포넌트 설계 시 이것보다 더 나은 방법이 있다면 알려주세요!

4/#5 judahhh working

imb96

코드가 깔끔해 보여요 😄
과제하느라 고생 하셨습니다. 👏

imb96 · 2023-07-08T06:47:46Z

src/App.js

+
+  const mainPage = new MainPage({
+    $target,
+    initialState: "주다현",


p5: INITIAL_USER_NAME or DEFAULT_USER_NAME 이런식으로 문자열을 상수화를 해서 사용한다면 더 좋지 않을까 생각합니다. 😁

DongjaJ

다현님 고생하셨습니다!
바닐라 JS에 익숙하지 않아 스트레스 많이 받으셨는데 과제 하시는거 보니까 점점 바닐라 JS에도 익숙해지시는거 같아요. 조금만 지나면 라이브러리 쓸 수 있으니 화이팅해요~
수고하셨어요 !

DongjaJ · 2023-07-09T13:08:46Z

.gitignore

@@ -0,0 +1 @@
+ApiKey.js


p5: 이거는 혹시 무슨 이유로 추가하셨나요??

apiKey가 여기서 노션 api에 접근하기 위한 auth인데 gitignore로 지금 감춰놨습니다.
여기 github 프로젝트에서는 다른 사람이 확인할수 없으므로 포함시켜주는게 맞습니다. @judahhh
만약 감추고 싶다면 배포된 페이지 링크라도 있어야해요.

DongjaJ · 2023-07-09T13:10:37Z

src/components/DocumentEditPage.js

+    this.state = nextState;
+    const document = this.state.document;
+    if (document) {
+      if (document.title === "제목없음") {


p4: 이런 조건들은 상수로 빼도 괜찮을 것 같아요!

DongjaJ · 2023-07-09T13:12:01Z

src/components/DocumentEditPage.js

+  });
+
+  this.setState = async (nextState) => {
+    if (this.state.documentId !== nextState.documentId) {


p3: 내부에서 this.state내부 참조를 많이하는데

const {document, documentId} = this.state;

이런식으로 미리 빼두면 더 편하게 사용할 수 있을 것 같아요

DongjaJ · 2023-07-09T13:13:01Z

src/components/DocumentEditPage.js

+  const fetchDocument = async () => {
+    const { documentId } = this.state;
+
+    if (this.state.documentId !== null) {


p2: 위에서 구조분해로 빼놓고 여기서는 다시 안빼는 이유가 있을까요..?

DongjaJ · 2023-07-09T13:16:53Z

src/utils/displayDocumentList.js

+export const displayDocumentList = (documents) => {
+  return `<ul>
+          ${documents
+            .map((document) => {


p3: 이런식으로 미리 빼두면 더 편할 것 같아요 : )

Suggested change

.map((document) => {

.map(({id,title,documents}) => {

DongjaJ · 2023-07-09T13:21:21Z

src/components/DocumentEditPage.js

+
+      if (this.state.documentId === "new") {
+        editor.setState(
+          this.state.document || {
+            title: "",
+            content: "",
+          }
+        );
+      } else {
+        await fetchDocument();
+      }


p4: 빈 게시글로 상태변경하는 로직이 반복되는 것 같은데 이런식으로 할수도 있을 것 같아요?

Suggested change

if (this.state.documentId === "new") {

editor.setState(

this.state.document || {

title: "",

content: "",

}

);

} else {

await fetchDocument();

}

function getEmptyDocument(){

return {title:'',content:''}

}

if (this.state.documentId === "new") {

editor.setState(

this.state.document || getEmptyDocument()

);

} else {

await fetchDocument();

}

DongjaJ · 2023-07-09T13:27:17Z

src/components/DocumentList.js

+  $documentList.addEventListener("click", (e) => {
+    const { target } = e;
+    const element = target.closest("[name]");


p4: 이런식으로도 바꿀수 있을것 같습니다! 그리고 element라는 변수명은 약간 추상적일수 있다고 생각하는데 조금더 구체적으로 네이밍하면 어떨까요?

Suggested change

$documentList.addEventListener("click", (e) => {

const { target } = e;

const element = target.closest("[name]");

$documentList.addEventListener("click", ({target}) => {

const element = target.closest("[name]");

DongjaJ · 2023-07-09T13:27:56Z

src/components/DocumentList.js

+    const element = target.closest("[name]");
+
+    if (element) {
+      const id = element.dataset.id;


p5: 비슷하지만 저는 요즘 이렇게 사용해요!

Suggested change

const id = element.dataset.id;

const {id} = element.dataset;

DongjaJ · 2023-07-09T13:29:55Z

src/style/style.css

+textarea {
+  border: none;
+  text-align: justify;
+  line-height: 1.8;


p5: 저는 line-height에 px같은 단위나 %를 붙여서 사용했는데 단위를 안붙이면 어떻게 적용되나요?

nsr1349

안녕하세요 다현님 과제 하시느라 고생하셨습니다! 다현님 코드 보면서 뭔가 여러가지 알아가는 느낌이라 좋네요 늦은 시간에 죄송합니다 ㅠㅠ

nsr1349 · 2023-07-09T14:57:05Z

src/components/DocumentPage.js

+
+  this.render = async () => {
+    await fetchDocuments();
+    $target.prepend($documentPage);


이런건 처음 봤네요 배워갑니다..

nsr1349 · 2023-07-09T14:58:51Z

src/App.js

+    const { pathname } = window.location;
+    if (pathname === "/") {
+      removeDiv(".edit-page");
+      mainPage.render();


확실히 이렇게 메인 페이지가 따로 있는 게 좋은 선택인 것 같네요 ㄷㄷ

nsr1349 · 2023-07-09T15:02:21Z

src/components/DocumentEditPage.js

+        });
+
+        if (this.state.document.title) {
+          if (this.state.document.title !== document.title) {


this.state.document.title 가 예상치 못한 값일 수 있어서 조건문이 두개인걸까요? 뭔가 아래 조건문 하나로도 될 것 같은 데 정확히는 모르겠네요..

nsr1349 · 2023-07-09T15:04:06Z

src/components/DocumentPage.js

+
+  new Header({
+    $target: $documentPage,
+    initialState: "주다현",


사소한 문제 같긴하지만 이름은 또 쓰일 수도 있으니까 변수에 담아 쓰면 좋을 것 같네요!

yohanpro · 2023-07-10T07:15:27Z

index.html

+      src="https://kit.fontawesome.com/0ab4707042.js"
+      crossorigin="anonymous"
+    ></script>
+  </head>


<meta name="viewport" content="width=device-width, initial-scale=1.0">
도 있어야 할 것 같아요

yohanpro · 2023-07-10T07:26:43Z

src/components/DocumentEditPage.js

+    if (this.state.documentId !== null) {
+      const document = await request(`/${documentId}`);
+
+      const getTempDocument = getItem(documentLocalSaveKey, {


이 함수는 쓰이지 않는것 같네요

yohanpro · 2023-07-10T07:30:45Z

src/components/DocumentEditPage.js

+  $editPage.className = "edit-page";
+  this.state = initialState;
+
+  let documentLocalSaveKey = `temp-document-${this.state.documentId}`;


클래스안에 이렇게 변화할수있는 변수가 있는 것은 좋지 않습니다. 나중에 React를 쓰더라도 마찬가지입니다.

this.getDocumentLocalSaveKey =()=> { return `temp-document-${this.state.documentId}`; }; const tempDocument = getItem(this.getDocumentLocalSaveKey(), { title: "", content: "", });

위처럼 질의함수로 바꾸는 연습을 해보세요

yohanpro · 2023-07-10T07:33:22Z

src/components/DocumentEditPage.js

+        clearTimeout(timer);
+      }
+
+      timer = setTimeout(async () => {


debounce용으로 타이머를 onEditing에 넣으신 것으로 추측됩니다.

debounce를 따로 util/debounce.js로 만들고 아래와 같이 사용할 수 있도록 변경해보세요

onEditing: debounce(async (document) => { // ... }, 500),

yohanpro · 2023-07-10T07:42:49Z

src/utils/router.js

+  });
+};
+
+export const push = (element) => {


이름이 너무 범용적입니다. 좋은 네이밍을 고민해보세요

yohanpro · 2023-07-10T07:43:02Z

src/utils/router.js

+import { request } from "../api/api.js";
+
+const ROUTE_EVENT_NAME = "click-event-route-change";
+const ROUTE_EDIT_EVENT = "edit-event-route-change";


이 이벤트는 안쓰는 것 같네요

yohanpro · 2023-07-10T07:48:11Z

src/utils/router.js

+const ROUTE_EVENT_NAME = "click-event-route-change";
+const ROUTE_EDIT_EVENT = "edit-event-route-change";
+
+export const editorRoute = (onRoute) => {


이렇게 작성하면 객체지향원칙에서 Open-Closed Principle (OCP)을 위반한 것입니다. 제가 만약 이 코드를 유지보수해야한다고 하면, 직접 수정해야 하므로 모든 로직이 제대로 제대로 돌아갈지 의심하면서 작성해야합니다.

뿐만 아니라 else if가 많아서 한번에 이해하기가 어렵네요.

const eventHandlers = { 'list': async (id, documentId) => { if (id == null) { id = documentId; } history.pushState(null, null, `/${id}`); onRoute(null); }, 'remove-btn': (id, documentId) => { if (documentId === id) { history.pushState(null, null, "/"); } onRoute(null); }, ..... }

window.addEventListener(ROUTE_EVENT_NAME, async (e) => { const { type, id } = e.detail; const { pathname } = window.location; const [, documentId] = pathname.split("/"); const handler = eventHandlers[type]; if (handler) { await handler(id, documentId); } });

위처럼 작성해보는건 어떨까요?

yohanpro · 2023-07-10T07:49:44Z

src/style/style.css

+}
+
+.document-list {
+  font-size: 0.98em;


어떻게 font size가 결정될지 두근두근하면서 확인해야하는 ... font size네요

yohanpro · 2023-07-10T07:50:14Z

src/components/Header.js

@@ -0,0 +1,23 @@
+import { push } from "../utils/router.js";
+
+export default function Header({ $target, initialState }) {


헤더는 깔끔하게 잘 작성하셨네요

yohanpro · 2023-07-10T07:55:56Z

src/components/Editor.js

+  const $editor = document.createElement("div");
+  $editor.className = "editor";
+  $target.appendChild($editor);
+  let isinitialize = false;


카멜케이스에 isInitialize 따라 바꾸면 좋을 것 같습니다. 그리고 질의함수로 바꾸는걸 연습해보세요.

this.isInitialized = function() { return isInitialize; };

yohanpro · 2023-07-10T07:57:29Z

src/components/Editor.js

+
+  this.render();
+
+  $editor.addEventListener("keypress", (e) => {


후에 React를 쓰실때도 참고사항이지만 keypress는 deprecated예정으로 keyup등을 사용해보세요

https://developer.mozilla.org/en-US/docs/Web/API/Element/keypress_event

yohanpro · 2023-07-10T08:01:14Z

가장 어려운 노션 페이지 만드는 것을 잘 완성해내셨습니다.

다만 제가 결과를 확인할 수가 없는데, 배포된 페이지나 apiKey를 추가해야 할 것 같습니다.

page로 나눈 것은 좋은 구조입니다.
네이밍 연습을 더 해보시면 좋을 것 같습니다.
displayDocumentList는 핵심로직인데 util에 들어가 있습니다. util에는 데이터 포맷팅(날짜, 돈 계산, debounce )같은 함수들이 들어갑니다.

judahhh added 15 commits June 28, 2023 18:27

chore : 디렉토리 구조 및 컴포넌트 설계

90234de

chore : 폴더 구조 변경

d7edf51

feat : 로컬 스토리지에 저장 및 삭제, 조회하는 기능 추가

1c2f419

feat : 사이드바 안에 헤더를 누르면 소개 페이지로 가는 라우팅 추가

6cfe708

feat : 글 추가, 삭제 시 해당 페이지나 홈으로 이동하는 라우팅 추가

e4f1dd6

feat : 삭제 버튼 클릭 시 글 삭제

2c47f50

style : css 추가

660c9e9

keypress 이벤트를 통한 실시간 편집 기능 추가

00df870

chore : api base url 비공개

a8d3c62

feat : 재귀를 통해 하위페이지 조회

be1ffe6

feat : 각 글 목록 보여주기

d05ec51

feat : DocumentEditPage에서 Editor 컴포넌트 렌더링

1449d38

feat : 사이드 바에서 각 목록 리스트를 담은 컴포넌트 렌더링

f2bd999

chore : 폴더 구조 변경 및 파일 삭제

d2dfeb2

Merge pull request #26 from judahhh/4/#5_judahhh_working

305cdfd

4/#5 judahhh working

judahhh added 💧 순한맛 요한팀 피드백 요청 과제 리뷰 요청 labels Jul 6, 2023

judahhh requested a review from yohanpro July 6, 2023 14:53

judahhh self-assigned this Jul 6, 2023

imb96 reviewed Jul 8, 2023

View reviewed changes

DongjaJ reviewed Jul 9, 2023

View reviewed changes

nsr1349 reviewed Jul 9, 2023

View reviewed changes

yohanpro reviewed Jul 10, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mission5/주다현] Project_Notion_VanillaJs 과제 #37

[Mission5/주다현] Project_Notion_VanillaJs 과제 #37

judahhh commented Jul 6, 2023 •

edited

Loading

imb96 left a comment

imb96 Jul 8, 2023

DongjaJ left a comment

DongjaJ Jul 9, 2023

yohanpro Jul 10, 2023

DongjaJ Jul 9, 2023

DongjaJ Jul 9, 2023

DongjaJ Jul 9, 2023

DongjaJ Jul 9, 2023

DongjaJ Jul 9, 2023

DongjaJ Jul 9, 2023

DongjaJ Jul 9, 2023

DongjaJ Jul 9, 2023

nsr1349 left a comment

nsr1349 Jul 9, 2023

nsr1349 Jul 9, 2023

nsr1349 Jul 9, 2023

nsr1349 Jul 9, 2023

yohanpro Jul 10, 2023 •

edited

Loading

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro Jul 10, 2023

yohanpro commented Jul 10, 2023

		@@ -0,0 +1,23 @@
		import { push } from "../utils/router.js";

		export default function Header({ $target, initialState }) {

		@@ -0,0 +1 @@
		ApiKey.js

[Mission5/주다현] Project_Notion_VanillaJs 과제 #37

Are you sure you want to change the base?

[Mission5/주다현] Project_Notion_VanillaJs 과제 #37

Conversation

judahhh commented Jul 6, 2023 • edited Loading

📌 과제 설명

👩‍💻 요구 사항과 구현 내용

기본 요구사항

파일 구조

스크린샷

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

imb96 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DongjaJ 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

nsr1349 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

yohanpro Jul 10, 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

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

yohanpro commented Jul 10, 2023

judahhh commented Jul 6, 2023 •

edited

Loading

yohanpro Jul 10, 2023 •

edited

Loading