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

[Mission4/유지영] - Project_Notion_VanillaJS #8

Open
wants to merge 21 commits into
base: 3/#4_jiyoung
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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 @@
apiUrl.js
1 change: 1 addition & 0 deletions icon/chevron-right-solid.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions icon/notion-logo.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<html lang="en">

Choose a reason for hiding this comment

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

사실 지금 그렇게 신경쓸 필요는 없지만!
lang과 mata tag들을 적절하게 바꿔는것도 좋다고 하네요~!!, 저도 (mata쪽을 변경해야겠어요)
(SEO 위해서..!)

<head>
<meta charset="UTF-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<link rel="preconnect" href="https://fonts.googleapis.com" />
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin />
<link
href="https://fonts.googleapis.com/css2?family=Noto+Sans+KR:wght@300&display=swap"
rel="stylesheet"
/>
<script src="/src/main.js" type="module"></script>
<link rel="stylesheet" href="/style/style.css" />
<title>노션 클로닝 프로젝트</title>
</head>
<body>
<main id="app"></main>
</body>
</html>
53 changes: 53 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import PostEditPage from "./Page/editPage/PostEditPage.js";
import StartPage from "./Page/startPage/StartPage.js";
import PostPage from "./Page/listPage/PostPage.js";
import { initRouter } from "./utils/router.js";
import instanceCheck from "./utils/instanceCheck.js";

export default function App({ $target }) {
instanceCheck(new.target);

Choose a reason for hiding this comment

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

저번 순요님의 코드를 잘 활용하신게 보기 좋습니다!


const $postListContainer = document.createElement("div");
const $postEditContainer = document.createElement("div");

$target.appendChild($postListContainer);
$target.appendChild($postEditContainer);
Comment on lines +10 to +14

Choose a reason for hiding this comment

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

이렇게 중복되는 로직은 하나로 묶어 줄여보는것도 좋을것 같아요!


const postPage = new PostPage({
$target: $postListContainer,
});

const startPage = new StartPage({
$target: $postEditContainer,
});

const postEditPage = new PostEditPage({
$target: $postEditContainer,
initialState: {
postId: "new",
posts: {
title: "",
content: "",
},
},
listUpdate: () => {

Choose a reason for hiding this comment

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

주로 동사를 먼저 배치하다 보니 updateList라는 말이 어울리는것 같아요!

postPage.render();
},
});

this.route = () => {
$postEditContainer.innerHTML = "";
const { pathname } = window.location;
if (pathname.indexOf("/documents/") === 0) {
const [, , postId] = pathname.split("/");
postEditPage.setState({ postId });
} else {
postPage.render();
startPage.render();
}
};

this.route();

initRouter(() => this.route());
}
50 changes: 50 additions & 0 deletions src/Page/editPage/Editor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import instanceCheck from "../../utils/instanceCheck.js";

/**편집기를 그리는 컴포넌트 */
export default function Editor({
$target,
initialState = {
title: "",
content: "",
},
onEditing,
}) {
instanceCheck(new.target);

const $editor = document.createElement("div");
$editor.classList.add("editor");
$target.appendChild($editor);

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 = () => {
$editor.innerHTML = `
<input type="text" name="title" class="title" value="${this.state.title}"/>
<textarea name="content" class="content" placeholder="내용을 입력하세요.">${this.state.content}</textarea>
`;
};

this.render();

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

Choose a reason for hiding this comment

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

(e) => 이부분에서 바로 ({target}) => 으로 구조 분해하는 방법도 좋을것 같아요!


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

if (this.state[name] !== undefined) {
const nextState = {
...this.state,
[name]: target.value,
};

this.setState(nextState);
onEditing(this.state);
}
});
}
55 changes: 55 additions & 0 deletions src/Page/editPage/MarkUpList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { push } from "../../utils/router.js";
import instanceCheck from "../../utils/instanceCheck.js";

/**편집기 아래에 해당 document의 하위 documents를 그리는 컴포넌트 */
export default function MarkUpList({ $target, initialState }) {
instanceCheck(new.target);

const $markUpList = document.createElement("div");
$markUpList.classList.add("markUpList");
$target.appendChild($markUpList);

this.state = initialState;

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

const markUpList = (list, text) => {
text += `
<ul>
${list
.map(
({ id, title, documents }) => `
<div class='documentsTree'>
<li data-id="${id}">
<img class="svg" src="../icon/chevron-right-solid.svg" />
${title}
</li>
${documents.map((document) => markUpList([document], text)).join("")}
</div>
`
)
.join("")}
</ul>
`;

Choose a reason for hiding this comment

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

저도 text를 더해가는 방식을 처음에 생각했었는데, 멘토님과 상우님 말씀대로 return하면서 구현해 봐야겠네요!


return text;
};
Copy link

Choose a reason for hiding this comment

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

text에 더하지 않고 return 하면 되지 않을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 그냥 return 해도 되겠군요...왠지 더해야겠다고만 생각했었는데!! 안 더하고 return 하는 방법으로 수정해보겠습니다 😄

Choose a reason for hiding this comment

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

아무래도 InnerHtml이 기존에 있는 Html을 삭제하니 text를 더하신것 같은데
return으로 바꾸고
innerHtml += 리턴된값
을 하시면 기존 Html값에다가 추가 됩니다!

Choose a reason for hiding this comment

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

또는 insertadjacenthtml을 이용해 보는것도 좋아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

그런 방법으로 변경하면 좋겠네요!! insertadjacenthtml에 대해서도 공부해봐야겠습니다! 의견 감사합니다 상우님 :)


this.render = () => {
const documentsList = markUpList(this.state, "");
$markUpList.innerHTML = `<div class="list">${documentsList}</div>`;
};

this.render();

$markUpList.addEventListener("click", (e) => {
const $li = e.target.closest("li");
const id = $li.dataset.id;
if ($li) {
push(`/documents/${id}`);
}
});
}
114 changes: 114 additions & 0 deletions src/Page/editPage/PostEditPage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { request } from "../../utils/api.js";
import Editor from "./Editor.js";
import MarkUpList from "./MarkUpList.js";
import { getItem, removeItem, setItem } from "../../utils/storage.js";
import instanceCheck from "../../utils/instanceCheck.js";

/**편집기의 내용을 서버에 저장하고 불러오는 컴포넌트 */
export default function PostEditPage({ $target, initialState, listUpdate }) {
instanceCheck(new.target);

const $page = document.createElement("div");
$page.classList.add("editPage");

this.state = initialState;

let postLocalSaveKey = `temp-post-${this.state.postId}`;

const editor = new Editor({
$target: $page,
initialState: {
title: "",
content: "",
},
onEditing: (post) => {
storageSave(post);
serverSave(post);
},
});

const markupList = new MarkUpList({
$target: $page,
initialState: [],
});

let storageTimer = null;
let serveTimer = null;
Copy link

Choose a reason for hiding this comment

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

serverTimer
오타인 것 같습니다ㅎㅎ


const storageSave = (post) => {
if (storageTimer !== null) {
clearTimeout(storageTimer);
}

Choose a reason for hiding this comment

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

밑에 serverSave 함수에서도 동일한 형태가 반복되는데, 함수로 빼서 argument만 받는 형태로 진행해보는 건 어떨까요? :)

Choose a reason for hiding this comment

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

또한 함수를 선언할 땐, saveStorage와 같은 형태로 선언하는 것이 좋을 것 같습니다 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

debounce 함수를 따로 빼서 만들어봐야겠네요! 이름도 수정해봐야겠습니다 좋은 의견 감사합니다 지연님 😊


storageTimer = setTimeout(() => {
setItem(postLocalSaveKey, {
...post,
tempSaveDate: new Date(),
});
}, 500);
};

const serverSave = (post) => {
if (serveTimer !== null) {
clearTimeout(serveTimer);
}

serveTimer = setTimeout(async () => {
await request(`/documents/${post.id}`, {
method: "PUT",
body: JSON.stringify(post),
});

removeItem(postLocalSaveKey);
listUpdate();

const data = await request(`/documents/${post.id}`);
markupList.setState(data);
Copy link

Choose a reason for hiding this comment

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

api가 두번 호출될 것 같은데요~! 수정이 필요할 듯 합니다

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 이 부분이 계속 마음에 걸리더라구요 ㅠㅠ 수정해보겠습니다!!

Copy link

Choose a reason for hiding this comment

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

현재 PostEditPage 컴포넌트의 setState를 보면 this.state의 postId를 항상 최신으로 유지하는 방어코드가 이미 작성이 되어 있어요! 그러니까 그냥 push() 한방만 때리면 금방 해결될 수 있을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

어떻게 해결할까 고민하고 있었는데 이런 방법이 있었다니!! 감사합니다 순요님 :)😊

}, 1000);
};

this.setState = async (nextState) => {
if (this.state.postId !== nextState.postId) {
postLocalSaveKey = `temp-post-${nextState.postId}`;
this.state = nextState;
await fetchPost();
return;
}
this.state = nextState;

this.render();

if (this.state.post) {
markupList.setState(this.state.post);
editor.setState(this.state.post);
}
};

const fetchPost = async () => {
const { postId } = this.state;
const post = await request(`/documents/${postId}`);
const tempPost = getItem(postLocalSaveKey, {
title: "",
content: "",
});

if (tempPost.tempSaveDate && tempPost.tempSaveDate > post.updatedAt) {
if (confirm("저장되지 않은 임시 데이터가 있습니다. 불러오시겠습니까?")) {

Choose a reason for hiding this comment

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

confirm의 결과값이 true false로 나오기때문에 윗 조건식에 &&이나 ||르 추가하셔도 될것 같습니다!

this.setState({
...this.state,
post: tempPost,
});
return;
}
}

this.setState({
...this.state,
post,
});
};

this.render = () => {
$target.appendChild($page);
};
}
34 changes: 34 additions & 0 deletions src/Page/listPage/Header.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { push } from "../../utils/router.js";
import instanceCheck from "../../utils/instanceCheck.js";

/**user와 email을 받아서 헤더로 그리는 컴포넌트 */
export default function Header({ $target, initialState }) {
instanceCheck(new.target);

const $header = document.createElement("div");
$header.classList.add("header");
$target.appendChild($header);

this.state = initialState;

this.render = () => {
const { user, email } = this.state;
$header.innerHTML = `
<img class="logo" src="../icon/notion-logo.svg" />
<div class="user">
<h3> ${user}의 notion</h3>
<h5>${email}</h5>
</div>
`;
};

this.render();

$header.addEventListener("click", (e) => {
const $user = e.target.closest(".header");
Copy link

@yrnana yrnana Nov 17, 2022

Choose a reason for hiding this comment

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

$user === $header 아닐까요? 조건문이 불필요한 것 같습니다


if ($user) {
push("/");
}
});
}
Loading