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 과제 #37

Open
wants to merge 15 commits into
base: 4/#5_judahhh
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ApiKey.js
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

15 changes: 15 additions & 0 deletions index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html>
<head>
<title>Notion_cloning</title>
<link rel="stylesheet" href="./src/style/style.css" />
<script
src="https://kit.fontawesome.com/0ab4707042.js"
crossorigin="anonymous"
></script>
</head>
Copy link
Member

@yohanpro yohanpro Jul 10, 2023

Choose a reason for hiding this comment

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

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

<body>
<main id="app"></main>
<script src="/src/index.js" type="module"></script>
</body>
</html>
52 changes: 52 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import DocumentPage from "../src/components/DocumentPage.js";
import DocumentEditPage from "../src/components/DocumentEditPage.js";
import { editorRoute } from "../src/utils/router.js";
import MainPage from "../src/pages/MainPage.js";
import { removeDiv } from "./utils/removeDocumentList.js";

export default function App({ $target }) {
const documentPage = new DocumentPage({
$target,
});

const documentEditPage = new DocumentEditPage({
$target,
initialState: {
documentId: null,
document: {
title: "",
content: "",
},
},
});

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

Choose a reason for hiding this comment

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

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

});

this.render = () => {
documentPage.render();
};

this.render();

this.route = (parent) => {
const { pathname } = window.location;
if (pathname === "/") {
removeDiv(".edit-page");
mainPage.render();
Copy link

Choose a reason for hiding this comment

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

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

} else {
removeDiv(".main-page");
const [, id] = pathname.split("/");
documentEditPage.setState({
documentId: id,
parentId: parent,
});
}
};

this.route();

editorRoute((parent) => this.route(parent));
}
19 changes: 19 additions & 0 deletions src/api/Api.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import API_BASE_URL from "../utils/ApiKey.js";

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

if (res.ok) return await res.json();

throw new Error("API 처리 실패");
} catch (error) {
alert(error.message);
}
};
116 changes: 116 additions & 0 deletions src/components/DocumentEditPage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import Editor from "./Editor.js";
import { setItem, getItem, removeItem } from "../utils/storage.js";
import { request } from "../api/api.js";
import { push } from "../utils/router.js";

export default function DocumentEditPage({ $target, initialState }) {
const $editPage = document.createElement("div");
$editPage.className = "edit-page";
this.state = initialState;

let documentLocalSaveKey = `temp-document-${this.state.documentId}`;
Copy link
Member

Choose a reason for hiding this comment

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

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

this.getDocumentLocalSaveKey =()=> {
  return `temp-document-${this.state.documentId}`;
};

const tempDocument = getItem(this.getDocumentLocalSaveKey(), {
  title: "",
  content: "",
});

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


const tempDocument = getItem(documentLocalSaveKey, {
title: "",
content: "",
});

let timer = null;

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

timer = setTimeout(async () => {
Copy link
Member

Choose a reason for hiding this comment

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

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

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

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

setItem(documentLocalSaveKey, {
...document,
tempSaveData: new Date(),
});

await request(`/${this.state.documentId}`, {
method: "PUT",
body: JSON.stringify(document),
});

if (this.state.document.title) {
if (this.state.document.title !== document.title) {
Copy link

Choose a reason for hiding this comment

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

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

push({
type: null,
id: null,
});
}
}
removeItem(documentLocalSaveKey);
}, 500);
},
});

this.setState = async (nextState) => {
if (this.state.documentId !== nextState.documentId) {
Copy link
Member

Choose a reason for hiding this comment

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

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

const {document, documentId} = this.state;

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

documentLocalSaveKey = `temp-document-${this.state.documentId}`;
this.state = nextState;

if (this.state.documentId === "new") {
editor.setState(
this.state.document || {
title: "",
content: "",
}
);
} else {
await fetchDocument();
}
Comment on lines +59 to +69
Copy link
Member

Choose a reason for hiding this comment

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

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();
}


return;
}

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

Choose a reason for hiding this comment

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

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

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

this.render();
};

this.render = () => {
$target.appendChild($editPage);
};

const fetchDocument = async () => {
const { documentId } = this.state;

if (this.state.documentId !== null) {
Copy link
Member

Choose a reason for hiding this comment

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

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

const document = await request(`/${documentId}`);

const getTempDocument = getItem(documentLocalSaveKey, {
Copy link
Member

Choose a reason for hiding this comment

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

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

title: "",
content: "",
});

this.setState({
...this.state,
document,
});
}
};
}
54 changes: 54 additions & 0 deletions src/components/DocumentList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { displayDocumentList } from "../utils/displayDocumentList.js";
import { push } from "../utils/router.js";
import { setItem, getItem, removeItem } from "../utils/storage.js";

export default function PostList({ $target, initialState }) {
const $documentList = document.createElement("div");
$documentList.className = "document-list";
$target.appendChild($documentList);

this.state = initialState;

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

this.render = () => {
$documentList.innerHTML = `
<button name ="add-btn" data-id = "null" class="root-page-add" >페이지 추가</button>
<div class ="list-box">
${displayDocumentList(this.state)}
</div>
`;
};

this.render();

$documentList.addEventListener("click", (e) => {
const { target } = e;
const element = target.closest("[name]");
Comment on lines +28 to +30
Copy link
Member

Choose a reason for hiding this comment

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

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]");


if (element) {
const id = element.dataset.id;
Copy link
Member

Choose a reason for hiding this comment

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

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

Suggested change
const id = element.dataset.id;
const {id} = element.dataset;

const listToggleState = `isOpened-${id}`;
const name = element.getAttribute("name");
if (target.className === "toggle-btn") {
const getToggleState = getItem(listToggleState);
getToggleState
? removeItem(listToggleState)
: setItem(listToggleState, "block");
this.render();

return;
}

if (name) {
push({
type: name,
id,
});
}
}
});
}
36 changes: 36 additions & 0 deletions src/components/DocumentPage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import DocumentList from "./DocumentList.js";
import { request } from "../api/api.js";
import Header from "./Header.js";
import { listRoute } from "../utils/router.js";

//Sidebar
export default function documentPage({ $target }) {
const $documentPage = document.createElement("div");
$documentPage.className = "document-page";

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

Choose a reason for hiding this comment

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

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

});

const documentList = new DocumentList({
$target: $documentPage,
initialState: [],
});

const fetchDocuments = async () => {
const documents = await request("/");
documentList.setState(documents);
};

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

Choose a reason for hiding this comment

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

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

};

this.route = () => {
this.setState();
};

listRoute(() => fetchDocuments());
}
48 changes: 48 additions & 0 deletions src/components/Editor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
export default function Editor({
$target,
initialState = {
titile: "",
content: "",
},
onEditing,
}) {
const $editor = document.createElement("div");
$editor.className = "editor";
$target.appendChild($editor);
let isinitialize = false;
Copy link
Member

Choose a reason for hiding this comment

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

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

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


this.state = initialState;

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

this.render = () => {
if (!isinitialize) {
$editor.innerHTML = `
<input type ="text" name = "title" class="editor-title" placeholder ="제목없음" value = "${this.state.title}"/>
<textarea name ="content" class="editor-content" placeholder ="여기에 내용을 입력해 주세요.">${this.state.content}</textarea>
`;
isinitialize = true;
}
};

this.render();

$editor.addEventListener("keypress", (e) => {
Copy link
Member

Choose a reason for hiding this comment

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

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

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

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);
}
});
}
23 changes: 23 additions & 0 deletions src/components/Header.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { push } from "../utils/router.js";

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

Choose a reason for hiding this comment

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

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

const $header = document.createElement("h3");
$target.appendChild($header);
this.state = initialState;

this.render = () => {
$header.innerHTML = `🧡 ${this.state}'s Notion 🧡`;
};

this.render();

$header.addEventListener("click", (e) => {
const { target } = e;
if (target) {
push({
type: "header",
id: null,
});
}
});
}
5 changes: 5 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import App from "./App.js";

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

new App({ $target });
14 changes: 14 additions & 0 deletions src/pages/MainPage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export default function MainPage({ $target, initialState }) {
const $mainPage = document.createElement("div");
$mainPage.className = "main-page";

this.state = initialState;

$mainPage.innerHTML = `<div class ="main-title">${this.state}님, Notion에 오신 것을 환영합니다.</div>

<div class="main-content">문서를 추가하여 작업을 시작해보세요</div>`;

this.render = () => {
$target.appendChild($mainPage);
};
}
Loading