-
Notifications
You must be signed in to change notification settings - Fork 29
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 과제 #36
base: 4/#5_yanghyejin
Are you sure you want to change the base?
Conversation
- ESLint(Airbnb style) & Prettier - Babel dependency - Webpack configuration - path alias setting(Webpack & Babel & VSCode) - husky pre-commit & pre-push rules
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.
혜진님 과제 너무 잘하셨고 고생 많으셨습니다 ㅎㅎ
일주일을 거의 잠도 안자고 몰두하면서 만드신 노고가 코드에 다 담겨 있는 것 같아 저도 시간을 들여 코드를 봤네요 ㅎㅎ
편집기 이슈는 저도 rich한 편집기를 만들지 못해 큰 도움을 드리지 못한 것 같아 미리 죄송합니다.. 다른 분들의 코드를 보고 저희끼리 스터디를 해보는 건 어떨까... 아니면 슬랙에 자연스레 채팅을 한다거나... 등 조심스레 추천드립니다. ㅎㅎ
저는 component 패턴과 state 관리가 크게 복잡해 보인다고 생각되지는 않았습니다. redux나 다른 상태관리 라이브러리들을 사용한 경험이 없어서 그럴 수도 있습니다... 다만 저도 과제할 때 컴포넌트의 depth가 깊어지다 보니 상태를 추적하는게 버겁긴 했습니다... ㅠㅠ
프로젝트의 요구사항이 많다보니 혜진님의 장점인 관심사의 분리를 통한 프로젝트 설계가 빛을 발하는 것 같습니다. 다만 변수명이나 함수명의 일관성도 챙길 수 있었으면 더 좋지 않았을까... 하는 아쉬움이 남습니다. 근데 그걸 감안해도 너무 잘 만드셨습니다 ㅎㅎㅎ. 좋은 코드 감사합니다! 저도 다음번에는 함수형 말고, 클래스로 컴포넌트를 만들어 봐야겠습니다 ㅎㅎ
src/components/NotionSidebar/CreateButton/SidebarCreateButton.js
Outdated
Show resolved
Hide resolved
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.
수고하셨습니다 혜진님!! 👍 👍
우선 components, pages 폴더 빼고 리뷰를 남겨두었습니다!!
나머지는 한번 더 이어서 리뷰 후 남기겠습니다
const data = await fetch(`${API_END_POINT}${url}`, { | ||
...options, | ||
}) | ||
.then((response) => { | ||
if (!response.ok) throw new Error('API request error'); | ||
|
||
return response.json(); | ||
}) | ||
.catch((e) => { | ||
console.error(e); | ||
}); |
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.
단순 궁금한 사항입니다.
여기에서는 promise chaining을 사용한 특별한 이유가 있으실까요?
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.
체이닝으로 하니까 뭔가... 저는 try, catch보단 더 읽기 쉬워진 느낌이 듭니다!
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.
try-catch 문을 사용하면, try 문안에서 response.json()
을 반환하므로, catch 문 바깥에서 return 이 없으면 lint 에러가 발생했습니다!
lint 에러를 피하기 위해서는 catch 문 바깥에서 return null 을 해주는 것보다, 그냥 chaining 을 사용하여 data 를 구해주는 것이 더 좋을것 같아 이렇게 코드를 작성했습니다!
@@ -0,0 +1,22 @@ | |||
import { getItem, removeItem, setItem } from './localStorage'; | |||
|
|||
const SIDEBAR_KEY = 'side-bar-state'; |
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.
documentStorage에서는 key 값이 있는 반면 여기는 없는데요, 누락된 사항일까요?
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.
id 를 말씀하시는 걸까요? 멘토님의 질문을 잘 이해하지 못했습니다ㅠㅠ
만약 id 를 말씀하시는 거라면, sidebar state 는 id 가 필요하지 않아서 key 값에 id를 추가하지 않았습니다!
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.
혜진님 수고하셨습니다! 💯
코드 뿐만아니라 사용성 측면까지 아주 잘 고려해서 작성해주셨습니다!
준일님의 Core Component를 base로 컴포넌트 작성을 해주셔서 가독성이 아주 좋았었습니다.
읽으면서 예측가능한 코드였습니다! 약간 React Class Component를 읽는 듯한 느낌이었습니다.(실제로 라이프사이클 코드가 없다는 것 빼곤 거의 유사하기도 하고요! )
해결 못한 이슈에 대해서는 1번 composing 이슈는 저도 조금 더 확인을 해보겠습니다! 영어와 달리 한국말은 쉽게 안되는 부분이네요. 관련해서 TUI Editor는 어떻게 해결했는지 참고해봐도 좋을 것 같습니다!
저는 이번에 Component 패턴이 저번에 Todo 과제보다 더 가독성이 좋았습니다. 역시나 익숙함의 문제일까 싶기도 하네요. 저는 계속 말씀드렸지만, API로 데이터 받아서 component에서 가공하고 필요시 자식컴포넌트로 내려보다는 과정으로 개발을 쭉 해와서 자연스러웠습니다. 오히려 이제는 자식 컴포넌트에도 props로 내려 보내지 않으려고 더욱 커스텀 훅을 사용 중이고요.
혜진님 말씀대로 모든 비즈니스 로직이 모여있지 않고 각각에서 처리되어서 한번에 파악하기 어려운 부분은 명확히 존재하네요.
3번의 경우에는 혜진님 말씀대로 마크다운 문법으로 서버에 저장하면 될 것 같습니다.(물론 기획에 따라 달라질 것 같긴합니다만, 보통 마크다운이면 마크다운으로 저장하는 편입니다 )
지금처럼 명시적으로 save button이 없는 경우에는 어쩔 수 없이 debounce를 통해서 실시간으로 stringify, parse 하는 수 밖에 없을 것 같습니다.
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.
혜진님 배포 링크를 PR에 빠트리신 것 같길래 대신 올려드립니다! 링크!
그나저나 좋은 코드 정말 잘 읽었습니다. 코드들이 목적과 규칙에 따라 딱 정리되어 있는게 너무 깔끔해보이네요. 보고 많이 배워야겠습니다! 🪄
다만 Caret 부분은 지금 보기엔 너무 생소해서 제대로 이해하진 못했네요.. 리뷰 끝나고 다시 곰씹어보겠습니다.
그런데 새로고침하면 404 뜨는 데 이건 서버세팅 오류일까요?(저도 세팅을 안한 거였네요.. 강의 보다 알았습니다.)
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.
혜진님 너무 너무 너무 잘하셨어요😭
정말 수고많으셨습니다!
클래스 컴포넌트 형식으로 구현해주셔서 코드가 매우 방대함에도 잘 읽을 수 있었습니다!
클래스 컴포넌트 형식을 매우 잘 구현하시는 것을 보면 과거 자바를 얼마나 잘하셨는지가 느껴집니다ㅎㅎ
- 추가적으로 구현되었으면 좋을 것 같은 기능 한 가지
하위 document를 가지고 있는 상위 document를 삭제할 때, 하위 document도 함께 삭제할 수 있는 기능을 넣으면 좋을 것 같습니다!
저는confirm
으로 띄워서 사용자가 삭제할지 말지 선택하게끔 하였습니다!
sidebarStorage.setSidebarDataItem({ expandedState: expanded }); | ||
} | ||
|
||
super.setState(newState); |
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.
가능한 super관련 메소드는 스코프 내 가장 상단에 위치시키는 것으로 알고 있습니다!
혹시 가장 아래에 두신 이유가 있을까요?
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.
super 보다 위쪽의 함수들이 super 실행 전에 실행되어야 해서 super 메서드를 불가피하게 가장 아래에 두었습니다ㅠㅠ
저도 이렇게 super 를 가장 하단에 위치시키는 방법이 이상하긴 한데... 달리 다른 해결 방법을 찾지 못했습니다...ㅠㅠ
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.
과제 수행하시느라 고생했습니다~!
혜진님 코드는 볼 때마다 정말 많이 고민하고 생각한 흔적들이 많이 보이는 것 같아요 ㅎㅎ
항상 쉬운 코드, 깔끔하고 이해할 수 있는 코드를 짜기 위해 고민하신다고 하셨는데 점점 더 보기 쉬워지는 것 같아요!
특히 파일명과 css별로 나눠서 구분하기 쉬웠고, 네이밍에 신경쓰셔서 코드를 이해하지 못하더라도 이 코드가 어떤 역할을 수행하는지 짐작할 수 있게끔 해주셔서 좋았습니다 ~
📌 과제 설명
Vanilla JS 노션 클로닝 과제를 진행했습니다.
UI를 노션과 최대한 비슷하게 구현하기 위해 노력했습니다.
개발 환경 구성
기술 스택
Vanilla JS + Web Component Pattern
실행 방법
다음 명령어 실행 후,
localhost:3000
에서 확인Build 환경
yarn
을 사용Webpack
을 사용해서 번들링을 진행Babel loader
,CSS loader
및Style loader
html-webpack-plugin
webpack-dev-server
를 사용해서 실행webpack alias
및babel-plugin-module-resolver
를 사용해 별칭 경로(path alias) 추가Code Style
기타 라이브러리
husky
&lint-staged
→
pre-commit
: lint-staged 를 통해 커밋 전 git stage 에 올라온 파일들 린팅해주기 위해 사용→
pre-push:
나의 working-branch 이외의 브랜치에 실수로 푸시하는 것을 방지하기 위해 사용사용자 스토리
사용성을 실제 Notion 앱과 유사하게 만들기 위해 노력했습니다.
유저 스토리를 처음 써봤는데, 혹시 이렇게 쓰는게 자연스러운지 확인해주시면 감사하겠습니다.
Document 추가
이 프로젝트에서 각각의 글을
Document
라고 표현합니다. (노션의 Page 개념)새로운 Root Document 를 추가하기 위해서는 왼쪽 사이드바 상단의
New Document
버튼을 누릅니다.Document 하위에 새로운 Document 를 추가하기 위해서는 다음의 단계를 거칩니다.
+
버튼이 보여진다.+
버튼을 누른다.Document 작성
Document 조회
Document 조회는 3가지 방법으로 가능합니다
Document 삭제
설계 구조
Core Component
Component
class 를 만들었습니다.state
의 흐름을 예측 가능하도록 했습니다.App
App 은 Page 컴포넌트를 가지로 있으며, url 이 바뀌면 각 페이지를 route 합니다.
setState()
해줍니다.Sidebar
Sidebar 은 Document List 를 state 로 가지고 있습니다.
Document
Document 는 Document Data를 state 로 가지고 있습니다.
👩💻 요구 사항과 구현 내용
기본 요구사항
바닐라 JS만을 이용해 노션을 클로닝합니다.
기본적인 레이아웃은 노션과 같으며, 스타일링, 컬러값 등은 원하는대로 커스텀합니다.
보너스 요구사항
div와 contentEditable을 조합하여 Markdown Heading 을 흉내낸 Rich한 에디터를 만들어 보았습니다.
편집기 최하단에 현재 편집 중인 Document의 하위 Document 링크를 렌더링하도록 추가했습니다
편집기 내에서 다른 Document name을 적은 경우, 자동으로 해당 Document의 편집 페이지로 이동하는 링크를 거는 기능을 추가했습니다.
그외 개선하거나 구현했으면 좋겠다는 부분이 있으면 적극적으로 구현해봅니다!
✅ PR 포인트 & 궁금한 점
개선 사항
추가가 필요한 기능
현재는 heading 만 추가할 수 있는, 기능이 하나밖에 없는 poor editor 입니다.
editor 가 조금 더 rich 해질 수 있도록 기능들을 더 추가하고 싶습니다.
리팩토링이 필요한 부분
1. 비즈니스 로직을 UI가 가지고 있는데, 이 계층을 분리하고 싶습니다.
2. 편집기의 커서 위치 저장 방식 변경
3. 컴포넌트의 UI 를 초기화하는 방법 통일
고민 사항
1. 해결 못한 known issue
편집기의 isComposing 이슈
2. Component 패턴과 state 관리
Component 패턴에서 state 관리를 이런 식으로 개발하는 것이 자연스러운지,
아니면 제가 state 관리를 상식과 다르게 조금 이상하게 구현해서 어려웠던 건지 궁금합니다.
리뷰어 분들이 보시기에 제 컴포넌트들의 상태관리 코드가 이해하기 괜찮으신지, 아니면 개선했으면 하는 부분이 있는지 궁금합니다.
3. 정규 표현식을 사용한 contenteditable 편집기
현재 local 과 api 에 문서를 저장할 때는, 에디터에 div 태그로 감싸진 내용을 바로 저장하지 않고, Markdown string 으로 변환해서 저장하도록 하고 있습니다. 굳이 서버에서까지 편집기에 있는 내용의 html tag 정보까지 알고 잇을 필요는 없다고 생각했습니다.
그래서 편집기의 내용이 수정되면, html 을 Markdown 으로 stringify 해주고, Markdown string 을 다시 html 로 파싱해서 에디터에 뿌려주는 방식을 사용하고 있습니다.
이렇게 html 을 Markdown 으로 변환해주는 방식이 너무 비효율적이지는 않은지 궁금합니다.
4. 편집기의 많은 버그들
편집기에 버그가 많습니다. 제 편집기의 설계가 너무 허술해서 그렇다고 생각합니다.
버그가 적은 편집기를 위해 편집기 구조를 개선할 수 있는 방안이 있다면 조언해주시면 감사하겠습니다.