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

Open
wants to merge 42 commits into
base: 4/#5_jgjgill
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
bc9b44a
feat: 기본 폴더 및 파일 세팅
jgjgill Jun 30, 2023
f60c9cb
feat: 문서 추가 기능 구현
jgjgill Jul 1, 2023
4edc593
feat: 모든 문서 불러오기 기능 구현
jgjgill Jul 1, 2023
78870f8
feat: 특정 문서 읽기 기능 구현
jgjgill Jul 1, 2023
d0db2fd
feat: 문서 수정 기능 구현
jgjgill Jul 1, 2023
7d7660f
feat: 문서 트리 형태로 불러오기 기능 구현
jgjgill Jul 1, 2023
134fb2c
fix: 중복 이벤트 발생 에러 수정
jgjgill Jul 1, 2023
5f3005e
feat: 문서 삭제 기능 구현
jgjgill Jul 2, 2023
1357324
fix: 문서 추가 방식 변경
jgjgill Jul 2, 2023
64a6d5c
refactor: 라우팅 방식 변경
jgjgill Jul 2, 2023
6d490c6
feat: 문서 수정 기능 변경
jgjgill Jul 2, 2023
502b74d
refactor: 코드 중복 제거 및 추상화 작업
jgjgill Jul 2, 2023
5e81344
chore: Edit 파일 Document로 이름 변경
jgjgill Jul 2, 2023
ac3e47b
feat: 재귀 관련 템플릿 작업
jgjgill Jul 2, 2023
66716bc
feat: 자식 문서로 이동 기능 구현
jgjgill Jul 2, 2023
b989893
chore: Document 관련 이름 변경
jgjgill Jul 3, 2023
7e5e929
feat: api 요청에 따른 상태 업데이트하도록 임시 구현
jgjgill Jul 3, 2023
c51e413
feat: 상태 관리 방식으로 변경 작업
jgjgill Jul 4, 2023
a522eb2
feat: 자식 문서 추가시 상태 최신화 구현
jgjgill Jul 4, 2023
7809c08
fix: 수정 및 제거 관련 자식 문서 최신화하도록 변경
jgjgill Jul 4, 2023
9d47d3a
refactor: 현재 문서 삭제시 홈으로 이동하는 방식으로 변경
jgjgill Jul 4, 2023
3c38e75
fix: 라우팅 관련 작업
jgjgill Jul 4, 2023
0778f63
feat: 자동 완성 및 최근 검색 기능 구현
jgjgill Jul 4, 2023
6918089
chore: 개행 및 안쓰는 코드 정리
jgjgill Jul 5, 2023
97c35be
style: 스타일 관련 작업
jgjgill Jul 5, 2023
f619831
fix: 문서 삭제 관련 로직 변경
jgjgill Jul 5, 2023
0ba048e
fix: 타이틀 변경시 에디터 상태로 변경하도록 로직 변경
jgjgill Jul 5, 2023
931b895
style: Home, Editor 관련 스타일 작업
jgjgill Jul 5, 2023
8872665
feat: NotFound 관련 작업
jgjgill Jul 5, 2023
13cb534
refactor: 자잘한 리팩토링 진행
jgjgill Jul 5, 2023
0979e02
feat: 검색 관련 포커스 기능 추가
jgjgill Jul 5, 2023
6428239
fix: 자식 문서 여백 문제 관련 작업
jgjgill Jul 5, 2023
6bc441b
feat: 토글 기능 구현
jgjgill Jul 5, 2023
f281d38
refactor: 디바운스 함수 분리
jgjgill Jul 6, 2023
d5f507a
feat: 툴팁 기능 구현
jgjgill Jul 6, 2023
c1870b6
refactor: 파일 구조 변경 및 코드 수정
jgjgill Jul 6, 2023
f29f3a1
style: 에디터에서의 자식 문서 스타일 작업
jgjgill Jul 6, 2023
6100dc0
chore: 오타 수정
jgjgill Jul 6, 2023
00d57ac
fix: 스크롤 초기화되는 문제 관련 작업
jgjgill Jul 6, 2023
f229c64
refactor: 변수명 변경
jgjgill Jul 17, 2023
b93a98a
fix: 에러 관련 작업
jgjgill Jul 17, 2023
6c2a351
style: 토글 디폴트값 변경
jgjgill Jul 17, 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
node_modules/
13 changes: 13 additions & 0 deletions index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!DOCTYPE html>
<html lang="ko">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Jongtion</title>
<link rel="stylesheet" href="/src/styles/styles.css" />
</head>
<body>
<main id="app"></main>
<script src="/src/main.js" type="module"></script>
</body>
</html>
14 changes: 14 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "FEDC4-5_Project_Notion_VanillaJS",
"version": "1.0.0",
"main": "index.js",
"repository": "https://github.com/prgrms-fe-devcourse/FEDC4-5_Project_Notion_VanillaJS.git",
"author": "jgjgill <[email protected]>",
"license": "MIT",
"scripts": {
"dev": "yarn serve -s"
},
"devDependencies": {
"serve": "^14.2.0"
}
}
147 changes: 147 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import { getDocuments, putDocument } from "./api/document.js";
import Layout from "./components/common/Layout.js";
import { PATH } from "./constants/path.js";
import { initRouter, push } from "./utils/route.js";
import {
TrieDocument,
addChildDocument,
editTitleDocument,
insertAllDocument,
findChildDocuments,
removeDocument,
} from "./utils/document.js";
import NotFound from "./components/common/NotFound.js";
import { debounce } from "./utils/debounce.js";
import DocumentList from "./components/domain/document/DocumentList.js";
import DocumentEditor from "./components/domain/document/DocumentEditor.js";
import Home from "./components/domain/home/Home.js";
import RecurDocumentList from "./components/domain/document/template/RecurDocumentList.js";

/**
* @param {{appElement: Element | null}}
*/

export default function App({ appElement }) {
if (!new.target) return new App(...arguments);

Choose a reason for hiding this comment

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

와 이걸 깜빡하고 있었네요! 저도 리팩토링할 때에 추가해줘야겠습니다.

Choose a reason for hiding this comment

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

오 디테일 하시네요!! 👍

Copy link
Member Author

@jgjgill jgjgill Jul 13, 2023

Choose a reason for hiding this comment

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

저도 모든 컴포넌트에는 추가하지 못했습니다.. 😂


const wrapperContainer = document.createElement("div");
const leftContainerElement = document.createElement("div");
const rightContainerEleement = document.createElement("div");
const leftListElement = document.createElement("div");

Choose a reason for hiding this comment

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

TMI

저도 실제로 left와 right를 자주 사용하긴 하는데.
안드로이드 등에서는 컴포넌트를 만들 때 left, right 대신 start, end 라는 용어를 쓰더라구요.
그 이유가 무엇인지 아시나요?

바로 언어 호환 때문인데요! 중동어의 경우 시작이 오른쪽에서 왼쪽으로 흘러가기 때문에
왼쪽 정렬이라는 말을 쓰게 된다면 이게 시작점을 의미하는 것인지? 끝 점을 이야기 하는 것인지 혼선이 있을 수 있기 때문이에요!

여기서 strat, end를 쓰라는 뜻은 아니였고 멀티 랭기지를 챙기게 되면 이런 혼선이 있을 수 있다!! 정도로
그냥.. TMI 로 말씀 드려봐요.. ㅋㅋㅋ

다만 여기서는 left, right 보다는 sidebar, contents(or main) 등 시멘틱한 용어를 써서 구분을 지어도 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

오오..! 전혀 생각지도 못한 부분이였네요..! 😂
좋은 꿀팁 감사합니다!!


wrapperContainer.className = "wrapper-container";
leftContainerElement.className = "left-container";
rightContainerEleement.className = "right-container";
leftListElement.className = "left-list-container";
Copy link

Choose a reason for hiding this comment

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

클래스명을 이렇게도 줄 수 있군요! 이렇게 주면 어떤점이 좋다고 생각하시나요?! innerHTML을 안써도 되는 점일까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 요소는 따로 렌더링하기 위한 내부 요소들이 필요하지 않아서 className으로 준 것 같네요..!
비슷한 코드끼리 묶어두면 구분하기에도 용이하구요!! 😆


const trie = new TrieDocument();

const processEdit = debounce(async (documentId, docunemt) => {
await putDocument({ documentId, data: docunemt });
}, 1000);

this.state = [];

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

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

const layoutComponent = new Layout({ parentElement: leftContainerElement });

Choose a reason for hiding this comment

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

layout Component를 따로 분리하셨는데 의도가 궁금합니다! 다른 컴포넌트에서도 사용될 것을 염두해서 분리하신건가요?🤔

Copy link
Member Author

@jgjgill jgjgill Jul 13, 2023

Choose a reason for hiding this comment

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

노션 구조에 대해 계속 생각하면서 막상 변경되는 것은 컨텐츠 부분만 해당하더라구요.
그래서 처음에는 레이아웃 부분이 많을 것 같아서 이렇게 구성하게 된 것 같네요..! 🧐


const documentListComponent = new DocumentList({
parentElement: leftListElement,
renderItemComponent: (parentElement) => {
RecurDocumentList({
rootDocuments: this.state,
parentElement,
childRender: (parentId, newDocument) => {
const nextState = addChildDocument(parentId, this.state, newDocument);
this.setState(nextState);
},
removeRender: (documentId) => {
const newState = removeDocument(documentId, this.state);
const stringDocumentId = window.location.pathname.split("/")[2];

Number(stringDocumentId) !== documentId
? this.editorSetState(newState)
: push(PATH.HOME);

this.setState(newState);
},
depthCount: 1,
});
},
onAddButtonClick: (newDocument) => {
const nextState = [...this.state, newDocument];
this.setState(nextState);
},
});

const homeComponent = new Home({
parentElement: rightContainerEleement,
search: (text) => trie.search(text),
});

const documentEditorComponent = new DocumentEditor({
parentElement: rightContainerEleement,
onEditing: (document) => {
const { documentId, title, isChangeTitle } = document;

if (isChangeTitle) {
const newState = editTitleDocument(documentId, this.state, title);
this.setState(newState);
}

processEdit(documentId, document);
},
getChildDocuments: (documentId) =>
findChildDocuments(this.state, documentId),
});

const notFoundComponent = new NotFound({
parentCompoent: rightContainerEleement,
});

window.addEventListener("popstate", () => {
this.route();
});

initRouter(() => this.route());

this.init = async () => {

Choose a reason for hiding this comment

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

이렇게 Init 함수를 만든 것 너무 좋네요~~ 👍

appElement.append(wrapperContainer);
wrapperContainer.append(leftContainerElement, rightContainerEleement);

layoutComponent.render();
leftContainerElement.append(leftListElement);

const newState = await getDocuments();
this.setState(newState);

insertAllDocument(newState, (title, id) => trie.insert(title, id));

this.route();
};

this.route = () => {
const { pathname } = window.location;
rightContainerEleement.innerHTML = ``;

if (pathname === PATH.HOME) {
trie.reset();
insertAllDocument(this.state, (title, id) => trie.insert(title, id));

homeComponent.render();
} else if (pathname.split("/")[1] === "documents") {
documentEditorComponent.render();
} else {
notFoundComponent.render();
}
Comment on lines +138 to +147

Choose a reason for hiding this comment

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

P2;

추후 서비스가 확장된다면 다양한 url이 추가될 것 같은데 그때 else if 문이 반복된다면 코드를 파악하는데 오랜 시간을 써야할 것 같다는 생각이 들었어요.

switch 문을 써보는 것은 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

네넵..! 해당 부분은 변경이 필요해 보이네요!! 참고하겠습니다! 😆

};
}
30 changes: 30 additions & 0 deletions src/api/document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { PATH } from "../constants/path.js";

Choose a reason for hiding this comment

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

오 이렇게 관심사 별로 fetch 함수들을 따로 빼는 것 너무 좋네요!! 👍

중복되는 값들은 별도의 상수로 관리하는 것도 너무 좋구요!! 👍👍

import { request } from "./request.js";

export const postDocument = async (data) => {
return await request(PATH.DOCUMENTS, {
method: "POST",
body: JSON.stringify(data),
});
};

export const getDocuments = async () => {
return await request(PATH.DOCUMENTS);
};

export const getDocument = async (documentId) => {
return await request(`${PATH.DOCUMENTS}/${documentId}`);
};

export const putDocument = async ({ documentId, data }) => {
return await request(`${PATH.DOCUMENTS}/${documentId}`, {
method: "PUT",
body: JSON.stringify(data),
});
};

export const deleteDocument = async (documentId) => {
return await request(`${PATH.DOCUMENTS}/${documentId}`, {
method: "DELETE",
});
};
25 changes: 25 additions & 0 deletions src/api/request.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { PATH } from "../constants/path.js";
import { push } from "../utils/route.js";

const API_END_POINT = "https://kdt-frontend.programmers.co.kr";

export const request = async (url, options = {}) => {

Choose a reason for hiding this comment

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

P5;

url은 보통 https~~ 가 포함된 것을 의미하기 때문에 지금의 경우 path라고 정의하는 것이 더 적절할 것 같아요!
image

Copy link
Member Author

Choose a reason for hiding this comment

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

그렇군요!! 아무 생각없이 반복적으로 url이라는 단어를 사용하고 있었네요!!
이렇게 하나 또 배워갑니다..! 🧐

Choose a reason for hiding this comment

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

추가로 요런 request 함수를 어떻게 말아서 쓰면 좋을까? 고민이 되신다면

axios 또는 ky 라이브러리는 어떻게 쓰라고 되어있는지?
왜 이렇게 쓰게끔 했는지?
만약 fetch 함수로 ky 또는 axios를 만들려면 어떻게 할 수 있을지?

쓰윽 고민해보시면 좋을 것 같아요!

TMI

  • axios는 fetch 이전의 예에에전 방식의 xmlhttprequest 방식을 쓰고 있어서 fetch와 같은 역할을 모두 지원하기 위해 번들 자체가 느리고 무겁지만 다양한 미들웨어를 제공하고 유니버셜하게 쓸 수 있어서 많이 사용되는 라이브러리에요.
  • ky의 경우 fetch 기반으로 되어있어서 매우 가볍고 편하지만 interceptor 와 같은 미들웨어를 지원하지 않아요.

Copy link
Member Author

Choose a reason for hiding this comment

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

네넵..! 해당 라이브러리 참고해보겠습니다!
감사합니다..! 🙇‍♂️

try {
const res = await fetch(`${API_END_POINT}${url}`, {
...options,
headers: {
"Content-Type": "application/json",
"x-username": "jgjgill",
},
});

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

throw new Error("API 에러가 발생했습니다!");
Comment on lines +16 to +20

Choose a reason for hiding this comment

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

P3;

res.ok가 아닌 경우에는 어떻게 핸들링 하나요?
4xx번대 에러와 5xx번대 에러의 경우 각기 다르게 핸들링 해야할 것 같은데 여기서는 throw new Error("API 처리 중 이상 발생"); 로 퉁쳐지는 것 같아서요!

ask; 퀴즈

request 함수 관점에서
서버에서 정상적으로 응답이 온 경우!
특히 유저가 실수 해서 난 Error인 4xx번대 에러인 경우 이를 코드상 에러라고 보는 것이 맞을까요?
아니면 정상적으로 동작하는 것으로 보는게 좋을까요?
만약 정상이라고 본다면 어떻게 처리하는 것이 좋을까요?

Copy link
Member Author

@jgjgill jgjgill Jul 13, 2023

Choose a reason for hiding this comment

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

에러에 대해서 깊게 생각하지 않고 하나로 퉁쳤네요. 😂
말씀하신 것처럼 코드상 에러가 아닌 경우도 있어서 생각을 더 해봐야 되겠네요..
에러 핸들링과 관련해서도 코드를 짜보는 연습을 해보겠습니다..! 감사합니다!! 🙇‍♂️

} catch (err) {
alert(err.message);
push(PATH.HOME);

Choose a reason for hiding this comment

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

P1;

request라는 함수의 역할을 생각해 보았을 때
딱 있는 그대로 데이터를 요청하고, 그에 맞는 응답을 return 하는 것이라고 생각돼요.

request 함수 이름으로는 error가 발생 헀을 때 UI상의 변경(alert가 뜬다던지, 특정 페이지로 리다이렉트 된다던지) 하는 것을 유추하기 힘들 뿐 아니라.
상황에 따른 유연한 대처가 쉽지 않아 좋지 않은 코드라고 생각돼요.
(만약 글쓰기 화면에서 글 작성 API를 요청하고 실패 했을 때 실패했다 안내와 함께 home으로 리다이렉트 된다면 매우 화가날 것 같아요.)

여기서는 딱 request 함수 이름에서 나타내는 하나의 역할에만 집중하고, 에러 핸들링(특히 UI)의 경우 UI 관련된 곳에서 처리하게 하는 것이 좋을 것 같은데 어떤가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

네넵..! 해당 코드를 다시 보니 당장의 편리함에 request에 강제적인 에러 처리를 한 것 같네요!! 😥
request는 데이터 요청과 응답에만 집중한다! UI와 관련된 에러 핸들링은 그와 관련된 곳에서 처리한다!
명심하겠습니다!!

}
};
36 changes: 36 additions & 0 deletions src/components/common/AddButton.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import Tooltip from "./Tooltip.js";

export default function AddButton({
parentElement,
Copy link

@nayeon-hub nayeon-hub Jul 10, 2023

Choose a reason for hiding this comment

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

parentElement라는 변수명 이해하기 정말 쉽네요! 부모 element에 들어가는 컴포넌트들에게 target 대신 parentElement 변수명을 사용하니까 직관적이고 더 명확하게 의미를 알게되네요😮

Copy link
Member Author

Choose a reason for hiding this comment

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

근데 장단점이 있는 것 같습니다..! 😂
targetElement로 쓰이는 경우도 생길 것 같은데 계속 사용해보면서 고민할 것 같네요!!

onClick,
text,
tooltipText,
}) {
const buttonElement = document.createElement("button");
buttonElement.className = "add-button";

const tooltipElement = new Tooltip({ text: tooltipText });

buttonElement.addEventListener("click", () => {
onClick();
});

buttonElement.addEventListener("mouseover", (e) => {
if (!e.target.closest(".text")) return;

tooltipElement.toggle(e.target);
});

buttonElement.addEventListener("mouseout", (e) => {
if (!e.target.closest(".text")) return;

tooltipElement.toggle(e.target);
});

this.render = () => {
parentElement.append(buttonElement);
buttonElement.innerHTML = tooltipElement.render(
`<div class="text">${text}</div>`
);
};
}
21 changes: 21 additions & 0 deletions src/components/common/Layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { PATH } from "../../constants/path.js";
import { push } from "../../utils/route.js";

export default function Layout({ parentElement }) {
if (!new.target) return new Layout(...arguments);

const containerElement = document.createElement("div");

containerElement.addEventListener("click", (e) => {
if (e.target.closest("h1")) {
push(PATH.HOME);
}
});

this.render = () => {
parentElement.append(containerElement);
containerElement.innerHTML = `
<h1 class="logo">Jongtion</h1>
`;
};
}
22 changes: 22 additions & 0 deletions src/components/common/NotFound.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { PATH } from "../../constants/path.js";
import { push } from "../../utils/route.js";

export default function NotFound({ parentCompoent }) {
const containerElement = document.createElement("div");
containerElement.className = "not-found-container";

containerElement.addEventListener("click", (e) => {
if (!e.target.closest(".home-button")) return;

push(PATH.HOME);
});

this.render = () => {
parentCompoent.append(containerElement);

containerElement.innerHTML = `
<h2 class="not-found-title">잘못된 경로입니다!</h2>
<button class="home-button">홈으로 이동하기</button>
`;
};
}
14 changes: 14 additions & 0 deletions src/components/common/Tooltip.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export default function Tooltip({ text }) {
this.toggle = (targetElement) => {
targetElement.nextElementSibling.classList.toggle("toggle");
};
Comment on lines +1 to +4
Copy link

Choose a reason for hiding this comment

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

p5 : Tooltip이라는게 hover시에 표시를 해주는 뜻이라는 걸 처음 알게 되었습니다! tooltip이라는 단어를 잘몰라서 그럴수도 있는데 toggle 보다는 hover가 조금 더 직관적으로 와닿는 느낌? 인것 같습니다..!

Copy link
Member Author

Choose a reason for hiding this comment

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

코드를 작성할 때는 classList.toggle로 되어 있어서 생각의 흐름이 저절로 toggle로 이름을 짓게 된 것 같네요..! 😂
저도 기능이나 용도는 hover가 더 맞는 것 같은데 사용할 때 옆에 toggle이 있어서 고민 좀 해보겠습니다..!


this.render = (content) => {
return `
<div class="tooltip">
${content}
<div class="tooltip-text">${text}</div>
</div>
`;
};
Comment on lines +6 to +13
Copy link

Choose a reason for hiding this comment

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

return을 사용하신 점이 인상적입니다!! 👍

}
Loading