[Mission5/임지선] Project_Notion_VanillaJs 과제 #50

Lim-JiSeon · 2023-07-06T15:10:29Z

📌 과제 설명

노션 클로닝 프로젝트 과제

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

문서 생성 기능
문서 자동 저장 기능
문서 자동 수정 기능
하위 문서 생성 기능(수정 필요)
문서 삭제 기능

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

현재 기본적인 요구사항들은 모두 구현한 상태입니다.
다만 문서 목록이 왼쪽에 고정되고, 편집기는 오른쪽에 고정되어야 하는데 rendering 할 때마다 위치가 랜덤으로 바뀝니다.
이는 제가 appendChild를 쓰다보니 element가 순서 상관없이 넣어지면서 발생한 문제같은데... 현재 해결을 못한 상태입니다.
혹시 이와 관련해 조언 부탁드립니다.
디자인... 최소한의 알아 볼 수 있게만 구현했습니다.. (더 예쁘게 수정할께요 ㅠㅜ)
코드.. 이번에도 거침없이 피드백 부탁드립니다!

arclien

지선님!! 수고하셨습니다!

렌더링 순서로 인해서 좌우가 바뀌는 이슈는 제가 해당 컴포넌트에서 리뷰 남겨드렸습니다!

지난번 Todo 과제보다 더 잘하신 것 같아요!
일단 컨벤션이 지켜져서 가독성이 너무 좋고 명료했습니다!

이번 과제의 요구사항에서 디자인에 대해서는 관여하지 않기로 햇으니 너무 걱정하지 않으셔도 됩니다!
하나씩 차근차근 해나가면 될 것 같고요!
단지 url path구조 요구사항에 대해 지켜지지 않았고, SPA도 되지 않아서 다른 분들의 코드를 보고 조금 더 고민을 해보시면 좋겠습니다!

이번 지선님의 코드는 과하거나 복잡하지 않고, 심플하게 코딩해서 요구사항을 맞춘 것 같습니다! 👍 👍 👍

arclien · 2023-07-09T07:06:31Z

src/App.js

+  const postNavBar = new PostNavBar({
+    $target,
+  });
+
+  const postEditPage = new PostEditPage({
+    $target,
+    initialState: {
+      postId: "new",
+      post: {
+        title: "",
+        content: "",
+      },
+    },
+  });


지선님 추측대로, appendChild하면서 타이밍 이슈로(아직 왜 그런진 파악 중입니다) 실제로 아래 사진과 같이 contentWrap 하위에 순서가 바뀌어서 렌더링 됩니다.
제일 쉬운 방법은, 의도적으로 contentWrap 하위에 2개의 div를 만들고 하나는 sidebar, 하나는 postContainer로 정의하여
각각의 $target을, postNavBar, postEditPage 생성시에 다른 타겟으로 넘겨주면 문제가 없을 것 같습니다!

관련해서 주하님의 PR 일부를 공유드릴테니 참고해서 바꿔봐주세요! 아주 조금만 수정하면 해결 됩니다!
https://github.com/prgrms-fe-devcourse/FEDC4-5_Project_Notion_VanillaJS/pull/48/files#diff-3d74dddefb6e35fbffe3c76ec0712d5c416352d9449e2fcc8210a9dee57dff67R6-R22

arclien · 2023-07-09T07:07:22Z

src/App.js

+    if (pathname !== "/" && pathname.indexOf("/") === 0) {
+      const [, postId] = pathname.split("/");
+      postEditPage.setState({ postId });
+    }


노션 클로닝 프로젝트 요구사항

History API를 이용해 SPA 형태로 만듭니다. 루트 URL 접속 시엔 별다른 편집기 선택이 안 된 상태입니다. /documents/{documentId} 로 접속시, 해당 Document 의 content를 불러와 편집기에 로딩합니다.

요구사항과 살짝 다르게 개발이 되었습니다.

arclien · 2023-07-09T07:13:56Z

src/PostNavBar.js

+import PostList from "./PostList.js";
+import { pushRouter } from "./router.js";
+
+export default function PostNavBar({ $target }) {


PostSideNavbar, PostSidebar 등의 이름이 더 좋을 것 같아요!
Navbar는 통상적으로 상단 GNB(Global Nav Bar)를 뜻할 것 같다고 느껴집니다!

arclien · 2023-07-09T07:18:05Z

src/PostNavBar.js

+    $nav.appendChild($title);
+    $nav.appendChild($createButton);


$nav 에다가 PostList를 생성하고나서
title, createButton을 추가하셨습니다!
노션목록의 아이템이 많아지면, 문서 생성하기 CTA버튼이 하단으로 내려가서 스크롤을 하게 되어서 UX가 좋지 않습니다.
하단에 fixed를 하던가, title, button을 먼저 렌더링 후에 PostList를 보여주면 좋겠습니다

arclien · 2023-07-09T07:25:59Z

src/api.js

+    throw new Error("API 처리중 에러가 발생했습니다.");
+  } catch (e) {
+    alert(e.message);
+    console.error();


console.error(e);
이렇게 처리해주셔야합니다!

arclien · 2023-07-09T07:34:31Z

src/PostList.js

+    this.render();
+  };
+
+  this.makeList = ($wrap, data) => {


$wrap말고 조금 더 의미있는 파라미터 명을 작성해주세요!!
$target은 PostList에서 이미 사용중이라 일부러 $wrap을 하신 것 같습니다.

$postItem 혹은 $itemContainer 등은 어떨까요

arclien · 2023-07-09T07:43:37Z

src/PostItem.js

+  $title.addEventListener("click", async () => {
+    request(`/documents/${$title.className}`);
+    pushRouter(`/${$title.className}`);
+  });
+
+  $addButton.addEventListener("click", async () => {
+    const createdPost = await request("/documents", {
+      method: "POST",
+      body: JSON.stringify({
+        title: "제목 없음",
+        parent: $addButton.className,
+      }),
+    });
+    pushRouter(`/${createdPost.id}`);
+  });
+
+  $removeButton.addEventListener("click", async () => {
+    await request(`/documents/${$removeButton.className}`, {
+      method: "DELETE",
+    });
+    pushRouter(`/`);
+  });


지선님!! 코드는 정말 깔끔하게 잘 짜셨어요 👍
기능도 잘 작동하고요!!

이제 조금 더 팁을 드려보겠습니다!
지금 PostItem각각에 eventListener를 추가하고 있는데, 나중에 수천개 수만개의 아이템이 생겼을 경우
너무 많은 event가 브라우저에서 등록되어 관리가 되어야 합니다.
그래서 이벤트 버블링 및 캡쳐링을통한 위임을 하면 더 좋을 것 같습니다!

아래 링크를 읽어보시면 좋을 것 같습니다! :)
https://joshua1988.github.io/web-development/javascript/event-propagation-delegation/

arclien · 2023-07-09T07:45:04Z

src/PostEditPage.js

+export default function PostEditPage({ $target, initialState }) {
+  const $page = document.createElement("div");
+  $page.id = "editPage";
+  $page.className = "2";


클래스 이름은...조금 더 의미있게 지어주세요!!! :)

arclien · 2023-07-09T07:47:07Z

src/Editor.js

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


카멜케이스로 작성 해주면 좋습니다!
isInitialized

wukdddang

지선님 과제 고생 많으셨습니다! ☺️

기본 요구사항대로 잘 구현해주셔서 코드가 깔끔했던 것 같습니다.
다만 예상치 못한 동작이 발생하는 것이나, 화면 깜박임을 신경써주시면 더 좋은 애플리케이션이 될 것 같습니다! 👍

고생 많으셨습니다!

wukdddang · 2023-07-10T04:53:25Z

src/PostNavBar.js

+  $createButton.addEventListener("click", async () => {
+    const createdPost = await request("/documents", {
+      method: "POST",
+      body: JSON.stringify({


저도 이렇게 request 내부에서 stringify를 했는데, 오프 멘토님 리뷰를 보니 직렬화 과정을 별도로 분리하는 게 좋을 것 같다는 피드백을 주셨더라구요. 다르게 구현하는 방법을 생각해보는거도 좋을 것 같습니다!

wukdddang · 2023-07-10T05:00:07Z

src/router.js

+
+export const pushRouter = (nextUrl) => {
+  window.dispatchEvent(
+    new CustomEvent("route-change", {


위에 ROUTE_CHANGE_EVENT_NAME을 상수로 선언해주셨으니 상수를 사용하시는 게 좋을 것 같습니다.

wukdddang · 2023-07-10T05:01:57Z

src/PostEditPage.js

+          pushRouter(`/${this.state.postId}`);
+          await getDocument();
+        }
+      }, 2000);


저도 이렇게... 직접 수를 입력했는데요, 매직 넘버이기 때문에 나중에 프로젝트가 커지면 관리가 쉽지 않을 수 있다고 합니다. 상수로 관리하시는 것을 추천드립니다. ☺️

오프 멘토님 리뷰

오우 갑자기 멘션당해서 깜짝 놀랐습니다 ㅎㅎㅎ 맞아요 매직 넘버와 매직 스트링 쓰는 습관을 들이는건 좋은 것 같습니다! 상수의 경우 여러 곳에서 동일한 의미와 용도로 사용된다면 매직 넘버로 할당하시는게 좋고, 오탈자가 발생하면 에러가 일어날 확률이 높은 문자열은 매직 스트링으로 할당해서 사용하시는게 좋습니다! (IDE의 자동 완성의 힘을 믿어보자구요!)

wukdddang · 2023-07-10T05:21:42Z

src/Editor.js

+
+  this.render = () => {
+    if (!isinitialize) {
+      $editor.innerHTML = `


새로운 글을 클릭할 때마다 Editor가 렌더링 되다보니, input과 textarea가 매번 생성되서 화면 깜빡임(Flickering) 현상이 발생하는 것 같습니다. 맨 처음 Editor를 생성할 때 외에는 value 값만 변경하는 게 어떨지 추천드립니다. ☺️

저도 창욱님의 말씀에 동의합니다! 아무래도 동적으로 추가되고 삭제될 일이 없는 DOM element 는 컴포넌트 초기화 시점애 미리 만들어두는 것이 좋을것 같다고 생각합니다!

wukdddang · 2023-07-10T05:31:09Z

style.css

+  margin: 0 5 0 5;
+}
+
+button:hover {


cursor: pointer; 를 사용해서 더 나은 UX를 제공해줄 수 있을 것 같습니다. ☺️

호원님 리뷰

주하님 리뷰

hayamaster

지선님! 과제하느라 너무 고생하셨습니다!!

저도 강의 실습에서 진행했던 내용을 토대로 과제를 진행했기에 지선님의 코드와 비슷한 부분이 많아 매우 잘 읽혔습니다!
렌더링 시 순서가 바뀌는 문제는 피드백을 참고하시면 금방 해결하실거라 생각됩니다!!

저희 서로 노션 과제 리펙토링할때 더 잘 꾸며봐요! ㅎㅎ😆

hayamaster · 2023-07-10T11:25:02Z

src/App.js

+  const postNavBar = new PostNavBar({
+    $target,
+  });
+
+  const postEditPage = new PostEditPage({
+    $target,
+    initialState: {
+      postId: "new",
+      post: {
+        title: "",
+        content: "",
+      },
+    },
+  });


지석 멘토님께서 말씀해 주셨듯이 사이드 바와 편집기 두개를 각각 새로운 컴포넌트로 만들어서 상위 $target에 넣어주면 렌더링시 순서가 바뀌는 문제를 쉽게 해결할 수 있을 것 같습니다!

hayamaster · 2023-07-10T12:19:03Z

src/PostItem.js

+    const createdPost = await request("/documents", {
+      method: "POST",
+      body: JSON.stringify({
+        title: "제목 없음",
+        parent: $addButton.className,
+      }),
+    });


api 통신은 여러번 쓰일 수 있으니 따로 분리해두는 것이 편리할 수 있을 것 같습니다!
저같은 경우는 api.js 파일에 "PUT", "POST", "DELETE", "GET" 함수를 모아두어 필요할 때마다 꺼내 사용했습니다!

hayamaster · 2023-07-10T12:21:10Z

src/PostEditPage.js

+
+  this.render = () => {
+    $target.appendChild($page);
+    console.log($target)


아마 테스트 해보실 때 console찍어보신 것 같습니다 ㅎㅎ
불필요한 코드는 지워주셔도 좋을 듯 합니다!
(사실 이부분은 제가 고쳐야 되는 부분입니다...🥲)

hayamaster · 2023-07-10T13:18:14Z

src/PostNavBar.js

+  const $createButton = document.createElement("button");
+  const $title = document.createElement("h1");
+  $nav.id = "nav";
+  $nav.className = "1";


위에서 지석멘토님께서 피드백 주셨듯이, 의미있는 className을 정해주는 것이 좋을 것 같습니다!
또한 $nav에는 이미 id를 주셨는데, 별도로 className을 넣어주신 이유가 궁금합니다!

howons

지선님도 수고 많으셨습니다~
다른 분들이 선점하신 게 많아서 저는 추가할 게 별로 없네요..
vercel 배포가 생각보다 처음해도 쉽던데 지선님도 시도해보시는 걸 추천드립니다! ☺️

howons · 2023-07-10T13:17:48Z

src/router.js

+export const initRouter = (onRoute) => {
+  window.addEventListener(ROUTE_CHANGE_EVENT_NAME, (e) => {
+    const { nextUrl } = e.detail;
+
+    if (nextUrl) {
+      history.pushState(null, null, nextUrl);
+      onRoute();
+    }
+  });
+};


지선님도 뒤로가기/앞으로가기 때 발생하는 popstate 이벤트 처리 깜빡하신 것 같습니다!

howons · 2023-07-10T13:30:47Z

style.css

+li {
+    padding: 10 5 10 5;
+}


저만 몰랐을 수도 있지만 단위를 안 적어주면 px로 판단된다고 하네요.
확실하게 단위를 적어주는 게 가독성이 좋을 것 같습니다. 🙂

kutta97

지선님, 노션 클로닝 과제 하시느라 정말 수고 많으셨습니다!!! 지석 멘토님 말씀대로 확실히 지선님 코드가 점점 깔끔해지고 발전하고 있는다는 것이 느껴졌습니다ㅎㅎ

코드리뷰를 꼴찌로 했더니, 제가 하고 싶은 얘기들은 모두 리뷰로 먼저 달려서... 리뷰를 많이 못해드린 점 죄송합니다ㅠㅠ

kutta97 · 2023-07-10T18:28:15Z

src/Editor.js

+  onEditing,
+}) {
+  const $editor = document.createElement("div");
+  $editor.id = "editor";


id 를 사용할 명확한 이유가 없다면, className 을 사용하능 것이 더 좋다고 알고 있습니다!

https://dev.to/clairecodes/reasons-not-to-use-ids-in-css-4ni4

kutta97 · 2023-07-10T18:30:38Z

src/Editor.js

+    $editor.querySelector("[name=title]").value = this.state.title;
+    $editor.querySelector("[name=content]").value = this.state.content;


this.state 가 여러번 반복되는 것 같아서... 구조분해 할당을 사용해주시는 것은 어떨까요?

const { title, content } = this.state;

kutta97 · 2023-07-10T18:33:14Z

src/Editor.js

+
+  this.render = () => {
+    if (!isinitialize) {
+      $editor.innerHTML = `


저도 창욱님의 말씀에 동의합니다! 아무래도 동적으로 추가되고 삭제될 일이 없는 DOM element 는 컴포넌트 초기화 시점애 미리 만들어두는 것이 좋을것 같다고 생각합니다!

kutta97 · 2023-07-10T18:35:23Z

src/PostEditPage.js

+        if (this.state.postId) {
+          await request(`/documents/${this.state.postId}`, {
+            method: "PUT",
+            body: JSON.stringify(post),
+          });
+          pushRouter(`/${this.state.postId}`);
+          await getDocument();


이 부분도 postid 를 구조분해하여 꺼내 사용하면 가독성이 더 좋아질 것 같습니다!

kutta97 · 2023-07-10T18:38:09Z

src/PostItem.js

+  const $addButton = document.createElement("button");
+  $addButton.textContent = "+";
+  $addButton.className = id;
+  $li.appendChild($addButton);


button 을 생성하는 로직이 반복되는 것 같습니다! 이 부분은 따로 함수화하거나, Button 컴포넌트를 만들어 사용해주시면 좋을 것 같습니다!

Lim-JiSeon added 6 commits July 6, 2023 15:27

[FEAT] 과제 세팅 완료 및 API 수정

67775ff

[FIX] Router 수정

53e14ec

[FIX] API 통신 수정

e741067

[FIX] API 오류 해결

03f954a

[FIX] 상태 관리 오류 해결 & 하위 문서 생성

21ab7c4

[FEAT] 하위 문서 수정 작업

32d75bc

Lim-JiSeon self-assigned this Jul 6, 2023

Lim-JiSeon added 🔥 매운맛 지석팀 피드백 요청 과제 리뷰 요청 labels Jul 6, 2023

Lim-JiSeon added 17 commits July 7, 2023 00:31

[FIX] console.log 삭제

7668c85

[FEAT] 하위 문서 생성 및 바로가기 이동

a4ff1d7

[FEAT] storage 삭제

b14c73e

[FEAT] 한 화면에 목록과 편집기 구현

d3f3b9e

[FIX] file rename

037630a

[FIX] LinkButton 파일 삭제 및 파일 생성 기능 수정

612fa32

[FIX] API 정리

722e43a

[FIX] editor 수정

80933e5

[FEAT] 루트 파일 생성 코드 수정

631e92f

[FIX] postEdit 새로운 문서 생성 로직 수정

59b36a6

[FEAT] 글 수정시 네비바 업데이트 기능 추가

35b68fd

[FIX] 함수 이름 변경

6853d66

[FIX] 문서 트리 구조 구현

09e3956

[FIX] id -> classname으로 통일

e7b3dff

[FIX] 코드 정리

bded9fe

[STYLE] 디자인 구현

9c3557f

[FIX] element 추가 수정중(미해결)

0e0fec1

arclien reviewed Jul 9, 2023

View reviewed changes

wukdddang reviewed Jul 10, 2023

View reviewed changes

hayamaster reviewed Jul 10, 2023

View reviewed changes

howons reviewed Jul 10, 2023

View reviewed changes

kutta97 approved these changes Jul 10, 2023

View reviewed changes

Lim-JiSeon added 2 commits July 19, 2023 17:45

[FIX] 랜더링 오류 및 피드백 반영(1)

b6a613a

[FIX] 오류 수정 완료

ae82433

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mission5/임지선] Project_Notion_VanillaJs 과제 #50

[Mission5/임지선] Project_Notion_VanillaJs 과제 #50

Lim-JiSeon commented Jul 6, 2023 •

edited

Loading

arclien left a comment

arclien Jul 9, 2023

arclien Jul 9, 2023

arclien Jul 9, 2023

arclien Jul 9, 2023

arclien Jul 9, 2023

arclien Jul 9, 2023

arclien Jul 9, 2023

arclien Jul 9, 2023

arclien Jul 9, 2023

wukdddang left a comment

wukdddang Jul 10, 2023

wukdddang Jul 10, 2023

wukdddang Jul 10, 2023

sscoderati Jul 11, 2023

wukdddang Jul 10, 2023

kutta97 Jul 10, 2023

wukdddang Jul 10, 2023

hayamaster left a comment

hayamaster Jul 10, 2023

hayamaster Jul 10, 2023

hayamaster Jul 10, 2023

hayamaster Jul 10, 2023

howons left a comment

howons Jul 10, 2023

howons Jul 10, 2023

kutta97 left a comment

kutta97 Jul 10, 2023

kutta97 Jul 10, 2023

kutta97 Jul 10, 2023

kutta97 Jul 10, 2023

kutta97 Jul 10, 2023

		$editor.querySelector("[name=title]").value = this.state.title;
		$editor.querySelector("[name=content]").value = this.state.content;

[Mission5/임지선] Project_Notion_VanillaJs 과제 #50

Are you sure you want to change the base?

[Mission5/임지선] Project_Notion_VanillaJs 과제 #50

Conversation

Lim-JiSeon commented Jul 6, 2023 • edited Loading

📌 과제 설명

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

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

arclien 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

wukdddang 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

hayamaster 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

howons left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kutta97 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

Lim-JiSeon commented Jul 6, 2023 •

edited

Loading