[Mission4/류건열] - Project_Notion_VanillaJS #36

rjsduf0503 · 2022-11-19T13:29:03Z

PR이 삭제돼서 다시 작성합니다..ㅠㅠ

📌 과제 설명

Vanilla JS를 이용한 Notion 구현

https://ryu-notion.vercel.app/

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

프로젝트 초기 설정 및 기본 스타일 적용

프로젝트 초기 설정 및 스타일 적용

좌측 navigator ui 구현 및 documents 데이터 바인딩

좌측 navigator 기능, ui 구현 및 API를 이용하여 documents 데이터 바인딩

문서 추가 기능 및 스크롤 구현

document 추가 기능 및 navigator에 스크롤러 구현

document 삭제 ui 및 기능 구현

document 삭제 기능, ui 구현

rich 에디터 구현

rich 에디터 구현
- '/' 입력 후 space : italic
- '*' 입력 후 space : bold
- '_' 입력 후 space : underline
- '-' 입력 후 space : line-through
- '#' 입력 후 space : h1 tag
- '##' 입력 후 space : h2 tag
- '###' 입력 후 space : h3 tag
- '@' 및 제목 입력 후 click : 외부 document link

하위 페이지 링크 및 다른 페이지 링크 구현

하위 document가 존재할 경우 화면 하단에 표시 기능 및 외부 documents 링크 구현

외부 documents 링크 에러 해결, 검색 구현 및 헤더에 폴더 구조 추가

외부 documents 링크 에러 해결
documents 링크 걸 때 title로 검색 구현
documents 링크 모달 창 draggable 구현
화면 상단에 root로 부터 현재 document 까지의 폴더 구조 ui 구현

변수명, import, 상수 정리

title 작성 중 content 이동 시 반영 안되는 문제, getItem defaultValue

title 작성 중 content 변경 시 반영 안되는 문제 해결
getItem에 누락된 defaultValue 추가

✅ 피드백 반영사항 지난 코드리뷰에서 고친 사항을 적어주세요. 재PR 시에만 사용해 주세요! (재PR 아닌 경우 삭제)

변수 네이밍 변경
router 함수 위치 변경
debounce 모듈화
rich text 로직 분리
storage 값 array -> object 로 변경하여 삭제 시간 ⬇️
document가 열려있을 때만 렌더링 되도록 로직 변경

✅ PR 포인트 & 궁금한 점

현재 하나의 컴포넌트에 너무 많은 기능이 포함되어 있어 비효율적이라는 생각이 듭니다.
위의 문제로 state 관리가 꼬여서 state에 대한 리팩토링이 필요할 것 같습니다.
외부 document 링크 추가 후 추가된 document navigator에서 삭제해도 그대로 남아 있는 문제가 존재합니다.
vercel 배포 시 api를 최상단에 위치시키지 않고 vercel.json에 추가하여 그대로 사용하는 방법이 있는지.. 궁금합니다.

1. documents 링크 걸고 제목 변경 시 업데이트 2. documents 링크 모달 ui 향상 3. documents 링크 모달 드래그 구현 4. documents 링크 클릭 시 상위 documents 전부 펼쳐지도록 구현 5. documents 링크에 검색 기능 구현 6. editor 헤더에 상위 documents 구조 추가

Kal-MH

건열님!

벌써 마지막 코드리뷰라니, 아쉽습니다 그 동안 같은 팀으로 함께 공부할 수 있어서 즐거웠습니다:) rich content 부터 모달창까지 구현력이 정말 뛰어나신 거 같습니다. 덕분에 건열님 코드 보면서 selection객체 이용하는 법을 공부할 수 있을 거 같아 저는 횡재(?) 했네요 ㅎㅎ

몇 가지 동작 리뷰 사항 남깁니다:)

페이지가 스크롤바가 생길 정도로 많이 생성된 경우, document-add블록 또한 스크롤바 안에 속해 있기 때문에 화면에서 사라져 버리더군요. scroller와 동일한 뎁스로 빼는 것은 어떨까요?
하위 document가 있는 상태에서 부모 document를 삭제했을 경우, 하위 모든 문서가 같이 삭제되는 것이 아닌 documents의 가장 마지막 위치로 하위 문서들이 이동하는 것을 볼 수 있었습니다. 건열님의 의도된 구현방향인 지 궁금합니다:)

Notion Project 진하게 구현하신 건열님 수고많으셨어요:)

Kal-MH · 2022-11-20T15:07:29Z

src/app.js

+    $target: $wrapper,
+    initialState: this.state,
+    addDocument: async (targetDocumentId) => {
+      const document = await request(


p3) fetchDocument()에서의 document 변수는 특정 id에 대해서 조회한 결과라는 걸 알 수 있었습니다. 따라서 navigator의 addDocument()에서의 document 또한, 새로 생성된 document가 아닌 조회의 결과라는 맥락으로 해석될 수 있을 거 같습니다. 간단히 "newDocument" 정도로 변경하는 것은 어떨까요?

그렇네요!! document라는 네이밍 보다는, 새로 생성한 문서이기 때문에 newDocument라는 네이밍이 더 적합하겠군요! 감사합니다 :)

Kal-MH · 2022-11-20T15:13:56Z

src/app.js

+      if (document) {
+        const initializedDocument = await request(
+          `/documents/${document.id}`,
+          { method: METHOD.PUT },


ask) 새로 document 생성을 하고 PUT을 통해 다시 불러오는 것은 content의 값을 empty string으로 만들기 위함인가요? 수정을 거치지 않은 처음 생성된 document의 경우, content가 null로 초기화되는 것으로 알고 있습니다. 어떤 의도가 들어있는지 궁금하네요!

음,, 뭔가 새로 만든 document의 content가 null 값인게 문득 이상하기도 했고, 프로젝트 진행할 때 에디터 부분에서 에러가 나서 초기화 했던 것 같습니다!

Kal-MH · 2022-11-20T15:19:05Z

src/app.js

+
+  this.route();
+
+  window.addEventListener(EVENT.POPSTATE, () => this.route());


p4) router.js가 정의되어 있으니 initRouter()에 모아두는 것은 어떨까요?

오 그렇네요 ㅋㅋㅋ initRouter의 인자 onRoute()로 안에서 설정해 주면 되는거였군요 감사합니다 :)

Kal-MH · 2022-11-20T15:22:49Z

src/components/editor.js

+          let $parent = anchorNode.parentNode;
+
+          if ($parent.classList.contains('editor-content')) {
+            $parent.innerHTML = `<div>${anchorNode.textContent}</div>`;


div를 삽입해서 동작하도록 한다는 부분이 여기였군요! 아이디어 굿:)

아이디어 굿~~👍

metacode22

건열님 코드보면서 최대한 많은 기능을 구현하려 노력하셨다는 점이 확 느껴지네요. 실제로 배포된 사이트로 가서 봐도 기능들이 잘 되네요! 5주간 같이 팀으로 지내면서 느낀 점이 건열님은 다른 것도 잘하시지만 구현을 정말 잘하시는 것 같다는 점이었어요!

저도 vercel로 배포 후 서버리스 함수를 사용할 때 api 파일을 최상단에 위치시키지 않고, vercel.json을 변경해서 사용하는 방법을 알고 싶네요... ㅋㅋㅋㅋ 아니면 dotenv를 사용하던가 해야될 것 같습니다. js 파일로 숨기고 gitignore하면 될 줄 알았는데 결국엔 배포할 때 보이게 되니까... 꼭 필요한 조치 같네요!

5주간 덕분에 많이 배웠습니다! 👍

metacode22 · 2022-11-21T07:17:13Z

index.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html lang="en">


저도 받은 피드백 사항인데, 사소한 것이지만 en으로 설정해두면 시각 장애인 분들이 해당 웹 페이지에 접속하셨을 때 TTS 엔진이 한글을 어색하게 읽을 수도 있다고 합니다! ko로 설정하셔서 해당 문제가 발생하지 않도록 하는 건 어떨까요!?

reference : https://github.com/prgrms-fe-devcourse/FEDC3-3_VanillaJS_1/pull/27#discussion_r1018983230

앗 저도 저번주에 받은 피드백인데 깜빡하고 반영을 못했네요.. ㅋㅋㅋ 감사합니다 :)

metacode22 · 2022-11-21T07:19:55Z

src/app.js

+  this.setState = (nextState) => {
+    for (const [key, value] of Object.entries(nextState)) {
+      this.state[key] = value;
+    }


this.state = {...nextState};처럼 재할당하는 것이 아니라 이렇게 key를 바꾸는 이유가 혹시 있나요? 순수하게 의도가 궁금합니다!

변경할 부분만 수정하기 위해 작성했던 코드였는데, 지금 생각해보니

this.state = {...this.state, ...nextState}

로 작성해도 됐을 것 같은 코드네요... ㅋㅋㅋㅋ

metacode22 · 2022-11-21T07:26:21Z

src/app.js

+  });
+
+  let timer = null;
+


P5 : debounce를 모듈화해보는 거에 대해서는 어떻게 생각하시나요? '이 콜백 함수에 debounce를 적용시켰구나~'라고 단번에 말해주기 좋을 것 같습니다! 밑에는 제가 참고했던 코드들입니다. 확실히 lodash는 타이트하게 구현해두었네요... 저는 첫 번째 글을 보고 간단하게 구현했습니다 ㅎㅎㅎ

reference : http://yoonbumtae.com/?p=3584
reference : https://github.com/lodash/lodash/blob/4.17.15/lodash.js#L10304

안그래도 이번주 강의에서 debounce를 따로 빼서 사용하시길래 저도 한번 적용해볼까 했는데 마침 승준님께서 코멘트를 작성해주셨네요!! ㅎㅎ 모듈화 해보겠습니다~~

metacode22 · 2022-11-21T07:27:56Z

src/components/editor.js

+            $parent.classList.contains('underline')
+              ? $parent.classList.remove('underline')
+              : $parent.classList.add('underline');
+          }


P5 : makeRichContent? 와 같은 이름으로 따로 해당 로직을 분리해도 좋을 것 같습니다!

저도 작성 하면서 코드가 너무 길어지는게 맘에 걸렸는데, 알려주셔서 감사합니다 ㅎㅎ

metacode22 · 2022-11-21T07:35:34Z

src/components/editor.js

+    const $documentLinks = $editor.querySelectorAll('.document-link');
+
+    if ($documentLinks && $documentLinks.length) {
+      [].forEach.call($documentLinks, ($documentLink) => {


오호, 처음엔 이게 뭐지? 싶었는데 $documentLinks가 element들을 담고 있는 유사배열이기에 Array 프로토타입의 forEach를 빌려오는 거군요! 맞나요??? ㅋㅋㅋㅋㅋㅋ 좋은 방법 하나 알아갑니다!

dmswl98

건열님!
항상 코드 스타일이 저와 달라서? 코드 읽으면서 많이 배우는 것 같아요 👀
역시나 rich 에디터가 기가막힙니다!!!
저도 나중에 모달창말고 '#' 방식으로도 구현해보아야겠어요!
그리고 navigator에 낙관적 업데이트를 구현하셨네요! 나중에 코드 보러 와야겠어요ㅎㅎ

마지막 코드 리뷰라 많이 아쉽습니다...
5주 동안 수고하셨고 같이 공부할 수 있어서 너무나 좋았습니다! 🥰
내년 1월에 뵈어요!

dmswl98 · 2022-11-21T10:15:28Z

src/components/editor.js

+          let $parent = anchorNode.parentNode;
+
+          if ($parent.classList.contains('editor-content')) {
+            $parent.innerHTML = `<div>${anchorNode.textContent}</div>`;


아이디어 굿~~👍

dmswl98 · 2022-11-21T10:18:08Z

src/components/editor.js

+        } else if (e.key === EVENT_KEY.ESC) {
+          const $parent = anchorNode.parentNode;
+          const text = $parent.innerHTML;
+          removeModal($parent, text);


맞다.. parentNode가 있었죠! 이제껏 e.target.closest만 썼는데 나중에 활용해봐야겠어요~

dmswl98 · 2022-11-21T10:21:33Z

src/components/navigator.js

+  const getDocuments = (documents, depth = 0, opened = false) => {
+    if (!documents || documents.length === 0) return '';
+    return `
+      <div class='flex-column'>
+        ${documents
+          .map(({ id, title, documents }) => {
+            const isOpened = this.state.openedDocuments.map((key) => parseInt(key)).includes(id);
+            return `


P5 : openedDocuments 데이터를 객체로 관리하면 매번 발생하는 O(N^2)만큼의 시간을 줄일 수 있을 것 같아요!

저도 은지님 코드 리뷰 하면서 하나 배웠던 점입니다!! 안그래도 적용해야 겠다고 생각을 했었어요..!! 감사합니다 ㅎㅎ

dmswl98 · 2022-11-21T10:27:55Z

src/components/navigator.js

+              <div key=${id} class='document document-item flex-row' style='padding-left: ${depth}rem; display: ${
+              depth === 0 || isOpened || opened ? 'flex' : 'none'
+            };'>
+                <div class='icon-wrapper'>${isOpened ? chevronDownIcon : chevronIcon}</div>
+                <div class='icon-wrapper'>${documentIcon}</div>
+                <div class='title-wrapper' id='id-${id}' data-title='${title}'>
+                  ${title}
+                  <div class='visible-when-hover'>
+                    <div class='icon-wrapper document-delete'>${trashIcon}</div>
+                    <div class='icon-wrapper document-add'>${plusIcon}</div>
+                  </div>
+                </div>
+              </div>
+              ${getDocuments(documents, depth + 1, isOpened)}
+            `;
+          })
+          .join('')}
+      </div>
+    `;
+  };


P4: isOpened에 따라 display를 이용해서 열린 문서들을 보이거나 숨기도록 하셨네요! 👍
고런데 display: none; 적용했더라도 DOM 트리에 추가되는 걸로 알고 있습니다...(만약 아니라면 말씀해주세요!)
해당 문서의 isOpened가 true일 경우에만 랜더링되도록 하는 것은 어떨까요???

${isOpened ? getDocuments(documents, depth + 1) : ''}

DOM 트리에 추가되는게 사실 하면서도 찜찜했는데.. 은지님 덕분에 고치고 갑니다 :)
그런데 한번에 렌더링할 때 event도 고정시켜놔서 다시 event 로직을 수정했읍니다....... ㅠ_ㅠ

dmswl98 · 2022-11-21T10:32:02Z

src/components/navigator.js

+    const chevrons = $navigator.querySelectorAll('.chevron');
+    [].forEach.call(chevrons, (chevron) => {


이거슨 유사배열에 배열 메서드를 이용하려고 작성하신 거군요..!

dmswl98 · 2022-11-21T10:34:20Z

src/utils/constants.js

+export const EVENT_KEY = {
+  DISABLE_API_CALL: {
+    SHIFT: 'Shift',
+    COMMAND: 'Meta',
+    ALT: 'Alt',
+    CONTROL: 'Control',
+    CAPSLOCK: 'Capslock',
+    ARROW_UP: 'ArrowUp',
+    ARROW_DOWN: 'ArrowDown',
+    ARROW_LEFT: 'ArrowLeft',
+    ARROW_RIGHT: 'ArrowRight',
+  },
+  ENTER: 'Enter',
+  SPACE: ' ',
+  BACKSPACE: 'Backspace',
+  AT: '@',
+  ESC: 'Escape',
+};
+export const ICON = {
+  DOCUMENT: 'document',
+  CHEVRON: 'chevron',
+  PLUS: 'plus',
+  TRASH: 'trash',
+};
+export const EVENT = {
+  KEYUP: 'keyup',
+  KEYDOWN: 'keydown',
+  CLICK: 'click',
+  SCROLL: 'scroll',
+  MOUSEUP: 'mouseup',
+  MOUSEDOWN: 'mousedown',
+  MOUSEMOVE: 'mousemove',
+  POPSTATE: 'popstate',
+  ROUTECHANGE: 'routing-change',
+};


잘못 적어서 에러날 일은 절대 없겠다.... EVENT 객체는 저도 적용해보아야겠어요 😲

eastroots92

11월 26일 12시에 줌을 통해 구두로 피드백을 드렸고
기록 남기기용으로 구두로 이야기 나눴던 코드리뷰를 간략하게 적어두려 합니다.

CSS 관련

Reset CSS와 normalize CSS

가능하다면 의도적으로 넣어둘 것

JS 관련

클로저 패턴 + 관심사 별로 묶기

[vanilla-js-notion/api.js at feature · gxxrxn/vanilla-js-notion](https://github.com/gxxrxn/vanilla-js-notion/blob/feature/src/utils/api.js)

[vanilla-js-notion/storage.js at feature · gxxrxn/vanilla-js-notion](https://github.com/gxxrxn/vanilla-js-notion/blob/feature/src/utils/storage.js)
CustomEvent / 옵저버 패턴 / 펍섭패턴

기술 구조 관련

폴더 구조 잡기 (도메인 지역성을 고려한 구조, 기능 별로 모아두는 구조)
API
- ky, axios 등을 많이 사용함.

역할을 명확하게 하기

상수
- 전역에서 알아야 하는건가? 딱 그 곳에서만 알면 좋은 건가?

컴포넌트

customElements ⇒ 커스텀한 컴포넌트를 만들 수 있다.
container-component

const template = document.createElement('template');
template.innerHTML = `
  <style>
    h3{
      color:orange;
    }
  </style>
  <div class="user-card">
    <h3></h3>
  </div>
`

class UserCard extends HTMLElement{
  constructor(){
    super();

    //shadow DOM
    this.attachShadow({mode:'open'});
    this.shadowRoot.appendChild(template.content.cloneNode(true));
    this.shadowRoot.querySelector('h3').innerText = this.getAttribute('name')

    // this.innerHTML = `<h3>${this.getAttribute('name')}</h3>`
  }
}

window.customElements.define('user-card', UserCard)

<body>
	<user-card>안녕</user-card>
</body>

기능(function)과 화면(presentation)

api 에서 try catch 등을 통해 alert를 띄우는건 좋지않음.

import { push } from '../router.js';
import { API_END_POINT } from '../url.js';
import { USER_NAME } from './constants.js';

export const request = async (url, options = {}, data) => {
  const res = await fetch(`${API_END_POINT}${url[0] === '/' ? url : `/${url}`}`, {
    ...options,
    headers: {
      'x-username': USER_NAME,
      'Content-Type': 'application/json',
    },
    body: JSON.stringify(data),
  });

  if (res.ok) {
    return await res.json();
  } else {
    push('/');
    throw new Error(`${res.status} Error`);
  }
};

const getData({params}, callback);

Router 처리

역할 위임 (queryParams, url path)
관리하는법

const [, , documentId] = pathname.split("/"); // 이렇게 하면 URL 구조를 이해하기 쉽지 않음. 추가로 URL이 어려움.

HTML 관련

시맨틱 태그

기타

eof 이슈

editorConfig 등을 사용해서 지켜줄 것
XSS 이슈

rjsduf0503 added 9 commits November 9, 2022 17:41

chore: 프로젝트 초기 설정 및 기본 스타일 적용

5821bf7

feat: 좌측 navigator ui 구현 및 documents 데이터 바인딩

627af0c

feat: 문서 추가 기능 및 스크롤 구현

748f7ca

feat: document 삭제 ui 및 기능 구현

fbe3ab8

feat: rich 에디터 구현

4b7bec0

feat: 하위 페이지 링크 및 다른 페이지 링크 구현

f4804b8

chore: 변수명, import, 상수 정리

981029b

fix: title 작성 중 content 이동 시 반영 안되는 문제, getItem defaultValue

84aaf0c

rjsduf0503 requested a review from eastroots92 November 19, 2022 13:29

rjsduf0503 self-assigned this Nov 19, 2022

rjsduf0503 added 과제 리뷰 요청 피드백 요청 🔥 매운맛 동근팀 labels Nov 19, 2022

Kal-MH reviewed Nov 20, 2022

View reviewed changes

metacode22 reviewed Nov 21, 2022

View reviewed changes

dmswl98 reviewed Nov 21, 2022

View reviewed changes

rjsduf0503 added 8 commits November 24, 2022 21:01

refactor: 비구조화 할당 추가 및 네이밍 변경

d18b1b2

refactor: initRouter로 초기 routing 및 popstate event 이동

e8c0d8c

chore: language ko로 변경

389a437

refactor: onEditing에서 낙관적 업데이트 로직, debounce 로직 분리

c1c1028

refactor: rich text 변환 로직 분리

08b6f9f

refactor: navigator에서 열린 document만 렌더링되도록 변경

de820e1

refactor: openedDocuments 객체로 변경

1e543ac

fix: document 클릭 시 렌더링 되지 않은 document에 대한 open 처리

2e6e95e

eastroots92 reviewed Dec 6, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mission4/류건열] - Project_Notion_VanillaJS #36

[Mission4/류건열] - Project_Notion_VanillaJS #36

rjsduf0503 commented Nov 19, 2022 •

edited

Loading

Kal-MH left a comment

Kal-MH Nov 20, 2022

rjsduf0503 Nov 24, 2022

Kal-MH Nov 20, 2022

rjsduf0503 Nov 24, 2022

Kal-MH Nov 20, 2022

rjsduf0503 Nov 24, 2022

Kal-MH Nov 20, 2022

dmswl98 Nov 21, 2022

metacode22 left a comment

metacode22 Nov 21, 2022

rjsduf0503 Nov 24, 2022

metacode22 Nov 21, 2022

rjsduf0503 Nov 24, 2022

metacode22 Nov 21, 2022

rjsduf0503 Nov 24, 2022

metacode22 Nov 21, 2022

rjsduf0503 Nov 24, 2022

metacode22 Nov 21, 2022

dmswl98 left a comment

dmswl98 Nov 21, 2022

dmswl98 Nov 21, 2022

dmswl98 Nov 21, 2022 •

edited

Loading

rjsduf0503 Nov 24, 2022

dmswl98 Nov 21, 2022

rjsduf0503 Nov 24, 2022

dmswl98 Nov 21, 2022

dmswl98 Nov 21, 2022

eastroots92 left a comment


		this.route();

		window.addEventListener(EVENT.POPSTATE, () => this.route());

		const chevrons = $navigator.querySelectorAll('.chevron');
		[].forEach.call(chevrons, (chevron) => {

[Mission4/류건열] - Project_Notion_VanillaJS #36

Are you sure you want to change the base?

[Mission4/류건열] - Project_Notion_VanillaJS #36

Conversation

rjsduf0503 commented Nov 19, 2022 • edited Loading

📌 과제 설명

Vanilla JS를 이용한 Notion 구현

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

✅ 피드백 반영사항 지난 코드리뷰에서 고친 사항을 적어주세요. 재PR 시에만 사용해 주세요! (재PR 아닌 경우 삭제)

✅ PR 포인트 & 궁금한 점

Kal-MH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metacode22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmswl98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmswl98 Nov 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eastroots92 left a comment

Choose a reason for hiding this comment

CSS 관련

JS 관련

기술 구조 관련

HTML 관련

기타

rjsduf0503 commented Nov 19, 2022 •

edited

Loading

dmswl98 Nov 21, 2022 •

edited

Loading