[Mission4/김규란] - Project_Notion_Vanilla_JS by gxxrxn · Pull Request #12 · prgrms-fe-devcourse/FEDC3-4_Project_Notion_VanillaJS

gxxrxn · 2022-11-16T14:43:44Z

📌 과제 설명

Vanilla JS로 노션을 클로닝합니다.

실행방법

.최상위에 .evn.js 파일을 생성 후, API_END_POINT, USER_NAME을 작성해서 export 해주세요.
npm serve -s 으로 프로젝트를 실행시킬 수 있습니다.

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

글 단위를 Document라고 합니다. Document는 Document 여러개를 포함할 수 있습니다.

기본 요구사항

✅ PR 포인트 & 궁금한 점

- gitignore 설정

- Sidebar 컴포넌트 - SidebarSection > Details > DocumentItem - SidebarFooter

rjsduf0503

안녕하세요 규란님 :)
이번 노션 프로젝트도 수행하느라 고생 많으셨습니다 ㅎㅎ

어느새 팀원으로써 마지막 코드 리뷰를 남기게 되었는데 상당히 아쉽네요!

리뷰할 때마다 느끼는거지만 규란님은 항상 코드를 굉장히 효율적으로 짜시는 것 같아 부럽습니다.. 재밌게 잘 읽었습니다!

rjsduf0503 · 2022-11-20T12:33:02Z

src/utils/event.js

+const addEvent = ($target, eventType, selector, callback) => {
+  const children = [...$target.querySelectorAll(selector)];
+  const isTarget = (target) => children.includes(target) || target.closest(selector);
+
+  $target.addEventListener(eventType, (event) => {
+    if (!isTarget(event.target)) return false;
+    callback(event);
+  });
+};


event 추가하는 로직을 따로 빼내서 더 간결하게 짜셨군요.. 한 수 배우고 갑니다 :)

저도 배워야 할 점 같습니다. 저는 addEventListener를 엄청 남발해뒀는데 이렇게 로직을 따로 빼두면 코드를 더 간결하게 만들 수 있겠네요. 한 번 써먹어 보겠습니다!

rjsduf0503 · 2022-11-20T12:34:27Z

src/App.js

+import { routes } from "./constants/routes.js";
+
+export default function App({ $target }) {
+  this.$target = $target;


ask) 코드를 읽어보니 모두 this.$target = $target 으로 초기 설정을 하셨는데 이유가 뭔가요????? 궁금합니다.. 뭔가 규란님이라면 이유가 있어서 하셨을 것 같아서요 !!

rjsduf0503 · 2022-11-20T12:35:29Z

src/config.js

+export const USER = {
+  NAME: "규란",
+  PROFILE_URL:
+    "https://noticon-static.tammolo.com/dgggcrkxq/image/upload/v1603679366/noticon/dcvetqndre7gda3ttijy.gif",


ㅋㅋㅋㅋㅋ 무슨 프로필인가 궁금해서 들어갔는데 움직이는 강아지네요 웃고 갑니다

rjsduf0503 · 2022-11-20T12:37:05Z

src/components/component.js

@@ -0,0 +1,24 @@
+export default function Component({ $target, initialState }) {


p5) 현재 사용되지 않는 파일 같네요! 아마 클래스로 짜시다가 중간에 함수형으로 바꾸셨다고 했는데 그 과정에서 남기신게 아닐까.. 추측해봅니다!

rjsduf0503 · 2022-11-20T12:42:32Z

src/App.js

+    console.log(window.location.pathname);
+    console.log(/^\/documents\//.test(location.pathname));


로그 지우기~~!!!~!

rjsduf0503 · 2022-11-20T12:44:17Z

src/utils/debounce.js

@@ -0,0 +1,9 @@
+const debounce = (callback, delay) => {


debounce도 따로 빼서 만드신거 보고 감탄하고 갑니다..ㅎㅎ

metacode22

규란님 코드를 항상 보다보면 로직을 깔끔하게 처리할 뿐만 아니라 모듈화를 상당히 잘하시는 것 같아 정말 진심으로 배워가는 것이 많았습니다. 저도 황준일님이 작성하셨던 component 글을 봐서, 또 변수명과 코드 포맷팅을 잘 작성해주셔서 읽기 편했습니다!

그리고 5주간 ㅋㅋㅋ 저 때매 고생 많으셨습니다. ^ㅇ^ 코드 자주 훔쳐보러 오겠습니다.

metacode22 · 2022-11-21T04:01:33Z

src/utils/debounce.js

+  let inDebounce;
+  return (...args) => {
+    clearTimeout(inDebounce);
+    inDebounce = setTimeout(() => callback(...args), delay);


이렇게 arguments를 이용해서 쉽게 인자를 다음 함수에 전달할 수 있군요. 좋은 방법 알아갑니다!

metacode22 · 2022-11-21T04:02:30Z

src/utils/event.js

+const addEvent = ($target, eventType, selector, callback) => {
+  const children = [...$target.querySelectorAll(selector)];
+  const isTarget = (target) => children.includes(target) || target.closest(selector);
+
+  $target.addEventListener(eventType, (event) => {
+    if (!isTarget(event.target)) return false;
+    callback(event);
+  });
+};


저도 배워야 할 점 같습니다. 저는 addEventListener를 엄청 남발해뒀는데 이렇게 로직을 따로 빼두면 코드를 더 간결하게 만들 수 있겠네요. 한 번 써먹어 보겠습니다!

metacode22 · 2022-11-21T04:07:55Z

src/pages/MainPage.js

+  this.render = () => {
+    this.$target.innerHTML = `
+      <aside id="sidebar"></aside>
+      <main id="document-container"></main>


시멘틱 태그 사용을 적절히 잘하시는 것 같습니다!

createElement()가 아니라 요런 식으로 요소를 만들어 볼 수 있겠군요!

metacode22 · 2022-11-21T04:11:38Z

src/pages/MainPage.js

+      },
+    });
+  };
+


요건 그냥 궁금한 건데요! sidebar.setState나 document.setState로 해당 컴포넌트를 재구성하는 것이 아니라, 위와 같이 계속해서 새로운 인스턴스를 만들어 해당 컴포넌트를 다시 그리셨던 이유가 궁금하네요!

metacode22 · 2022-11-21T04:13:49Z

src/constants/routes.js

+  { path: /^\/documents\/new/, element: MainPage },
+  { path: /^\/documents\//, element: MainPage },
+  { path: /^\/404$/, element: NotFound },
+];


정규식 활용까지..! ㄷㄷ

와하... 멋있다... 저는 간단하게 if문으로 구현했는데... 배워갑니다.. (메모... 📝)

metacode22 · 2022-11-21T04:15:28Z

src/Router.js

+
+import { HISTORY_CHANGE_EVENT_NAME, routes } from "./constants/routes.js";
+
+export default function Router({ $target, onRoute }) {


혹시 Router 이렇게 생성자 함수를 통해 정의하신 이유가 따로 있나요??😀

저도 궁금해요! 👀

metacode22 · 2022-11-21T04:16:22Z

src/components/document/Document.js

+    if (documentId === "new") {
+      renderNewDocument($header, $body);
+    } else if (!!documentId) {
+      console.log(documentId);


꺄악! console.log!

metacode22 · 2022-11-21T04:23:55Z

src/components/document/DocumentHeader.js

+  };
+
+  this.setEvent = () => {
+    addEvent(this.$target, "keyup", "[name=title]", (event) => {


P5 : 저의 경우에는 event type을 keyup으로 해두면 화살표 키나 shift, caps lock 등 입력 값이 변하지 않았음에도 이벤트가 발생해서 유효하지 않은 api가 날라가더라구요. 그래서 input으로 바꿔서 입력값이 변할 때에만 api가 날라갈 수 있게 했었습니다.

그리고 이전에 React에서 겪은 적이 있는데, event type을 keyup으로 해두고 한글을 입력하면 이벤트가 중복해서 2번 발생하는 현상이 있었습니다. 실제로 검색해보니 javascript 자체의 문제더라구요. 한글 관련 에러여서 공식 자료도 찾기 어려웠었습니다. 이런 현상을 피하기 위해서 다른 event type을 고려해보시는 것도 좋을 것 같습니다!

p.s 과제에서 확인해보니, 여기선 keyup으로 해도 이벤트가 중복 발생하진 않네요!

reference : https://blog.naver.com/PostView.naver?blogId=bbak0105&logNo=222371386648&parentCategoryNo=&categoryNo=44&viewDate=&isShowPopularPosts=true&from=search

metacode22 · 2022-11-21T04:28:03Z

src/components/document/Document.js

+import API from "../../utils/api.js";
+import { USER } from "../../config.js";
+import { debounce } from "../../utils/debounce.js";
+import { getItemFromStorage, setItemToStorage } from "../../utils/storage.js";


import 구문도 잘 나누셔서 마음이 편안해집니다. ㅎㅎㅎ

metacode22 · 2022-11-21T04:31:43Z

src/App.js

+
+  this.route = () => {
+    const findMatchedRoute = () => routes.find((route) => route.path.test(location.pathname));
+    const Page = findMatchedRoute()?.element || NotFound;


제가 잘 못 찾는 것 같습니다! 혹시 Page가 어디서 어떻게 사용되는지 알 수 있을까요? element를 routes에서 찾고 나서 렌더링하려면 new Page()처럼 실행이 되어야 할 것 같은데 보이지 않아서요!

ask) 제가 아직 자바스크립트에 능통하지 못합니다.. findMatchedRoute()?.element에서 바로 뒤에 .연산자(?)가 붙으면 객체가 반환되는 건가요?

dmswl98

규란님!
항상 새벽까지 과제하시느라 수고많으셨습니다.
역시 깔끔함...! 🤯 규란님 코드보고 감탄만 하다가 갑니다..
그리고 배포해주셔서 꼭 자랑해주세요...

규란님 코드에서는 컴포넌트에 내부에 존재하는 코드가 모두 메서드나 함수로 감싸져있네요..?
전역으로 존재하는 변수가 없어서 컴포넌트 내부의 코드 실행 과정이 한 눈에 보여 더 좋은 것 같아요!

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

dmswl98 · 2022-11-21T08:12:16Z

src/Router.js

+
+import { HISTORY_CHANGE_EVENT_NAME, routes } from "./constants/routes.js";
+
+export default function Router({ $target, onRoute }) {


저도 궁금해요! 👀

dmswl98 · 2022-11-21T08:16:24Z

src/components/document/Document.js

+  const fetchDocument = async (documentId) => {
+    const response = await API.getDocuments(documentId);


API를 객체로 선언하시다니...! 이렇게 작성하면 실수를 줄일 수 있겠네요!😲

dmswl98 · 2022-11-21T08:19:31Z

src/components/document/DocumentContent.js

+  this.setEvent = () => {
+    addEvent(this.$target, "keyup", "[name=content]", (event) => {
+      onEdit(event.target.innerText, "content");
+    });
+  };


이벤트 리스너를 따로 함수로 만드신 건가요?! 이렇게 코드를 작성할 수도 있군요... 😮

dmswl98 · 2022-11-21T08:22:10Z

src/constants/routes.js

+  { path: /^\/documents\/new/, element: MainPage },
+  { path: /^\/documents\//, element: MainPage },
+  { path: /^\/404$/, element: NotFound },
+];


와하... 멋있다... 저는 간단하게 if문으로 구현했는데... 배워갑니다.. (메모... 📝)

Kal-MH

규란님! 마지막 코드리뷰를 하게 되어 아쉽습니다.
생성자함수를 사용해서 최대한 setState()를 덜 호출하게 하고 싶다고 했었는데
규란님 코드를 보면서 항상 표현하는 법을 많이 배웁니다:)

실제로 동작하는 부분까지 확인했을 때, 의도대로 잘 작동함을 확인할 수 있었습니다.
처음 진행하는 개인 프로젝트인데 수고 하셨어요:)

Kal-MH · 2022-11-21T08:33:43Z

src/App.js

+
+  this.route = () => {
+    const findMatchedRoute = () => routes.find((route) => route.path.test(location.pathname));
+    const Page = findMatchedRoute()?.element || NotFound;


ask) 제가 아직 자바스크립트에 능통하지 못합니다.. findMatchedRoute()?.element에서 바로 뒤에 .연산자(?)가 붙으면 객체가 반환되는 건가요?

Kal-MH · 2022-11-21T08:39:32Z

src/pages/MainPage.js

+  this.render = () => {
+    this.$target.innerHTML = `
+      <aside id="sidebar"></aside>
+      <main id="document-container"></main>


createElement()가 아니라 요런 식으로 요소를 만들어 볼 수 있겠군요!

Kal-MH · 2022-11-21T08:50:38Z

src/components/sidebar/Details.js

+    });
+
+    documents.forEach(({ id, title, documents }) => {
+      const $li = this.$target.querySelector(`[data-document-id="${id}"]`);


p5) SidebarBody에서는 findDocumentElement()로 따로 메서드를 만들었는데, 여기에도 적용해 볼 수 있을까요? 일종의 통일성을 위해서요

Kal-MH · 2022-11-21T09:05:32Z

src/components/sidebar/Sidebar.js

+        title: "Private",
+        rootDocuments: [...this.state.rootDocuments],
+      },
+      onAddButtonClick: addDocument.bind(this),


ask) 프로젝트 전반에 걸쳐 bind()가 사용되는 것이 보입니다. addDocument의 this가 생성된 Sidebar를 가리키고 있지 않을가 생각이 드는데, bind()를 사용함으로 얻어지는 효과에 대해서 알 수 있을까요?

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 이슈

gxxrxn added 5 commits November 16, 2022 23:24

feat: html, css 작성

5c7931d

- gitignore 설정

feat(Sidebar): Sidebar 컴포넌트 구현

0da15c1

- Sidebar 컴포넌트 - SidebarSection > Details > DocumentItem - SidebarFooter

feat(Document): Document 컴포넌트 구현

5e74b67

fix(DocumentItem): this.state를 가지도록 수정

9a7f4b2

feat(route): / 라우팅 기능 구현

f591bf4

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

gxxrxn requested a review from eastroots92 November 16, 2022 14:43

gxxrxn self-assigned this Nov 16, 2022

gxxrxn added 2 commits November 17, 2022 17:00

feat(router, document): document 라우팅, 수정 기능 구현

35bb3b3

refactor(route): App에서 페이지를 관리하도록 수정

c0f809c

rjsduf0503 reviewed Nov 20, 2022

View reviewed changes

metacode22 reviewed Nov 21, 2022

View reviewed changes

dmswl98 reviewed Nov 21, 2022

View reviewed changes

Kal-MH reviewed Nov 21, 2022

View reviewed changes

gxxrxn added 2 commits November 23, 2022 10:05

refactor(Main): 필요에 따라 document만 렌더링하도록 수정

5decc3d

refactor: 불필요한 this 바인딩, 함수 제거 및 분리

7454bd5

eastroots92 reviewed Dec 6, 2022

View reviewed changes

		@@ -0,0 +1,24 @@
		export default function Component({ $target, initialState }) {

		console.log(window.location.pathname);
		console.log(/^\/documents\//.test(location.pathname));


		import { HISTORY_CHANGE_EVENT_NAME, routes } from "./constants/routes.js";

		export default function Router({ $target, onRoute }) {

		const fetchDocument = async (documentId) => {
		const response = await API.getDocuments(documentId);

Conversation

gxxrxn commented Nov 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 과제 설명

실행방법

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

기본 요구사항

✅ PR 포인트 & 궁금한 점

Uh oh!

rjsduf0503 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metacode22 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmswl98 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kal-MH left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kal-MH Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

gxxrxn commented Nov 16, 2022 •

edited

Loading

Kal-MH left a comment •

edited

Loading

Kal-MH Nov 21, 2022 •

edited

Loading