-
Notifications
You must be signed in to change notification settings - Fork 28
[Mission4/김재현] - Project_Notion_VanillaJS #23
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
base: 3/#4_kimjaehyeon
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import { API_END_POINT, HEADER } from "./apiConstans.js"; | ||
|
|
||
| export const request = async (url, options = {}) => { | ||
| try { | ||
| const res = await fetch(`${API_END_POINT}${url}`, { | ||
| ...options, | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "x-username": HEADER, | ||
| }, | ||
| }); | ||
|
|
||
| if (res.ok) { | ||
| return await res.json(); | ||
| } | ||
|
|
||
| throw new Error("API 처리중 뭔가 이상합니다!"); | ||
| } catch (error) { | ||
| alert(error.message); | ||
| } | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| import ContentEditor from "./ContentEditor.js"; | ||
| import { request } from "../../api/api.js"; | ||
| import { push } from "../../utils/router.js"; | ||
|
|
||
| export default function Content({ $target, initialState }) { | ||
| const $content = document.createElement("div"); | ||
| $content.classList.add("layout-content"); | ||
|
|
||
| this.state = initialState; | ||
|
|
||
| const post = { | ||
| title: "", | ||
| content: "", | ||
| }; | ||
|
|
||
| let timer = null; | ||
|
|
||
| const contentEditor = new ContentEditor({ | ||
| $target: $content, | ||
| initialState: post, | ||
|
Comment on lines
+11
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. contentEditor의 초기값으로 post 변수를 만들어 넣어주셨는데, 이 post는 변수는 1회성으로 사용하시는 거 같아서 생성자 함수의 인자 initialState에 초기값을 직접 넣어주셔도 될 거 같고, 또 ContentEditor에도 이미 디폴트 파라미터를 정의하셔서 생략도 가능할 거 같습니다. |
||
| onEditing: (post) => { | ||
| if (timer !== null) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 단지 이 함수 내에서만 사용하는 목적으로 디바운스를 만들었는데 유틸성함수로 뽑아내서 여러군데에서 쉽게 사용할 수 있도록 만들어보세요 ㅎㅎ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p5) 타이머를 null로 사용한건 좋은 거 같아요! 분리 가능하다면 더 좋아질 것 같습니다! 😆 |
||
| clearTimeout(timer); | ||
| } | ||
|
|
||
| timer = setTimeout(async () => { | ||
| const isNew = this.state.documentId === "new"; | ||
|
|
||
| if (isNew) { | ||
| const createdPost = await request("/documents", { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아예 코드단에서는 request를 사용해서 url을 넘기는게 아니라 서비스 폴더를 하나 만들거나 해서 거기서 작성하도록 하는것도 좋아보여요. request(/document); 를 예로 들면 그냥 createPost()같은 메서드를 만들어서 컴포넌트 레벨에서 사용하고 request나 url은 서비스 파일에서 쥐고 관리하는거져. 그럼 훨씬 코드가 직관적으로 보이고 url관리도 좋을것 같습니다. |
||
| method: "post", | ||
| body: JSON.stringify(post), | ||
| }); | ||
|
|
||
| this.setState({ | ||
| documentId: createdPost.id, | ||
| }); | ||
|
|
||
| push(`/documents/${createdPost.id}`); | ||
| } else { | ||
| await request(`/documents/${post.id}`, { | ||
| method: "put", | ||
| body: JSON.stringify(post), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이런 메서드는 유틸로 묶어서 관리하면 코드가 깔끔해지는걸 경험 할 수 있습니다. ㅎㅎ |
||
| }); | ||
| } | ||
| }, 500); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 매직넘버는 상수화해서 사용하면 좋을것 같습니다 ㅎㅎ |
||
| }, | ||
| }); | ||
|
|
||
| this.setState = async (nextState) => { | ||
| if (this.state.documentId !== nextState.documentId) { | ||
| this.state = nextState; | ||
|
|
||
| if (this.state.documentId === "new") { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this.stat.documentId 중첩해서 나오는데 |
||
| const document = { | ||
| title: "", | ||
| content: "", | ||
| }; | ||
|
|
||
| contentEditor.setState(document); | ||
| contentEditor.render(); | ||
| this.render(); | ||
| } else { | ||
| await fetchPost(); | ||
| } | ||
|
|
||
| return; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p5) 마지막 리턴은 없어도 작동할 거 같아요! |
||
| } | ||
|
|
||
| this.state = nextState; | ||
| this.render(); | ||
| contentEditor.setState( | ||
| this.state.document || { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. document가 전역으로 있는 키워드이다보니까 다른 이름을 사용하는게 좀 더 좋아보여요! |
||
| title: "", | ||
| content: "", | ||
| } | ||
| ); | ||
| contentEditor.render(); | ||
| }; | ||
|
|
||
| this.render = () => { | ||
| $target.appendChild($content); | ||
| }; | ||
|
|
||
| const fetchPost = async () => { | ||
| const { documentId } = this.state; | ||
|
|
||
| if (documentId !== "new") { | ||
| const document = await request(`/documents/${documentId}`, { | ||
| method: "get", | ||
| }); | ||
|
|
||
| this.setState({ | ||
| ...this.state, | ||
| document, | ||
| }); | ||
| } | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| export default function ContentEditor({ | ||
| $target, | ||
| initialState = { | ||
| title: "", | ||
| content: "", | ||
| }, | ||
| onEditing, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이벤트 핸들러로 사용하시는 거니까 onEdit이나 onEditContent 처럼 on접두사 뒤에 동사형태가 좋을 거 같습니다ㅎㅎ |
||
| }) { | ||
| const $contentEditor = document.createElement("div"); | ||
|
|
||
| this.state = initialState; | ||
|
|
||
| this.setState = (nextState) => { | ||
| this.state = nextState; | ||
| }; | ||
|
|
||
| $contentEditor.innerHTML = ` | ||
| <input | ||
| type="text" | ||
| class="editor-title" | ||
| name="title" | ||
| /> | ||
| <textarea | ||
| class="editor-content" | ||
| name="content" | ||
| > | ||
| ${this.state.content} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this.state.content 말고 다른 대안으로 const {content} = this.state 이것도 괜찮아 보이네요 ㅎㅎ |
||
| </textarea> | ||
| `; | ||
|
|
||
| $target.appendChild($contentEditor); | ||
|
|
||
| this.render = () => { | ||
| $contentEditor.querySelector("[name=title]").value = this.state.title; | ||
| $contentEditor.querySelector("[name=content]").value = this.state.content; | ||
|
Comment on lines
+34
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 돔에 접근하기 위한 목적이라면 별도로 name같은걸로 접근하는거 대신에 id를 사용하는게 좋아보입니다. |
||
| }; | ||
|
|
||
| this.render(); | ||
|
|
||
| $contentEditor.addEventListener("keyup", (e) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이벤트 리스너 함수로 묶어서 호출하는 방식으로 |
||
| const { target } = e; | ||
| const name = target.getAttribute("name"); | ||
| const nextState = { | ||
| ...this.state, | ||
| [name]: target.value, | ||
| }; | ||
|
|
||
| this.setState(nextState); | ||
| onEditing(this.state); | ||
| }); | ||
|
Comment on lines
+48
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. onEditing에서 nextState 바로 넘겨줘도 괜찮아 보이네요 ㅎㅎ |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| import SideBarList from "./SideBarList.js"; | ||
| import { request } from "../../api/api.js"; | ||
|
|
||
| export default function SideBar({ $target }) { | ||
| const $sideBar = document.createElement("div"); | ||
| $sideBar.classList.add("layout-sidebar"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. classList.add 보다 className이 좀더 괜찮아 보이네요 ㅎㅎ |
||
|
|
||
| const sideBarList = new SideBarList({ | ||
| $target: $sideBar, | ||
| initialState: [], | ||
| }); | ||
|
|
||
| this.setState = async () => { | ||
| const documents = await request("/documents", { | ||
| method: "get", | ||
| }); | ||
|
|
||
| sideBarList.setState(documents); | ||
| this.render(); | ||
| }; | ||
|
|
||
| this.render = async () => { | ||
| $target.appendChild($sideBar); | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| export default function SideBarItem(item) { | ||
| return ` | ||
| <li data-id="${item.id}"> | ||
| <span class="item-load"> | ||
| ${item.title} | ||
| </span> | ||
| <button class="item-remove">-</button> | ||
| <button class="item-add">+</button> | ||
| <ul class="sidebar-list-ul"> | ||
| ${item.documents.length > 0 ? item.documents.map((item) => SideBarItem(item)).join("") : ""} | ||
| </ul> | ||
| </li> | ||
| `; | ||
| } | ||
|
Comment on lines
+1
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 비구조할당으로 item에서 id, title, documents를 빼서 바로 사용해도 좋을듯 싶습니다ㅎㅎ |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import SideBarItem from "./SideBarItem.js"; | ||
| import { request } from "../../api/api.js"; | ||
| import { push } from "../../utils/router.js"; | ||
|
|
||
| export default function SideBarList({ $target, initialState }) { | ||
| const $sideBarList = document.createElement("div"); | ||
|
|
||
| this.state = initialState; | ||
|
|
||
| this.setState = (nextState) => { | ||
| this.state = nextState; | ||
| this.render(); | ||
| }; | ||
|
|
||
| this.render = () => { | ||
| if (!this.state) return; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분은 굳이 없어도 될 것 같습니다 ㅎㅎ 아니면 따로 함수로 만들어서 관리하는 것도 좋겠네요 : ) |
||
|
|
||
| $sideBarList.innerHTML = ` | ||
| <ul class="sidebar-list-ul"> | ||
| ${this.state.map((item) => SideBarItem(item)).join("")} | ||
| </ul> | ||
| `; | ||
|
|
||
| $target.appendChild($sideBarList); | ||
| }; | ||
|
|
||
| this.render(); | ||
|
|
||
| $sideBarList.addEventListener("click", async (e) => { | ||
| const $item = e.target; | ||
| const { id } = $item.parentElement.dataset; | ||
|
|
||
| if ($item.className === "item-load") { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이런 클래스이름들도 상수화해서 사용해주세요~ |
||
| push(`/documents/${id}`); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if ($item.className === "item-remove") { | ||
| await request(`/documents/${id}`, { | ||
| method: "delete", | ||
| }); | ||
|
|
||
| push(`/`); | ||
|
|
||
| return; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return 없어도 될 것 같아요 ㅎㅎ |
||
| } | ||
|
|
||
| if ($item.className === "item-add") { | ||
| const tempPost = { | ||
| title: "", | ||
| parent: id, | ||
| }; | ||
| const createdPost = await request("/documents", { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "/documents" 상수화 하시면 좋을 것 같습니다 ㅎㅎ |
||
| method: "post", | ||
| body: JSON.stringify(tempPost), | ||
| }); | ||
|
|
||
| // this.setState() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사용하지 않는 코드들은 제거해주세요 ㅎㅎ |
||
| push(`/documents/${createdPost.id}`); | ||
| } | ||
| }); | ||
| } | ||
|
Comment on lines
+29
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이벤트 핸들러 같은 경우 모든 케이스를 if문으로 분기하시는 방법은 좋지만, 로직이 조금 길어질 때는 if 안의 로직을 따로 함수로 분리하셔도 좋을 거 같아요😊😊 |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| <html> | ||
| <head> | ||
| <title>Notion</title> | ||
| <link rel="stylesheet" href="/style/style.css" /> | ||
| </head> | ||
| <body> | ||
| <main id="app"></main> | ||
| <script src="/index.js" type="module"></script> | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import App from "./pages/App.js"; | ||
|
|
||
| const $target = document.querySelector("#app"); | ||
|
|
||
| new App({ $target }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||
| import SideBar from "../components/SideBar/SideBar.js"; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. App이 page 폴더에 있는건 살짝 투머치인것 같아요 이거는 tmi 유투브 링크입니다./ |
||||||
| import Content from "../components/Content/Content.js"; | ||||||
| import { initRouter } from "../utils/router.js"; | ||||||
|
|
||||||
| export default function App({ $target }) { | ||||||
| const $layout = document.createElement("div"); | ||||||
| $layout.classList.add("layout-main"); | ||||||
|
|
||||||
| const sideBar = new SideBar({ | ||||||
| $target: $layout, | ||||||
| }); | ||||||
| const content = new Content({ | ||||||
| $target: $layout, | ||||||
| initialState: { | ||||||
| documentId: "new", | ||||||
| }, | ||||||
| }); | ||||||
|
|
||||||
| this.route = async () => { | ||||||
| $target.innerHTML = ``; | ||||||
|
|
||||||
| const { pathname } = window.location; | ||||||
|
|
||||||
| if (pathname === "/") { | ||||||
| await sideBar.setState(); | ||||||
| await content.setState({ documentId: "new" }); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요런 id도 계속 사용하는것 같아서 상수화 하는게 버그 방지에 좋을것 같습니다. |
||||||
| } else if (pathname.indexOf("/documents/" === 0)) { | ||||||
| const [, , documentId] = pathname.split("/"); | ||||||
|
|
||||||
| await sideBar.setState(); | ||||||
| await content.setState({ documentId }); | ||||||
| } | ||||||
|
|
||||||
| $target.appendChild($layout); | ||||||
| }; | ||||||
|
|
||||||
| this.route(); | ||||||
|
|
||||||
| initRouter(() => this.route()); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
라우터 함수만 전달해도 될 거 같습니다! |
||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| .layout-main { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. css 모듈화 해보시는거 어떨까요? 폴더구조가 좀 더 직관적으로 보일 듯 합니다.ㅎㅎ |
||
| display: flex; | ||
| width: 100%; | ||
| height: 100%; | ||
| color: #37352f; | ||
| background: #ffffff; | ||
| } | ||
|
|
||
| .layout-sidebar { | ||
| width: 20%; | ||
| max-width: 280px; | ||
| min-width: 200px; | ||
| height: 100%; | ||
| font-size: 14px; | ||
| font-weight: bold; | ||
| color: #37352f; | ||
| background: #fbfbfa; | ||
| overflow: auto; | ||
| float: left; | ||
| } | ||
|
|
||
| .layout-content { | ||
| width: 80%; | ||
| height: 100%; | ||
| float: left; | ||
| } | ||
|
|
||
| .sidebar-list-ul { | ||
| list-style: none; | ||
| } | ||
|
|
||
| .editor-title { | ||
| width: 100%; | ||
| height: 5%; | ||
| max-height: 60px; | ||
| } | ||
|
|
||
| .editor-content { | ||
| width: 100%; | ||
| height: 95%; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| const localStorage = window.localStorage; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p5) 아마 기능 추가를 하려고 만들다가 시간이 부족해서 못하신 것 같내요 😂 |
||
|
|
||
| export const getItem = (key, defaultValue) => { | ||
| try { | ||
| const storedValue = localStorage.getItem(key); | ||
|
|
||
| return storedValue ? JSON.parse(storedValue) : defaultValue; | ||
| } catch (error) { | ||
| return defaultValue; | ||
| } | ||
| }; | ||
|
|
||
| export const setItem = (key, value) => { | ||
| localStorage.setItem(key, JSON.stringify(value)); | ||
| }; | ||
|
|
||
| export const removeItem = (key) => { | ||
| localStorage.removeItem(key); | ||
| }; | ||
|
Comment on lines
+1
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 로컬스토리지 관련 로직은 따로 사용 안 하신 거 같네요! |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alert로 하게 되면 유저들이 이 정보를 보게 될거같은데 이 정보가 유저들이 인지할 필요가 있는 정보일까요?_?