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

Open
wants to merge 18 commits into
base: 4/#5_kimjuha
Choose a base branch
from

Conversation

hayamaster
Copy link

@hayamaster hayamaster commented Jul 6, 2023

📌 과제 설명

바닐라JS를 이용한 노션 클로닝 프로젝트
기본적인 요구사항을 충족시키는 것에 집중하였습니다.
스크린샷 2023-07-06 오후 11 52 02

배포 사이트 : notion-cloning-vanilla-js.vercel.app

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

바닐라 JS만을 이용해 노션을 클로닝합니다.
기본적인 레이아웃은 노션과 같으며, 스타일링, 컬러값 등은 원하는대로 커스텀합니다.

  • 글 단위를 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 형태로 만듭니다.
    • 루트 URL 접속 시엔 별다른 편집기 선택이 안 된 상태입니다.
    • /documents/{documentId} 로 접속시, 해당 Document 의 content를 불러와 편집기에 로딩합니다.

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

코드가 너무 구조화되어 있지 않고, 가독성이 안좋습니다.
많은 고민을 하였지만, 과제 제출 시간과 많은 로직을 한 번에 신경쓰기 힘들어 방대한 양의 코드를 잘 정리하지 못했습니다.
코드 구조화에 대해 피드백 주시면 감사하겠습니다!

@hayamaster hayamaster changed the title 4/#5 kimjuha working [Mission5/김주하] Project_Notion_VanillaJs 과제 Jul 6, 2023
@hayamaster hayamaster requested a review from arclien July 6, 2023 15:00
@hayamaster hayamaster self-assigned this Jul 6, 2023
Copy link

@arclien arclien left a comment

Choose a reason for hiding this comment

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

수고하셨습니다. 우선 너무 한번에 100% 구조화된 코드를 만들기보다는
하나씩 리펙토링하면서 분리를 하면 좋겠습니다.

최대한 로직은 분리해서 작은단위로 하나의 역할만 하는 함수로 뺴서 명시적인 이름을 주어 작성하면 좋을 것 같습니다.

PostEditPage에서의 setState, DocumentList에서의 eventListener 를 우선 작은 단위로 나누어 생각해보시고, 그러면서 하나씩 구조적으로 리펙토링을 해보면 좋겠습니다.

Copy link

@wukdddang wukdddang left a comment

Choose a reason for hiding this comment

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

주하님 과제 고생 많으셨습니다. ☺️

전체적인 기능은 잘 구현하신 것 같습니다. UX도 신경써주시면 좋은 애플리케이션이 될 것 같습니다!
다만 DocumentList의 이벤트 리스너는 상위 컴포넌트에서 처리해주시거나 따로 모듈화하시는 게 좋을 것 같습니다! 고생 많으셨습니다 ㅎㅎ

Copy link

@kutta97 kutta97 left a comment

Choose a reason for hiding this comment

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

주하님, 노션 클로닝 과제하시느라 정말 수고 많으셨습니다!

코드리뷰 마감일이 얼마 남지 않아서 제 욕심만큼 꼼꼼하게 코드리뷰를 못해드린 점 정말 죄송합니다ㅠㅠ
그리고, 제가 리뷰하고 싶었던 부분들은 이미 지석멘토님과 창욱님께서 말씀해주셔서.. 제 리뷰 수가 너무 적네요...ㅠㅠ

주하님의 코드를 보면서 좋았던 점은, '부드럽게 읽힌다'는 것이었습니다! 이벤트 리스너 부분처럼 코드가 긴 부분도 읽기는 불편했지만 이해하기가 어렵지는 않았던것 같습니다. 주하님께서 코드를 읽기 쉽게 잘 작성하시는 것 같습니다ㅎㅎ

Q. 코드가 너무 구조화되어 있지 않고, 가독성이 안좋습니다.

컴포넌트는 충분히 잘 구조화하셨고, 제 개인적으로는 가독성이 좋았다고 생각합니다! 저는 주하님만큼 컴포넌트에서 View 로직을 분리하지 못했는데, 주하님은 충분히 잘 분리해주셨고, 그 덕분에 코드를 읽기가 훨씬 수월했다고 느껴집니다.

근데 이 Notion 클로닝 과제 자체가 페이지가 하나만 있고, 컴포넌트를 깔끔하게 나누기 조금 까다로운 요구사항들이 있다보니, 주하님 입장에서는 구조화되어 있지 않고, 가독성이 안좋다고 느껴지실수도 있다고 생각합니다. 제 생각에는 이 부분은 데브코스에서 많은 프로젝트들을 경험하면서 내공이 쌓이면 더 나아질 수 있다고 생각합니다!

Q. 많은 고민을 하였지만, 과제 제출 시간과 많은 로직을 한 번에 신경쓰기 힘들어 방대한 양의 코드를 잘 정리하지 못했습니다.

많은 고민을 하셨던 흔적들이 보입니다! 소피아 매니저님의 말씀대로, 이번에 노션 클로닝 과제를 제출하고 나서 다시는 이 과제에 손을 안댈게 아니잖아요? 과제 이외에도 저희 함께 노션 클로닝 과제를 리팩토링하고 개선해 나가면서 방대한 양의 코드를 정리하는 방법을 훈련하면 된다고 생각합니다ㅎㅎ

Q. 코드 구조화에 대해 피드백 주시면 감사하겠습니다!

저는 개인적으로 Page 는 화면 하나를 가리키는 말이라고 생각해서, 사이드바와 에디터 부분을 Page 라는 이름으로 나눈 부분이 조금 아쉬웠던것 같습니다

Copy link

@howons howons left a comment

Choose a reason for hiding this comment

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

주하님 고생하셨습니다~
Document 삭제 처리를 사용자 입장에서 생각해서 구현하신 게 인상 깊습니다! 백엔드가 주는 대로 기능 구현했던 저를 반성하게 되네요. 😌

이건 어디에 남길지 애매해서 여기에 쓰는데 DocumentList 쪽 버그가 몇 개 있습니다.
document 하나를 클릭한 다음에 바로 삭제 버튼을 클릭하면 ul이 두 개가 됩니다. 그리고 Add a Page 버튼을 클릭할 때 종종 렌더링이 바로 반영이 안되는 경우가 있습니다.
렌더링 반영 오류는 저도 겪었는데 어떤 문제였는지 기억이 안나네요.. 렌더링 과정을 좀 더 최적화 해보시면 좋을 것 같습니다.

Copy link
Member

@Lim-JiSeon Lim-JiSeon left a comment

Choose a reason for hiding this comment

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

과제 수행하시느라 고생하셨습니다~!
다들 너무 좋은 말씀을 많이 해주셔서 제가 할 말이 없었네요 ^^;;;;;;
보면서 고개만 여러번 끄덕끄덕했던 것 같습니다..
주하님 코드는 저에게 교과서와 같은 코드니 이번 노션 스터디동안 많이 참고해서 배우겠습니다~! ㅎㅅㅎ

Copy link
Author

@hayamaster hayamaster left a comment

Choose a reason for hiding this comment

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

코드 리뷰 반영 완료

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