[Mission4/최별] - Project_Notion_VanillaJS#34
[Mission4/최별] - Project_Notion_VanillaJS#34choibyeol wants to merge 9 commits intoprgrms-fe-devcourse:3/#4_choibyeol_workingfrom
Conversation
LucyYoo
left a comment
There was a problem hiding this comment.
안녕하세요 별님! 같은 유리팀 유지영입니다👍
직접 실행을 해보니, 삭제 버튼을 눌렀을 때 루트 document로 돌아가지만 새로고침하지 않으면 편집기에 적용이 안되더라구요! 이 부분도 추가되면 더욱 좋을 것 같아요 :)
첫 개인 프로젝트 너무 수고하셨습니다! 끝까지 마무리하셔서 과제 제출하시는 모습이 감동받았습니다><
|
|
||
| .add-btn { | ||
| position: absolute; | ||
| padding: 0.5rem 1rem; |
There was a problem hiding this comment.
add-btn 과 delete-btn의 padding이 겹쳐있어서 + 버튼을 누르면 삭제버튼이 눌리는 경우가 생기네용!! padding값을 조절해주면 좋을 것 같아요! 😊
| if (isNew) { | ||
| const createdPost = await request("/documents", { | ||
| method: "POST", | ||
| body: JSON.stringify(post), | ||
| }); | ||
| history.replaceState(null, null, `/documents/${createdPost.id}`); | ||
| removeItem(postLocalSaveKey); | ||
|
|
||
| this.setState({ | ||
| postId: createdPost.id, | ||
| }); |
There was a problem hiding this comment.
새 페이지를 추가하거나 하위문서가 추가되면 바로 postId로 주소를 넘겨서 postId가 new일 때의 로직은 사용되지 않아도 될 것 같아요! 😊
| editor.setState( | ||
| this.state.post || { | ||
| title: "", | ||
| content: "", | ||
| } | ||
| ); |
There was a problem hiding this comment.
동일한 문서를 재클릭했을 때 this.state.post 값이 undefined여서 기존의 제목과 내용이 뜨지 않는 것 같아요!
if(this.state.post){
editor.setState(this.state.post)
}이런 식으로 undefined일 땐 아예 setState가 일어나지 않도록 하는 것도 방법일 것 같아요! 👍
|
|
||
| $sidebarBody.addEventListener("click", (e) => { | ||
| const target = e.target; | ||
| const dataId = target.closest("li").dataset.id; |
There was a problem hiding this comment.
이부분도 고민 중이었는데 일단 실행은 되니까 넘어갔었어요 핳ㅎㅎ.. 좋은 방법 감사합니다!!
| const { postId } = this.state; | ||
|
|
||
| if (postId !== "new") { | ||
| const post = await request(`/documents/${[postId]}`); |
There was a problem hiding this comment.
postId가 배열로 들어가는 이유가 따로 있을까용?? 😊
| this.setState = (nextState) => { | ||
| this.state = nextState; | ||
| $editor.querySelector("[name=title]").value = this.state.title; | ||
| $editor.querySelector("[name=content]").innerHTML = this.state.content; |
There was a problem hiding this comment.
페이지의 내용을 수정하고 다른 list를 누르면 content 내용이 해당 list의 content가 아니라 수정했던 이전 list의 content가 계속 떠 있더라구요! 아마 이 부분이 value가 아니라 innerHTML로 되어있어서 그런 것 같습니다 😊
| <html> | ||
| <head> | ||
| <title>Star notion</title> | ||
| <link rel="stylesheet" href="src/style/style.css" /> |
There was a problem hiding this comment.
특정 document를 클릭하고 새로고침을 하면 css가 날라가서 찾아보니! css도 스크립트처럼 상대경로가 아닌 절대경로로 지정해주니 해결되는 것 같네요! 😊
<link rel="stylesheet" href="/src/style/style.css" />
There was a problem hiding this comment.
와 그렇구나 ㅠㅠㅠ 절대 경로로 지정해주는 습관 들여야겠어요...
| // const fetchNewPost = async (postId) => { | ||
| // push(`/documents/${postId}`); | ||
| // }; |
There was a problem hiding this comment.
주석은 삭제하는 습관을 들이는게 좋겠어요! git이 관리해주니까요:-)
| /* 연속으로 입력을 하고 있을 때는 계속 이벤트 발생을 지연시키다가 | ||
| 입력을 멈췄을 때, 즉, 마지막으로 이벤트가 발생하고 일정 시간이 지났을 때 | ||
| 지연시켰던 이벤트를 실행시키는 것 - 디바운스 | ||
| 디바운스를 이용하면 이벤트 발생하는 횟수를 줄일 수 있다. -> 성능, 최적화 | ||
| */ |
| $addBtn.innerHTML = "+"; | ||
| const $deleteBtn = document.createElement("button"); | ||
| $deleteBtn.className = "delete-btn"; | ||
| $deleteBtn.innerHTML = "x"; |
Heojiyeon
left a comment
There was a problem hiding this comment.
별님 안녕하세요!
사이드 바를 header, body, footer로 쪼갠 점이 인상 깊었고, 지영님께서 엄청 꼼꼼히 달아주셔서 저는 살짝 수정하면 좋을 내용 적고 갑니다~
수고 많으셨습니다 :)
| export default function PostEditPage({ $target, initialState }) { | ||
| const $postEditPage = document.createElement("div"); | ||
| $postEditPage.className = "post-edit-page"; | ||
|
|
||
| this.state = initialState; | ||
|
|
||
| let postLocalSaveKey = `temp-post-${this.state.postId}`; | ||
|
|
||
| const post = getItem(postLocalSaveKey, { |
There was a problem hiding this comment.
전반적으로 new 방어코드 처리를 추가해주시면 좋을 것 같습니다 :)
| /* | ||
| sidebar 배경색: rgb(250, 250, 249) | ||
| sidebar hover 색: rgb(230, 230, 230) | ||
| sidebar 눌러진 페이지: rgb(238, 238, 238) | ||
| */ |
KSoonYo
left a comment
There was a problem hiding this comment.
프로젝트하느라 고생많으셨습니다!
코드 보면서 간단하게 의견 남겨보았습니다~
| this.render = () => { | ||
| $target.appendChild($sidebar); | ||
| $sidebar.appendChild($sidebarHeader); | ||
| $sidebar.appendChild($sidebarBody); | ||
| $sidebar.appendChild($sidebarFooter); | ||
| }; | ||
|
|
There was a problem hiding this comment.
현재 sidebar를 컨테이너로 하신 것 같은데, 그러면 하위의 sidebarHeader, sidebarBody, sidebarFooter 에 타겟으로 sidebar 컨테이너를 넘겨주기만 해도 이러한 render 함수는 생략할 수 있지 않을까요?
new를 하는 시점에서 sidebar 컨테이너에 append가 되고, 어차피 렌더링은 innerHTML로 처리하니까요! 혹시 이후에 render 함수를 쓰는 일이 있나 봤는데 없는 것 같아서 말씀드려봅니당
| setState: this.setState(), | ||
| }); | ||
|
|
||
| new SidebarFooter({ | ||
| $target: $sidebarFooter, | ||
| setState: this.setState(), |
There was a problem hiding this comment.
함수를 props로 전달할 때에는 호출하지 않고 넘겨주셔야 더 안전할 것 같아요! this.setState만 넘겨주고 하위 컴포넌트에서 실행이 필요한 경우에만 setState() 로 실행하는 게 맞아 보입니다.
그리고 부모 컴포넌트의 setState를 자식 컴포넌트에서 실행하게 되면 나중에 상태관리가 꼬이게 되니까 다른 방법을 고민해봐야 할 것 같아요. 강의에서처럼 onClick과 같은 트리거 함수를 정의해서 자식에게 전달하고, 그 트리거 함수를 실행하면 부모 컴포넌트에서 setState를 실행하는 게 더 좋지 않을까 의견드려봅니다!
There was a problem hiding this comment.
제 생각에도 append 목적으로는 각 컴포넌트에서 해주는게 좋을것 같고
상위 setState를 통해 하위 state를 관리하는 방향이 더 좋을것 같습니다~
만약 상위의 상태값을 하위에서 바꿔야하는 로직(이벤트)이 있다면 해당 setState를 가져오는게 아닌 해당 로직을 호출하면서 내부에서 변경하는게 좋을것 같습니다~
//ex
new SidebarFooter ({
onClick : () => {
const nextState = 변경로직
this.setState(nextStatee)
}
})| const $sidebar = document.createElement("div"); | ||
| const $postEditPage = document.createElement("div"); | ||
|
|
||
| $target.appendChild($sidebar); | ||
| $target.appendChild($postEditPage); | ||
|
|
|
|
||
| this.render(); | ||
|
|
||
| $editor.addEventListener("keyup", (e) => { |
There was a problem hiding this comment.
e를 받아올때 한번에 {target}으로 분리해주는것도 좋을것 같아요~
| setState: this.setState(), | ||
| }); | ||
|
|
||
| new SidebarFooter({ | ||
| $target: $sidebarFooter, | ||
| setState: this.setState(), |
There was a problem hiding this comment.
제 생각에도 append 목적으로는 각 컴포넌트에서 해주는게 좋을것 같고
상위 setState를 통해 하위 state를 관리하는 방향이 더 좋을것 같습니다~
만약 상위의 상태값을 하위에서 바꿔야하는 로직(이벤트)이 있다면 해당 setState를 가져오는게 아닌 해당 로직을 호출하면서 내부에서 변경하는게 좋을것 같습니다~
//ex
new SidebarFooter ({
onClick : () => {
const nextState = 변경로직
this.setState(nextStatee)
}
})| $renderList.innerHTML = ""; | ||
| this.render(); |
There was a problem hiding this comment.
이 부분 로직은 계속 중복 되는것 같습니다!! State 랜더를 담당하는 부분까지 역할이 과중 된것 같습니다

📌 과제 설명
바닐라 JS만을 이용해 노션을 클로닝합니다.
기본적인 레이아웃은 노션과 같으며, 스타일링, 컬러값 등은 원하는대로 커스텀합니다.
동작 모습
파일 구조
👩💻 요구 사항과 구현 내용
기본 요구 사항
보너스 요구사항
개선해야 할 부분
새롭게 리뷰 받은 부분
✅ 피드백 반영사항
✅ PR 포인트 & 궁금한 점