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 #9

Open
wants to merge 17 commits into
base: 3/#4_soonyo
Choose a base branch
from

Conversation

KSoonYo
Copy link

@KSoonYo KSoonYo commented Nov 16, 2022

📌 과제 설명

바닐라 JS로 노션 클로닝

👩‍💻 요구 사항과 구현 내용

기본 요구사항

4주차과제_결과

  • 글 단위를 Document라고 합니다. Document는 Document 여러개를 포함할 수 있습니다.
  • 화면 좌측에 Root Documents를 불러오는 API를 통해 루트 Documents를 렌더링합니다.
  • Root Document를 클릭하면 오른쪽 편집기 영역에 해당 Document의 Content를 렌더링합니다.
  • 해당 Root Document에 하위 Document가 있는 경우, 해당 Document 아래에 트리 형태로 렌더링 합니다.
  • Document Tree에서 각 Document 우측에는 + 버튼이 있습니다. 해당 버튼을 클릭하면, 클릭한 Document의 하위 Document로 새 Document를 생성하고 편집화면으로 넘깁니다.
  • 편집기에는 기본적으로 저장 버튼이 없습니다. Document Save API를 이용해 지속적으로 서버에 저장되도록 합니다.
  • History API를 이용해 SPA 형태로 만듭니다.

추가 요구사항

  • 기본적으로 편집기는 textarea 기반으로 단순한 텍스트 편집기로 시작하되, 여력이 되면 div와 contentEditable을 조합해서 좀 더 Rich한 에디터를 만들어봅니다.
  • 편집기 최하단에는 현재 편집 중인 Document의 하위 Document 링크를 렌더링하도록 추가합니다.
  • 편집기 내에서 다른 Document name을 적은 경우, 자동으로 해당 Document의 편집 페이지로 이동하는 링크를 거는 기능을 추가합니다.

✅ PR 포인트 & 궁금한 점

CSS를 전혀 신경쓰지 않고 기본 기능 구현에만 집중했더니 결과물이 많이 심심해졌습니다..
일단 기한 내로 한 곳까지 PR을 하고 추후에 추가 기능과 CSS 모두 지속적으로 업데이트 하겠습니다.

궁금한 점

  • list style을 제가 원하는 이모지로 꾸며주고 싶은데 방법을 모르겠습니다..
  • 문서 트리에서 하위 document를 계속 추가하다보면 깊이가 매우 깊어지는 경우도 생길텐데, 이를 제한하는 것이 좋을까요?

@KSoonYo KSoonYo changed the title 3/#4 soonyo working [Mission4/김순요] - Project_Notion_VanillaJS Nov 16, 2022
@KSoonYo KSoonYo requested a review from yrnana November 16, 2022 14:21
@KSoonYo KSoonYo self-assigned this Nov 16, 2022
Copy link
Member

@LucyYoo LucyYoo left a comment

Choose a reason for hiding this comment

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

안녕하세요! 같은 유리팀 유지영입니다😊
저랑은 또 다른 방법으로 작성하신 코드들이 인상깊어요! 많이 배웠습니다 :)

  • list style의 경우 마크업 부분에서 span 태그 등으로 감싸서 이모지 추가하시거나 제목 바로 옆에 추가하셔도 되고, css에서 ::before을 사용해서 추가할 수도 있을 것 같습니다 :)

토글 부분도 구현해주신 거 넘 멋져요 👍
첫 개인 프로젝트 수고하셨습니다 !!

onDelete({ id });
}
} else if (target.getAttribute("name") === "item-content") {
const id = $li?.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.

id를 if문 안에서 반복해서 변수로 설정하고 있으니 if문 밖에서 선언해주는 건 어떨까요?! 😊

const id = $li?.dataset.id;
const selectedDocs = this.state.selectedDocs;

// subList toggle
Copy link
Member

Choose a reason for hiding this comment

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

함수에 대한 주석을 추가할때는

/**
*
*/
로 추가하는 것을 권장합니다.
그래야 ide에서 hover를 했을때 주석까지 출력이 되기 때문입니다

멘토님께서 이전 과제 때 주석은위와 같은 형식으로 다는 것을 권장한다고 하시더라구요! 😊 //대신 위와 같은 형태로 주석을 추가해주시면 좋을 것 같아요! :)

Comment on lines +13 to +18
onAdd: async ({ parent, title }) => {
await request("/documents", {
method: "POST",
body: JSON.stringify({ parent, title }),
});
this.setState();
Copy link
Member

Choose a reason for hiding this comment

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

실행해보니까 +버튼으로 새로운 글을 추가했을 때 추가한 글이 편집기에 바로 안 뜨더라구요!! 추가된 문서의 id를 받아서 해당 id의 주소로 이동해주면 좋을 것 같아요 😊

Choose a reason for hiding this comment

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

새 문서 만들기 버튼도 같은 함수가 사용되고 있어서 편집기에 바로 뜨지 않고 있어요! 바로 이동되면 좋을 것 같아요 :)

Comment on lines +81 to +85
if (selectedDocs.has(id)) {
selectedDocs.delete(id);
} else {
selectedDocs.add(id);
}
Copy link
Member

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.

토글 기능 너무 좋은 것 같아요👍
그런데 현재 모습에서는 하위 리스트가 있는지 알 수 없어서 나중에 토글 관련 화면이나 css를 추가해 주시면 더 좋을 것 같아요!

@yrnana
Copy link

yrnana commented Nov 18, 2022

궁금한 점

  • list style을 제가 원하는 이모지로 꾸며주고 싶은데 방법을 모르겠습니다..
  • 문서 트리에서 하위 document를 계속 추가하다보면 깊이가 매우 깊어지는 경우도 생길텐데, 이를 제한하는 것이 좋을까요?

list style은 보통 css before content로 구현하는 것 같네요ㅎㅎ
document depth 제한도 어느정도 두는 것이 좋은 것 같습니다

Comment on lines +29 to +30
const documents = await request("/documents");
docsList.setState({ docs: documents });
Copy link

Choose a reason for hiding this comment

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

받는쪽에서도 state를 documents로 정의했으면 좋았을 것 같네요ㅎㅎ


this.setMarkup = (docsList, hide = false) => {
const markup = `
<ul class=${hide ? "hidden" : ""} >
Copy link

Choose a reason for hiding this comment

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

<ul class="${hide ? "hidden" : ""}">
doublequote가 빠진 것 같네요

<ul class=${hide ? "hidden" : ""} >
${docsList
.map((doc) => {
return `<li class="item" data-id=${doc.id}>
Copy link

Choose a reason for hiding this comment

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

여기도 double quote가 빠진 것 같습니다

<span name="item-content"> ${
doc.title.trim() === "" ? "빈 제목" : doc.title
} </span>
<div data-group-id=${doc.id} class="button-group"">
Copy link

Choose a reason for hiding this comment

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

전반적으로 따옴표가 많이 빠져있는 것 같은데, jsx가 아니기 때문에 마크업에 대한 체크가 필요해보입니다~!

Comment on lines +51 to +59
const id = selectedDoc?.id;
if (!id) {
this.state = { id: "new" };
$ediotr.setState({ title: "", content: "" });
} else {
this.state = selectedDoc;
const doc = await request(`/documents/${id}`);
$ediotr.setState(doc);
}
Copy link

Choose a reason for hiding this comment

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

if else로 구성한다면 굳이 if (!id) 부터 시작하지 않아도 될 것 같습니다
if (id) else 로 작성하면 될 것 같네요~

Comment on lines +28 to +37
const res = await request("/documents", {
method: "POST",
body: JSON.stringify({ title, parent: null }),
});
if (res) {
this.state = res;
push(`/documents/${this.state.id}`);
} else {
throw new Error("Post method failed");
}
Copy link

Choose a reason for hiding this comment

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

이부분은 res의 유무가 아닌 trycatch로 처리를 해야하지 않을까요?

Copy link

@choibyeol choibyeol left a comment

Choose a reason for hiding this comment

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

안녕하세요 순요님! 첫 번째 프로젝트 고생 많으셨습니다 :)
토글 기능도 구현하시고 컴포넌트에 나눠져 있는 함수들을 활용을 잘해주신 것 같아요!
많이 배워갑니다 👍�

Comment on lines +81 to +85
if (selectedDocs.has(id)) {
selectedDocs.delete(id);
} else {
selectedDocs.add(id);
}

Choose a reason for hiding this comment

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

토글 기능 너무 좋은 것 같아요👍
그런데 현재 모습에서는 하위 리스트가 있는지 알 수 없어서 나중에 토글 관련 화면이나 css를 추가해 주시면 더 좋을 것 같아요!

});
onChange();
}
}, 1500);

Choose a reason for hiding this comment

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

처음 시작하는 화면에서 제목과 내용을 입력하면 새로운 페이지로 추가될 때 내용이 사라지는 현상이 있더라고요!
아마 처음에는 id가 new로 되어있어서 POST 날릴 때 제목만 넣어줘서 그런 것 같은데
제목 입력이 끝난 것을 감지해서 바로 POST를 한다거나 임시로 1.5초로 설정되어 있는 부분을 조금 짧게 설정해 주는 방법도 있을 것 같아요!

Comment on lines +58 to +59
<span> No documents </span>
<button name="add" > + 새 문서 만들기 </button>

Choose a reason for hiding this comment

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

같은 줄에 No documents와 새 문서 버튼이 출력돼서 span 태그 보다는 p 태그를 사용하거나 css를 통해 다른 줄로 만드는 게 조금 더 보기 좋을 것 같아요!

Comment on lines +13 to +18
onAdd: async ({ parent, title }) => {
await request("/documents", {
method: "POST",
body: JSON.stringify({ parent, title }),
});
this.setState();

Choose a reason for hiding this comment

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

새 문서 만들기 버튼도 같은 함수가 사용되고 있어서 편집기에 바로 뜨지 않고 있어요! 바로 이동되면 좋을 것 같아요 :)

Copy link

@IGhost-P IGhost-P left a comment

Choose a reason for hiding this comment

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

같은팀 양상우 입니다~!
순요님 만의 독특한 코드를 재밌게 보고 갑니다!!! 컴포넌트 분리에 고민을 하신 모습이 인상 깊어요!! 😎

질문주신것중에 대한 답은

  1. css의 before나 after 속성을 이용해보시는건 어떨까요? content에 이모지를 넣으면 구현할수 있습니다!
  2. 주로 무한스크롤이나 아니면 overflow : hiden or scroll등으로 해결 하는 방법이 있을것 같아요!

},
})
);
};

Choose a reason for hiding this comment

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

컴포넌트 폴더 안에 유틸 폴더가 있는건 무언가 네이밍과 맞지 않은것 같아요~!

};

$docsList.addEventListener("click", (e) => {
const { target } = e;

Choose a reason for hiding this comment

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

구조분해 할당은 매개변수 쪽에서 바로 하는것도 좋을것 같아요~!~!
({target} =>

if (target.name === "add") {
this.setState({ selectedDocs: this.state.selectedDocs.add(id) });
onAdd({ parent: id || null, title: tempTitle });
} else if (target.name === "delete") {

Choose a reason for hiding this comment

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

if문의 분리가 필요할것 같습니다!

if (res) {
this.state = res;
push(`/documents/${this.state.id}`);
} else {

Choose a reason for hiding this comment

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

try catch를 이용헤 res.ok가 아닐경우 에러를 발생시키는 방향도 좋을것 같아요~!

@@ -0,0 +1,14 @@
<!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.

SEO를 위해 ko로 변경하는건 어떨까요~

Copy link

@Heojiyeon Heojiyeon left a comment

Choose a reason for hiding this comment

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

순요님 안녕하세요!
코드 읽으면서 각 컴포넌트가 하는 일이 명확하게 잘 짜여진 것 같다는 느낌을 받았습니다
수고 많으셨습니다 :)

id: null,
};

const $ediotr = new EditorPage({

Choose a reason for hiding this comment

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

가벼운 실수겠지만, editor 오타가 난 것 같습니다 :)

import DocsList from "./DocsList.js";

export default function DocsContainer({ $target }) {
const $docsContainer = document.createElement("aside");

Choose a reason for hiding this comment

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

aside element는 처음 보는데, 순요님 덕분에 공부하고 지나갑니다~ :)

Comment on lines +5 to +8
export default function EditorContainer({ $target, onChange }) {
const $editorContainer = document.createElement("main");
$editorContainer.className = "editor-container";
$target.appendChild($editorContainer);

Choose a reason for hiding this comment

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

new 키워드 생성 방어코드를 추가하시면 좋을 것 같습니다 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants