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

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

Conversation

park-moen
Copy link

@park-moen park-moen commented Nov 18, 2022

📌 과제 설명

컴포넌트 구조

image

기능 구현

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

✅ PR 포인트 & 궁금한 점

  • 상수 관리에 있어서 상수만을 모아서 하나의 파일(constant.js)로 넣고 필요한 경우 import 하는 방식으로 프로젝트를 진행했는데 재사용하지 않는 상수도 constant.js로 넣고 import 했는데, 재사용하지 않으면 상수를 사용하는 파일에 넣고, 재새용하는 상수만을 constant.js에 넣는 방법이 좋은지 모든 상수를 constant.js에 넣고 import하는 방법이 좋은지 잘 모르겠습니다.

  • 렌더링 관련해서 vanilla JS로 state가 업데이트 된 경우 어떻게 렌더링 시켜주면 좋을 지 궁금합니다. 현재 저는 App 컴포넌트에서 모든 state를 자식 컴포넌트로 보내주면서, App 컴포넌트의 setState도 함께 내려주어서 setState가 되는 경우 params로 renderabel 문자열을 넣어줘서 리렌더링 시켜주는 부분을 알려서 비교를 통해 리렌더링을 시켜주는 방법을 사용했는데 다른 방법도 궁금합니다.

## Summary
- eslint airbnb-base로 설정하기
- prettier 설정하기
- .gitignore 설정하기
- .env 설정하기
## Summary
- fetch api를 사용하여 공통 API 구현하기
- 공통 api 통신 테스트하기
## Summary
- PostPage 최상위 부모 컴포넌트 구조화하기
- Sidebar 최상위 부모 컴포넌트 구조화하기
## Summary
- element 만드는 기능 구현하기
- atrribute 추가하기 기능 구현하기
## Summary
- PostTitle 컴포넌트 구조화하기
- PostContent 컴포넌트 구조화하기
## Summary
- PostTitle 컴포넌트 삭제하기
- PostContent 컴포넌트 삭제하기
## Summary
- Sidebar 컴포넌트 API 연동하기
- Sidebar 상태 변경에 따른 렌더링하기
- onParentSetState 이벤트 핸들러 구현하기
##Summary
- PostPage 컴포넌트 제목(title state)을 수정하면 SidebarPage 컴포넌트의 리스트 이름(doucmentTitle state)도 변경되게 하는 작업 구현하기
- input 수정하면 연속해서 API가 연동되어서 발생하는 불필요한 통신을 방지하기 위해 debounce를 input에 구현하기
- PostPage 컴포넌트 제목(title state), 내용(content state)을 수정하는 API 연결 구현하기
- PostPage 컴포넌트 내용(content state)이 존재하면 render 함수에 내용 추가하기
## Summary
- 공통 setItem 함수 구현하기
- 공통 getItem 함수 구현하기
## Summnary
- App 컴포넌트에서 작업하던 이벤트 핸들러 컨테이너 컴포넌트로 이동하기
- PostPage 컴포넌트에서 기능별로 자식 컴포넌트 분리하기
- SidebarPage 컴포넌트에서 기능별로 작식 컴포넌트 분리하기
- 상수 값은 constant.js 파일로 이동시키고 import로 상수 사용하기
## Summay

- 현재 작업한 컴포넌트를 전부 변경하였습니다.
- 변경 이유: 정확히 숙달하지 못한 문법을 사용하면서 효율성이 떨어지고 기간내에 프로젝트를 마무리 할 수 없다고 판단해서 변경하게 되었습니다.
; Conflicts:
;	src/api/notionApi.js
;	src/components/App.js
;	src/components/posts/PostContainer.js
;	src/components/posts/PostEditor.js
;	src/components/sidebar/SidebarBody.js
;	src/components/sidebar/SidebarContainer.js
;	src/components/sidebar/SidebarFooter.js
;	src/components/sidebar/SidebarHeader.js
;	src/index.js
;	src/utils/compare.js
Copy link

@dazzel3 dazzel3 left a comment

Choose a reason for hiding this comment

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

명재님! 과제 하시느라 고생하셨습니다 ㅎㅎ 컴포넌트 분리가 너무 명확하게 되어있고, 폴더 구조도 적당하게 잘 나누어졌다고 생각합니다. 코드가 너무 깔끔해서 리뷰할 게 많지 않았던 것 같아요! 고생하셨습니다

상수 관리에 있어서 상수만을 모아서 하나의 파일(constant.js)로 넣고 필요한 경우 import 하는 방식으로 프로젝트를 진행했는데 재사용하지 않는 상수도 constant.js로 넣고 import 했는데, 재사용하지 않으면 상수를 사용하는 파일에 넣고, 재새용하는 상수만을 constant.js에 넣는 방법이 좋은지 모든 상수를 constant.js에 넣고 import하는 방법이 좋은지 잘 모르겠습니다.
=> 저는 모든 상수를 한 파일에 묶어두고 사용하는 방법을 자주 접해서 그 방법도 사용되고 있는 것 같아요 ㅎㅎ

@@ -0,0 +1,24 @@
const createElementHelper = (tagName, ...options) => {
Copy link

Choose a reason for hiding this comment

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

와.. 이렇게 예상하지 못한 곳에서 예외처리를 넣어서 모듈을 만드신 점 너무 좋은 것 같습니다!ㅎㅎ

Copy link
Member

Choose a reason for hiding this comment

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

동의합니다!

Copy link

Choose a reason for hiding this comment

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

저도 너무 좋은 것 같네요 이부분 😊

@@ -0,0 +1,5 @@
const compareState = (previousState, nextState) => {
Copy link

Choose a reason for hiding this comment

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

사용되지 않는 코드인 것 같습니다!

Comment on lines +1 to +3
// import App from './components/App.js';

// new App({ $target: document.querySelector('#app') }).render();
Copy link

Choose a reason for hiding this comment

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

index.html에서 해당 모듈을 불러오는데, 주석처리가 되어 있네요ㅠㅠ 수정하면 좋을 것 같아요.

Copy link
Member

Choose a reason for hiding this comment

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

저도 사용되지 않는 코드인건가? 싶었는데,, 다연님이 딱 물어봐 주셨네요! 이유가 있는건가요?!

Copy link
Member

@WooDaeHyun WooDaeHyun left a comment

Choose a reason for hiding this comment

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

명재님 고생하셨어요! 전체적으로 제가 생각하지도 못한 부분들이 많아서 많이 보고 배웠습니다. 특히 eslint와 prettier까지 최선을 다했다는게 느껴져서 많이 배웠습니다.

Comment on lines +36 to +37
<textarea name="content" class="post-content" placeholder="🥹 입력된 글이 없습니다." value="${this.state.currentDocument.content}" ></textarea>
`;
Copy link
Member

Choose a reason for hiding this comment

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

textarea 태그 대신
div 태그를 사용해서
조금 더 풍부한 에디터를 만들었다면
더 완벽했을 것 같습니다.

@@ -0,0 +1,59 @@
import { readRootDocuments } from '../api/notionApi.js';
Copy link
Member

Choose a reason for hiding this comment

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

전체적으로 코드가 너무 깔끔해서 보기 편했습니다. 또 식별자 이름 하나 하나 신경써서 작성한게 눈에 보여 많은 생각을 했습니다. 특히,,이벤트 발생시, 생성되는 event객체도 e가 아니라 event로 정확하게 적어주시는 걸 보고 스스로를 반성하게 되었습니다. ㅠㅠㅠ

@@ -0,0 +1,24 @@
const createElementHelper = (tagName, ...options) => {
Copy link
Member

Choose a reason for hiding this comment

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

동의합니다!

Comment on lines +1 to +3
// import App from './components/App.js';

// new App({ $target: document.querySelector('#app') }).render();
Copy link
Member

Choose a reason for hiding this comment

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

저도 사용되지 않는 코드인건가? 싶었는데,, 다연님이 딱 물어봐 주셨네요! 이유가 있는건가요?!

Copy link

@SDWoo SDWoo left a comment

Choose a reason for hiding this comment

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

안녕하세요 명재님 !
코드가 넘 깔끔해서 말 남길게 거의 없었네요..ㅎ
제 능력 부족인걸로 하겟씀다..ㅎ
명재님 질문에 대한 제 생각 적기로 마무리 하겠습니다.

  1. 상수 관리에 있어서 상수만을 모아서 하나의 파일(constant.js)로 넣고 필요한 경우 import 하는 방식으로 프로젝트를 진행했는데 재사용하지 않는 상수도 constant.js로 넣고 import 했는데, 재사용하지 않으면 상수를 사용하는 파일에 넣고, 재새용하는 상수만을 constant.js에 넣는 방법이 좋은지 모든 상수를 constant.js에 넣고 import하는 방법이 좋은지 잘 모르겠습니다.
    => 제 생각엔 커피챗에서의 컴포넌트랑 비슷하게 재사용성과, 코드의 가독성을 높여주기 위해 쓰이는 상수들을 모아놓는 목적이기 때문에 넣어두는 것 좋은 것 같습니다!

  2. 렌더링 관련해서 vanilla JS로 state가 업데이트 된 경우 어떻게 렌더링 시켜주면 좋을 지 궁금합니다. 현재 저는 App 컴포넌트에서 모든 state를 자식 컴포넌트로 보내주면서, App 컴포넌트의 setState도 함께 내려주어서 setState가 되는 경우 params로 renderabel 문자열을 넣어줘서 리렌더링 시켜주는 부분을 알려서 비교를 통해 리렌더링을 시켜주는 방법을 사용했는데 다른 방법도 궁금합니다.
    => 저가 이번에 고양이 사진첩 예제를 하면서 느낀 것인데, 거의 모든 것이 가장 최상위 App에서의 setState로 가능하는 것이 좋고 해당 state들에 따라 화면이 변화된다고 생각하기 때문에, 각 동작들에선 상위로 state를 보내주고, state가 변화함에 따른 화면 동작들을 정의해두는게 맞다고 생각합니다. (정답은 아닙니다 😱)

Comment on lines +5 to +11
const createDocument = async id => {
try {
return await request(`documents/${id}`);
} catch (error) {
console.error(error.message);
}
};
Copy link

Choose a reason for hiding this comment

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

Suggested change
const createDocument = async id => {
try {
return await request(`documents/${id}`);
} catch (error) {
console.error(error.message);
}
};
const createDocument = async (id, body) => {
try {
return await request(`documents/${id}`, {
method: 'POST',
body: JSON.stringify(body), // {'title': '', 'parent': null});
} catch (error) {
console.error(error.message);
}
};

request body가 필요할 것 같습니다. POST로 parent 아래에 넣어야 된다고 생각하기 때문입니다 😊

@@ -0,0 +1,24 @@
const createElementHelper = (tagName, ...options) => {
Copy link

Choose a reason for hiding this comment

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

저도 너무 좋은 것 같네요 이부분 😊

Copy link
Member

@sgd122 sgd122 left a comment

Choose a reason for hiding this comment

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

  1. API_ENDPOINT 주소가 이게 아닌가요? 화면을 확인해보려고 해도 확인이 어렵군요...ㅠ
    image

  2. src/components/posts/EmtyEditor.js <- 이거는 진짜 비어있던데 미처 만들지 못한 부분일까요?

보니까 다른 비어있는 페이지들도 더 있는것 같네요. 아마도 만드실 수 있는 곳까지만 만드신것 같습니다.
너무 부담가지지 말고 꾸준히 열심히 하는 자세가 중요한 것 같습니다.
언제나 파이팅입니다~!

✅ PR 포인트 & 궁금한 점

  • 상수 관리에 있어서 상수만을 모아서 하나의 파일(constant.js)로 넣고 필요한 경우 import 하는 방식으로 프로젝트를 진행했는데 재사용하지 않는 상수도 constant.js로 넣고 import 했는데, 재사용하지 않으면 상수를 사용하는 파일에 넣고, 재새용하는 상수만을 constant.js에 넣는 방법이 좋은지 모든 상수를 constant.js에 넣고 import하는 방법이 좋은지 잘 모르겠습니다.

목적성에 맞게 constant.js가 아닌 폴더에 파일형태로 분리하여 관리할 수도 있을 것 같네요~!
정말로 특정 컴포넌트에 딱 한번만 사용하는 거라면 해당 부분에 정의해서 사용할 수는 있을 것 같습니다.

  • 렌더링 관련해서 vanilla JS로 state가 업데이트 된 경우 어떻게 렌더링 시켜주면 좋을 지 궁금합니다. 현재 저는 App 컴포넌트에서 모든 state를 자식 컴포넌트로 보내주면서, App 컴포넌트의 setState도 함께 내려주어서 setState가 되는 경우 params로 renderabel 문자열을 넣어줘서 리렌더링 시켜주는 부분을 알려서 비교를 통해 리렌더링을 시켜주는 방법을 사용했는데 다른 방법도 궁금합니다.

현재 바닐라 자바스크립트로 상태관리 하기에는 어느정도 한계성이 있을 수 있습니다 :)

$target: $navigation,
initialState: this.state,

onClickDoucment: async currentDocumentId => {
Copy link
Member

Choose a reason for hiding this comment

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

onClickDoucment -> onClickDocument
인 것 같습니다 :)

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.

5 participants