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

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

Open
wants to merge 25 commits into
base: 4/#5_limjiseon
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
67775ff
[FEAT] 과제 세팅 완료 및 API 수정
Lim-JiSeon Jul 6, 2023
53e14ec
[FIX] Router 수정
Lim-JiSeon Jul 6, 2023
e741067
[FIX] API 통신 수정
Lim-JiSeon Jul 6, 2023
03f954a
[FIX] API 오류 해결
Lim-JiSeon Jul 6, 2023
21ab7c4
[FIX] 상태 관리 오류 해결 & 하위 문서 생성
Lim-JiSeon Jul 6, 2023
32d75bc
[FEAT] 하위 문서 수정 작업
Lim-JiSeon Jul 6, 2023
7668c85
[FIX] console.log 삭제
Lim-JiSeon Jul 6, 2023
a4ff1d7
[FEAT] 하위 문서 생성 및 바로가기 이동
Lim-JiSeon Jul 6, 2023
b14c73e
[FEAT] storage 삭제
Lim-JiSeon Jul 6, 2023
d3f3b9e
[FEAT] 한 화면에 목록과 편집기 구현
Lim-JiSeon Jul 6, 2023
037630a
[FIX] file rename
Lim-JiSeon Jul 6, 2023
612fa32
[FIX] LinkButton 파일 삭제 및 파일 생성 기능 수정
Lim-JiSeon Jul 6, 2023
722e43a
[FIX] API 정리
Lim-JiSeon Jul 7, 2023
80933e5
[FIX] editor 수정
Lim-JiSeon Jul 7, 2023
631e92f
[FEAT] 루트 파일 생성 코드 수정
Lim-JiSeon Jul 7, 2023
59b36a6
[FIX] postEdit 새로운 문서 생성 로직 수정
Lim-JiSeon Jul 7, 2023
35b68fd
[FEAT] 글 수정시 네비바 업데이트 기능 추가
Lim-JiSeon Jul 7, 2023
6853d66
[FIX] 함수 이름 변경
Lim-JiSeon Jul 7, 2023
09e3956
[FIX] 문서 트리 구조 구현
Lim-JiSeon Jul 7, 2023
e7b3dff
[FIX] id -> classname으로 통일
Lim-JiSeon Jul 7, 2023
bded9fe
[FIX] 코드 정리
Lim-JiSeon Jul 7, 2023
9c3557f
[STYLE] 디자인 구현
Lim-JiSeon Jul 7, 2023
0e0fec1
[FIX] element 추가 수정중(미해결)
Lim-JiSeon Jul 7, 2023
b6a613a
[FIX] 랜더링 오류 및 피드백 반영(1)
Lim-JiSeon Jul 19, 2023
ae82433
[FIX] 오류 수정 완료
Lim-JiSeon Jul 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<html>
<head>
<title>Notion Project</title>
</head>
<body>
<main id="app"></main>
<link rel="stylesheet" href="style.css" />
<script src="/src/main.js" type="module"></script>
</body>
</html>
35 changes: 35 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import PostEditPage from "./PostEditPage.js";
import PostNavBar from "./PostNavBar.js";
import { initRouter } from "./router.js";

export default function App({ $target }) {
const postNavBar = new PostNavBar({
$target,
});

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

Choose a reason for hiding this comment

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

지선님 추측대로, 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

스크린샷 2023-07-09 오후 4 04 16

Choose a reason for hiding this comment

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

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


this.route = () => {
$target.innerHTML = "";
const { pathname } = window.location;

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

Choose a reason for hiding this comment

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

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

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

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

postNavBar.setState();
};

this.route();

initRouter(() => this.route());
}
50 changes: 50 additions & 0 deletions src/Editor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
export default function Editor({
$target,
initialState = {
title: "",
content: "",
},
onEditing,
}) {
const $editor = document.createElement("div");
$editor.id = "editor";
Copy link

Choose a reason for hiding this comment

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

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

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


let isinitialize = false;
Copy link

Choose a reason for hiding this comment

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

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


this.state = initialState;

$target.appendChild($editor);

this.setState = (nextState) => {
this.state = nextState;
$editor.querySelector("[name=title]").value = this.state.title;
$editor.querySelector("[name=content]").value = this.state.content;
Copy link

Choose a reason for hiding this comment

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

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

const { title, content } = this.state;

this.render();
};

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

Choose a reason for hiding this comment

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

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

Copy link

Choose a reason for hiding this comment

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

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

<input type="text" name="title" value="${this.state.title}" />
<textarea name="content">${this.state.content}</textarea>
`;
isinitialize = true;
}
};
this.render();

$editor.addEventListener("keyup", (e) => {
const { target } = e;

const name = target.getAttribute("name");

if (this.state[name] !== undefined) {
const nextState = {
...this.state,
[name]: target.value,
};
this.setState(nextState);
onEditing(this.state);
}
});
}
73 changes: 73 additions & 0 deletions src/PostEditPage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { request } from "./api.js";
import Editor from "./Editor.js";
import { pushRouter } from "./router.js";

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

Choose a reason for hiding this comment

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

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


this.state = initialState;

let timer = null;

const editor = new Editor({
$target: $page,
initialState: {
title: "",
content: "",
},
onEditing: (post) => {
if (timer !== null) {
clearTimeout(timer);
}

timer = setTimeout(async () => {
if (this.state.postId) {
await request(`/documents/${this.state.postId}`, {
method: "PUT",
body: JSON.stringify(post),
});
pushRouter(`/${this.state.postId}`);
await getDocument();
Copy link

Choose a reason for hiding this comment

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

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

}
}, 2000);

Choose a reason for hiding this comment

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

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

오프 멘토님 리뷰

Copy link
Member

Choose a reason for hiding this comment

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

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

},
});

this.setState = async (nextState) => {
if (this.state.postId !== nextState.postId) {
this.state = nextState;
await getDocument();
return;
} else {
this.state = nextState;
this.render();
}

editor.setState(
this.state.post || {
title: "",
content: "",
}
);
};

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

Choose a reason for hiding this comment

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

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

};

const getDocument = async () => {
const { postId } = this.state;

if (postId) {
const post = await request(`/documents/${postId}`);

this.setState({
...this.state,
post,
});
}
};
}
55 changes: 55 additions & 0 deletions src/PostItem.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { request } from "./api.js";
import { pushRouter } from "./router.js";

export default function PostItem(title, id) {
const $postItemBox = document.createElement("div");
$postItemBox.className = id;

const $li = document.createElement("li");
$li.className = id;

const $title = document.createElement("span");
$title.className = id;
$title.textContent = title;
$li.appendChild($title);

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

Choose a reason for hiding this comment

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

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


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

const $postSubItemBox = document.createElement("ul");

$postItemBox.appendChild($li);
$postItemBox.append($postSubItemBox);

$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,
}),
});

Choose a reason for hiding this comment

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

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

pushRouter(`/${createdPost.id}`);
});

$removeButton.addEventListener("click", async () => {
await request(`/documents/${$removeButton.className}`, {
method: "DELETE",
});
pushRouter(`/`);
});
Copy link

Choose a reason for hiding this comment

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

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

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

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


return { $postItemBox, $postSubItemBox };
}
32 changes: 32 additions & 0 deletions src/PostList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import PostItem from "./PostItem.js";

export default function PostList({ $target, initialState }) {
const $postList = document.createElement("ul");
$target.appendChild($postList);

this.state = initialState;

this.setState = (nextState) => {
this.state = nextState;
this.render();
};

this.makeList = ($wrap, data) => {
Copy link

Choose a reason for hiding this comment

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

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

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

data.forEach(({ title, documents, id }) => {
const { $postItemBox, $postSubItemBox } = PostItem(title, id);

$wrap.appendChild($postItemBox);

if (documents.length > 0) {
this.makeList($postSubItemBox, documents);
}
});
};

this.render = () => {
$postList.innerHTML = "";
this.makeList($postList, this.state);
};

this.render();
}
43 changes: 43 additions & 0 deletions src/PostNavBar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { request } from "./api.js";
import PostList from "./PostList.js";
import { pushRouter } from "./router.js";

export default function PostNavBar({ $target }) {
Copy link

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

$createButton.id = "createButton";
$title.id = "title";

const postList = new PostList({
$target: $nav,
initialState: [],
});

this.setState = async () => {
const documents = await request("/documents");
postList.setState(documents);
this.render();
};

this.render = async () => {
$createButton.textContent = "문서 생성하기";
$title.textContent = "Notion Project";
$nav.appendChild($title);
$nav.appendChild($createButton);
Copy link

Choose a reason for hiding this comment

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

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

$target.appendChild($nav);
};

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

Choose a reason for hiding this comment

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

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

title: "제목 없음",
parent: null,
}),
});
pushRouter(`/${createdPost.id}`);
});
}
23 changes: 23 additions & 0 deletions src/api.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export const API_END_POINT = "https://kdt-frontend.programmers.co.kr";
export const API_X_USERNAME = "API_X_USERNAME_LIMJISEON";

export const request = async (url, options = {}) => {
try {
const response = await fetch(`${API_END_POINT}${url}`, {
...options,
headers: {
"Content-Type": "application/json",
"x-username": API_X_USERNAME,
},
});

if (response.ok) {
return await response.json();
}

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

Choose a reason for hiding this comment

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

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

}
};
6 changes: 6 additions & 0 deletions src/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import App from "./App.js";

const $target = document.querySelector("#app");
$target.id = "contentWrap";

new App({ $target });
22 changes: 22 additions & 0 deletions src/router.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const ROUTE_CHANGE_EVENT_NAME = "route-change";

export const initRouter = (onRoute) => {
window.addEventListener(ROUTE_CHANGE_EVENT_NAME, (e) => {
const { nextUrl } = e.detail;

if (nextUrl) {
history.pushState(null, null, nextUrl);
onRoute();
}
});
};
Copy link

Choose a reason for hiding this comment

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

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


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

Choose a reason for hiding this comment

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

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

detail: {
nextUrl,
},
})
);
};
Loading