[Mission4/원다연] - Project_Notion_VanillaJS #32
Conversation
| <textarea name='content' class='content' placeholder='내용을 입력하세요.'>${ | ||
| !content ? '' : content | ||
| }</textarea> |
There was a problem hiding this comment.
textarea 태그 보다는
div 태그를 활용해서
에디터를 만들면 더 좋았을것 같아요! ㅎㅎ
| .querySelector(`[data-id='${this.state.id}']`) | ||
| .querySelector('span'); |
There was a problem hiding this comment.
querySelector를 두 번 사용한 이유가 있을까요?!
| <input type='text' name='title' class='title' placeholder='제목 없음' value='${ | ||
| title === '제목 없음' ? '' : title | ||
| }'/> | ||
| <textarea name='content' class='content' placeholder='내용을 입력하세요.'>${ | ||
| !content ? '' : content |
There was a problem hiding this comment.
코드가 엄청 깔끔하고 보기 편하게 작성이 된 것 같아요! 저도 보고 배웠습니다! ㅎㅎㅎ
There was a problem hiding this comment.
다연님 코드 잘봤습니다 ㅎ
전체적으로 코드가 깔끔해서 노력한 흔적들이 잘 보였습니다
다연님 질문에 대한 제 생각을 적어 보겠습니다 !
-
새로고침을 하면 리렌더링 되면서 Document 리스트가 루트 title만 보여지는데, 이 경우를 막기 위해서는 어떤 방법이 있을지 궁금합니다.
=> 저도 이것때문에 많이 고민해서 전체 HTML 을 다 localStorage 에 저장해서 풀었는데, 다른사람들 코드들을 보니 localStorage에 열려있는 요소 id 배열을 저장해놔서 그것을 토대로 렌더링 하는 것 같더라고요. 아니면 이번 고양이 사진첩에 있던 paths 느낌으로 state에 추가해도 좋을 것 같아요. 저도 참고 해보려고 합니다 😊 -
DocumentFooter에서 onCreate과 onScroll 함수가 있는데, 제가 의도한 것은 onCreate(새 페이지 생성)이 실행되고 나서 onScroll 함수가 작동하는 것인데 아무래도 비동기 처리로 인해 실행 순서가 반대로 나옵니다. 어떻게 하면 해결할 수 있을까요?
=> 저는 이런 부분에서 낙관적 업데이트를 사용할 수 있을 것 같습니다. 일단 돔요소를 만들어주고 요청이 잘됬으면 유지? 이런느낌으로 가면 좋을 것 같아요. 그냥 의견이라서 정답인지는 모르겠습니다..ㅎ -
중복되는 코드를 최대한 없애보려고 했지만 혹시 코드 내에 보인다면 여러 방법으로 중복되는 코드를 줄일 수 있는 방법에 대해 리뷰 남겨주시면 감사하겠습니다 ㅎㅎ
=> 고민 많이 하시고 작성하신것 같아 많이 없는 것 같아요 딱 그 DocumentList.js의 pathname 찾는 부분을 어떻게 바꾸면 한번 더 pathname 검사를 안 할 수 있지 않을까? 하는 생각을 하긴 했습니다 ㅎ
| if (className === 'toggle' || className === 'toggle toggle-active') { | ||
| onToggle(); | ||
| } else if (className === 'delete') { | ||
| onRemove(); | ||
| } else if (className === 'plus add') { | ||
| onAdd(); | ||
| } else { | ||
| onSet(); |
There was a problem hiding this comment.
| if (className === 'toggle' || className === 'toggle toggle-active') { | |
| onToggle(); | |
| } else if (className === 'delete') { | |
| onRemove(); | |
| } else if (className === 'plus add') { | |
| onAdd(); | |
| } else { | |
| onSet(); | |
| const {classList} = e.target | |
| ... | |
| if (classList.contains('toggle')) { | |
| onToggle(); | |
| } else if (classList.contains('delete')) { | |
| onRemove(); | |
| } else if (classList.contains('add')) { | |
| onAdd(); | |
| } else { | |
| onSet(); |
이런식으로 하면 조건을 줄일 수 있다고 생각합니다!
| initialState = { title: '', content: '' }, | ||
| onEditing, | ||
| }) { | ||
| const $div = document.createElement('div'); |
There was a problem hiding this comment.
| const $div = document.createElement('div'); | |
| const $editor = document.createElement('div'); |
$div의 이름을 좀 더 명확하게하면 좋을 것 같아요!
| onScroll: () => { | ||
| // 문제 있음 - onCreate가 실행된 이후에 실행시켜야함 | ||
| const $div = $target.querySelector('.document-list'); | ||
| $div.scrollTo({ top: $div.scrollHeight, behavior: 'smooth' }); | ||
| console.log('onScroll'); | ||
| }, |
| const { pathname } = window.location; | ||
| if (pathname.indexOf('/documents/') === 0) { | ||
| const [, , documentId] = pathname.split('/'); | ||
| if (documentId === id) { | ||
| push('/'); | ||
| } | ||
| } |
There was a problem hiding this comment.
App.js의 pathname검사 부분과 비슷한데 어떻게 재사용 할 수 있는 방법이 있지 않을까요?
There was a problem hiding this comment.
안녕하세요 :)
-
페이지 접근이 되지 않습니다.
variable.js로 api 접근이 가능한것 같던데, gitignore 처리가 되어있어 확인자체가 불가능합니다..
netlify 등으로 페이지 배포가 되어있으면 그걸로 확인이 가능할테고 아니라면,
api 주소부분은 nodeenv 등을 활용하게끔 해두고 .env.tmpl 등의 파일로 변수명칭 등을 공유하거나 하여야 할 것으로 보여집니다.
이러한 방식을 사용해야 다른개발자가 혼동되지 않고 사용할 수 있습니다~! -
제목없음 페이지 생성 후 삭제할 경우
DELETE요청이 아닌GET요청을 보내고 있습니다.
해당 요청에 대한 예외처리가 되어 있지 않은 것 같습니다 :) -
부모와 자식이 연결된 상태에서 부모를 삭제하면 자식부분은 보이지가 않게되는데 의도하신 건가요?
아니면 자식이 root 부분에 표시되는 형태여야하는데 잘못반영된건가요?
아마도 자식페이지 자체는 살아있지만 부모페이지로의 연결점을 찾지못해서 표현되지 않는 거 아닌가 생각되어서 말씀드렸습니다 :)
보통 노션프로젝트를 볼때 디자인적인 부분도 같이 살펴보면서 리뷰를 드릴려고 하는데, 다연님의 경우 추가적으로 디자인관련 리뷰를 드릴 부분은 찾지못한 것 같습니다 💯
끝으로 스크롤관련 부분에 대해서 개선점을 찾아낸다면 한번 더 공유해주시면 좋을 것 같습니다 👍
|
|
||
| export const request = async (url, options = {}) => { | ||
| try { | ||
| const res = await fetch(`${API_END_POINT}${url}`, { |
There was a problem hiding this comment.
취향차이일수도있지만 저는 response 처럼 끝까지 사용해주는 변수명칭이 좀 더 좋다고 생각합니다 👍
| $document.className = 'document-container'; | ||
| $editor.className = 'editor-container'; | ||
|
|
||
| $target.appendChild($document); |
There was a problem hiding this comment.
document 컨테이너 생성 및 editor 컨테이너 생성에 대한 함수를 만들어서 호출하는 형태로 하면 더 좋지 않을까요? 동일 코드의 반복같습니다 :)
| this.render = () => { | ||
| $button.innerHTML = ` | ||
| <img class='plus new' /> | ||
| ${'새 페이지'} |
There was a problem hiding this comment.
${} 이 부분은 변수를 사용할 때 사용하는 구문인데 필요없어 보입니다 :)
| onCreate(); | ||
| onScroll(); |
There was a problem hiding this comment.
이게 지난번 개인면담때 질문하셨던 코드인것 같네요. 혹시 그때 제가 말씀드린 부분에 맞추어서 추가적인 테스트 등 진행해보셨을까요? ㅎㅎ
| $ulChild.innerHTML = ` | ||
| <li style='pointer-events: none;'> | ||
| <div class='document'> | ||
| <span class='no-child' style='color: #a7a7a7;'>${`하위 페이지 없음`}</span> |
There was a problem hiding this comment.
${} 여기에도 비슷한 구문이 있네요. 혹시 이유가 있는건가요? ㅎㅎ
| onToggle(); | ||
| }; | ||
|
|
||
| if (className === 'toggle' || className === 'toggle toggle-active') { |
There was a problem hiding this comment.
ifelse ifelse if 가 계속해서 반복되면 switch 문으로 변경해도 좋을 것 같습니다.
| body: JSON.stringify(newDocument), | ||
| }); | ||
| await fetchDocument(); | ||
| console.log('onCreate'); |
| $target, | ||
| initialState: this.state.post, | ||
| onEditing: (state) => { | ||
| console.log(state); |
📌 과제 설명
바닐라JS로 구현한 Notion 클로닝 프로젝트
👩💻 요구 사항과 구현 내용
기본 요구 사항
추가 구현 사항
구조
ScreenShot
✅ PR 포인트 & 궁금한 점