[Mission5/이현준] Project_Notion_VanillaJs 과제 #56
Conversation
|
싱글톤 패턴이란 걸 처음 알아갑니당 과제하느라 고생하셨습니당👍👍 |
|
|
||
| async documentTitlePut({ id, title }) { | ||
| const { content } = this.state.documentContent; | ||
| await request(`/${id}`, { |
There was a problem hiding this comment.
request 함수의 인자로 들어간 URL이 /documents/${id} 가 아니라 ${id} 인 이유가 궁금합니다!
또한 documentTitlePut 메소드 에서는 content를 구조 분해 할당해서 사용하고,
documentContentPut 메소드에서는 title을 구조 분해 할당해서 사용하셨는데
이름과 동작이 반대인 것 같아서 궁금합니다!
There was a problem hiding this comment.
우선 좋은 질문 감사합니다!!
첫번째 질문인 request 함수의 인자로 들어간 URL이 /documents/${id} 가 아니라 ${id} 인 이유는
/documents/${id}로 진행하면 https://kdt-frontend.programmers.co.kr/documents/documents/89594 해당 url로 요청이 가게 되더군요... 저도 왜 저렇게 되는지 정말 궁금합니다... 그래서 일단 /${id} 해당 방식으로 진행 했습니다.
혹시 이유를 알 수 있을까요 ❔ ❔
두번째 질문은 title의 변경이 진행되면 현재 content는 유지가 되어야 하기 때문에 현재 state에서 content값을 구조 분해 할당해서 (현재 content, 변경 된 title) api PUT을 진행 했습니다. 반대로 content도 마찬가지 입니다!
| // <button class="action-style-btn" data-action="h3">h3</button> | ||
| // <button class="action-style-btn" data-action="h4">h4</button> | ||
| // <button class="action-style-btn" data-action="h5">h5</button> | ||
| // <button class="action-style-btn" data-action="h6">h6</button> |
There was a problem hiding this comment.
이 주석 부분은 사용되는 부분인지, 사용되지 않는 부분인지 궁금합니다!
만약 사용되지 않는 부분이라면 삭제해도 좋지 않을까요?
There was a problem hiding this comment.
시간이 부족해서 추후에 추가하려고 주석 처리만 진행해 놨습니다..!
| range.deleteContents(); | ||
| range.insertNode(wrapper); | ||
| range.setStartBefore(wrapper); | ||
| range.setEndAfter(wrapper); |
There was a problem hiding this comment.
오.. Range 객체에 이런 다양한 메소드들이 존재했었군요. 유용한 정보 배워갑니다!
|
|
||
| notifyEditor() { | ||
| this.editorSubscribers.forEach((subscriber) => subscriber()); | ||
| } |
There was a problem hiding this comment.
우와.. 이게 그 옵저버 패턴이라는 것인가요? 배워갑니다
|
고생많으셨어요 현준님!!! 잘 보고 배워갑니다. |
| range.deleteContents(); | ||
| range.insertNode(wrapper); | ||
| range.setStartBefore(wrapper); | ||
| range.setEndAfter(wrapper); |
| const action = target.dataset.action; | ||
|
|
||
| if (target.tagName === "BUTTON") { | ||
| switch (target.classList.value) { |
There was a problem hiding this comment.
저도 이번에 어느정도 케이스가 정해져있는 유형은 if문 대신 switch문을 사용해봤는데 더 깔끔해서 좋은 것 같아요! 현준님도 저랑 같은 생각을 하신 것 같아서 따봉 누르고 가겠습니다..👍👍
|
싱글톤 패턴 인상적이었습니다! 항상 다양한 방식으로 구현하려고 노력하시는 모습 멋있으세요!!👍 과제하느라 고생하셨습니다😆 |
| const title = event.target.value; | ||
| const id = $title.dataset.id; | ||
| store.documentTitlePut({ id, title }); | ||
| }, delay); |
|
|
||
| applyStyle(action) { | ||
| const textStyle = this.getSelectedTextStyle(); | ||
| const formattedElement = document.createElement("span"); |
There was a problem hiding this comment.
동작을 보면 계속 span이 추가되서 잘 작동하지 않는 것 같은데 테스트가 필요할 것 같습니다.
| string = this.createCustomListString(documents, string); | ||
| } | ||
| string += "</ul>"; | ||
| return string; |
There was a problem hiding this comment.
조금 가독성을 높이도록 리팩토링 해보면 좋을 것 같은 부분입니다.
크게 style을 지정하는 곳, document 엘리먼트를 추가하는 곳, 재귀 순회하는 곳으로 나뉘는 것 같은데 이 부분들의 역할이 분명히 보이게 함수를 분리할 수도 있을 것 같네요.
| const style = store.state.selectedStyles; | ||
| document.execCommand(style); | ||
| store.state.selectedStyles = ""; |
There was a problem hiding this comment.
스토어에서 선택된 스타일도 관리하시는 군요-!! store 상태를 직접 변경하는 거랑 setState 함수로 변경하는 거랑 어떤 방법이 더 좋은지 궁금하네요 👀👀
| export default class DocumentAddBtn { | ||
| constructor({ $target }) { | ||
| this.$target = $target; | ||
| this.$button = document.createElement("form"); |
There was a problem hiding this comment.
createElement를 button말고 form으로 하신 이유가 궁금합니당-!
| targetDocument.style.transform = "rotate(-90deg)"; | ||
| } else if (folded === "false") { | ||
| const foldedList = getItem("folded", []); | ||
| foldedList.splice(foldedList.indexOf($targetParent.id), 1); |
| string += `<span id=${currentDocumentId} class="select-document" ${selectStyle}> | ||
| ${ | ||
| nextDocument.length | ||
| ? `<img class="toggle-folder" ${imgStyle} src=${arrowImg} alt="arrow.svg"/>` | ||
| : "" | ||
| } | ||
| <img src=${documentImg} alt="document.svg"/> | ||
| <span id=${currentDocumentId} class="document-list-title"> |
There was a problem hiding this comment.
id는 unique한 값이라 data-id로 하면 좋을꺼 같습니당-!
| const htmlString = this.state.reduce((acc, doc) => { | ||
| return this.createCustomListString(doc, acc); | ||
| }, ""); |
| const currentTime = new Date().toISOString(); | ||
|
|
||
| clearTimeout(timer); | ||
| setItem(this.documentId, { content, saveTime: currentTime }); |
There was a problem hiding this comment.
제가 기억하기론 API에 createdAt과 updatedAt으로 생성시간과 수정시간이 기록되어 있기때문에 저장 시간을 설정하고 싶으셨다면 없어도 되지 않을까요?🤗
| this.$list.addEventListener("mouseover", (event) => { | ||
| const targetDocument = event.target; | ||
|
|
||
| if (targetDocument.documentListEditor) { | ||
| targetDocument.documentListEditor.show(); | ||
| return; | ||
| } | ||
|
|
||
| if (targetDocument.matches(".select-document")) { | ||
| const documentListEditor = new DocumentListEditor({ | ||
| $target: targetDocument, | ||
| }); | ||
|
|
||
| targetDocument.documentListEditor = documentListEditor; | ||
|
|
||
| targetDocument.addEventListener("mouseleave", () => { | ||
| targetDocument.documentListEditor.hide(); |
There was a problem hiding this comment.
저는 토글버튼을 이용했는데 클릭이벤트를 이용해 hide, show를 하는 방법이 있었군요. 배워갑니다!!
📌 노션 클로닝 프로젝트
프로젝트 링크
노션 클로닝 이지만 노션과 다른 점이 많습니다....
👩💻 요구 사항과 구현 내용
✅ PR 포인트 & 궁금한 점
store를 이용해서 싱글톤 패턴으로 진행했습니다. 모든 state가 store에 있으며, 관리되게 진행했습니다.
기능에 욕심을 둬서 가독성이 크게 떨어지는 코드를 작성한거 같아 아쉬움이 많습니다.
시간이 있었더라면 리팩토링과 각 컴포넌트의 의존성을 없앨 수 있었을 것 같습니다. 또한, XSS처리하지 못한 것과 editor Tool의 기능적 버그가 많아 아쉬움이 있습니다.
싱글톤 패턴이 잘 사용된 것인지 그리고 코드에 아쉬운 부분이 있다면 의견 주시길 바랍니다.
또한 라우터 관리가 잘된 것인지 궁금합니다.