-
Notifications
You must be signed in to change notification settings - Fork 6
[뉴스 뷰어 페이지 2단계] 이수진 미션 제출합니다. #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: sujin0707
Are you sure you want to change the base?
Changes from 4 commits
ff92039
3c59dbb
34a39a1
c69c9d1
4c02086
f561487
c92f379
be78885
3b7d32f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,8 @@ <h1>LINKHU<span>-NEWS</span></h1> | |
| </div> | ||
| <div id="modal" class="modal"> | ||
| <div class="modal-content"> | ||
| <span class="close-modal">×</span> | ||
| <div class="close-modal">×</div> | ||
| <div id="modal-header" class="modal-header"></div> | ||
| <h2>의견 남기기</h2> | ||
| <div class="model-section"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사소하지만 model이라고 오타가 발생한 것 같아요!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오타 수정하겠습니다 |
||
| <form id="opinion-form"> | ||
|
|
@@ -29,7 +30,7 @@ <h2>의견 남기기</h2> | |
| <div> | ||
| <label for="opinion-content">내용</label> | ||
| <div> | ||
| <textarea id="opinion-content" name="opinion-content" rows="4%" cols="45%" required></textarea> | ||
| <textarea id="opinion-content" name="opinion-content" rows="20%" cols="45%" required></textarea> | ||
| </div> | ||
| </div> | ||
| <button type="button" id="save-opinion">저장</button> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,61 +68,92 @@ class App { | |
| const modal = document.getElementById("modal"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 전체적으로 App클래스에서 담당하는 일이 너무 많아진 것 같아요
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 메소드에 부여된 기능이 너무 많습니다. (메소드 하나 당 하나의 기능을 가져도록 연습해보세요!)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네 알겠습니다! 다음 미션부터 기능별로 메소드 단위를 작게 나눠보겠습니다 |
||
| const closeModal = document.querySelector(".close-modal"); | ||
| const opinionForm = document.getElementById("opinion-form"); | ||
| const opinionTitleInput = document.getElementById("opinion-title"); | ||
| const opinionContentTextArea = document.getElementById("opinion-content"); | ||
| const opinionTitle = document.getElementById("opinion-title"); | ||
| const opinionContent = document.getElementById("opinion-content"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 다음과 같이 html에 직접적으로 파일을 작성하다보니 modal의 요구사항이 변경되면 유지보수가 불편해질 수 있어요. 예를 들어 모달의 form에 태그도 작성할 수 있도록 입력창을 추가하라는 요구사항이 생겼다면 html의 내용을 수정하고 index 파일내에서 코드를 처음부터 다시 읽고 해당 단락을 찾아 내용을 추가해야 할 겁니다. 파일의 구조가 복잡해지고 코드의 작성자가 아니라면 유지 보수를 하기가 까다롭겠죠? 코드 작성시 이런 점을 유의해서 코드를 작성하면 좀 좋은 코드를 작성할 수 있을거라 생각해요
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 알겠습니다! 해당 부분대로 html에 작성하는 걸 지양하는 방식으로 수정하려 했는데 계속해서 오류가 생겨서 일단 이 부분은 수정하지는 못했습니다 ㅠㅠ 이후에는 이런 방식으로 작성하지 않도록 해보겠습니다! |
||
|
|
||
| modal.style.display = "block"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. css관련 내용은 style파일에 보관하는 걸 추천드립니다.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. css파일에 보관되도록 수정하겠습니다! |
||
|
|
||
| const storedOpinion = localStorage.getItem(articleTitle); | ||
| if (storedOpinion) { | ||
| const parseOpinion = JSON.parse(storedOpinion); | ||
|
|
||
| opinionTitleInput.value = parseOpinion.title || ""; | ||
| opinionContentTextArea.value = parseOpinion.content || ""; | ||
| } else { | ||
| opinionTitleInput.value = ""; | ||
| opinionContentTextArea.value = ""; | ||
| } | ||
| closeModal.addEventListener("click", () => { | ||
| modal.style.display = "none"; | ||
| }); | ||
|
|
||
| const saveOpinionButton = document.getElementById("save-opinion"); | ||
| saveOpinionButton.addEventListener("click", () => { | ||
| const opinion = { title: opinionTitleInput.value, content: opinionContentTextArea.value }; | ||
|
|
||
| localStorage.setItem(articleTitle, JSON.stringify(opinion)); | ||
| modal.style.display = "none"; | ||
|
|
||
| }); | ||
|
|
||
| window.addEventListener("click", (event) => { | ||
| if (event.target === modal) { | ||
| modal.style.display = "none"; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 모달의 backdrop을 클릭했을 때 모달이 닫히도록 하는 기능을 구현해주셨네요
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 감사합니다 |
||
| }); | ||
|
|
||
| const storedOpinion = localStorage.getItem(articleTitle); | ||
|
|
||
| if (storedOpinion) { | ||
| const parseOpinion = JSON.parse(storedOpinion); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 단락은 별도의 유틸함수로 분리할 수 있겠네요
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. modal.js 파일을 따로 만들고, 해당 부분 별도의 함수로 분리하겠습니다. |
||
|
|
||
| const editOpinionButton = document.getElementById("edit-opinion"); | ||
| if (editOpinionButton) { | ||
| editOpinionButton.remove(); | ||
| } | ||
| const SavedOpinion = document.getElementById("modal-header"); | ||
| SavedOpinion.innerHTML = ` | ||
| <h3>${parseOpinion.title}</h3> | ||
| <p>${parseOpinion.content}</p> | ||
| `; | ||
|
|
||
| opinionTitle.value = parseOpinion.title; | ||
| opinionContent.value = parseOpinion.content; | ||
|
|
||
| const saveButton = document.getElementById("save-opinion"); | ||
| let editButton = document.getElementById("edit-opinion"); | ||
| let deleteButton = document.getElementById("delete-opinion"); | ||
|
|
||
| const editButton = document.createElement("button"); | ||
| editButton.textContent = "수정하기"; | ||
| editButton.id = "edit-opinion"; | ||
| if(saveButton) { | ||
| saveButton.remove(); | ||
| } | ||
|
|
||
| editButton.addEventListener("click", () => { | ||
| editButton.remove(); | ||
| if (!editButton) { | ||
| editButton = document.createElement("button"); | ||
| editButton.textContent = "edit"; | ||
| editButton.id = "edit-opinion"; | ||
|
|
||
| editButton.addEventListener("click", () => { | ||
| const opinion = { title: opinionTitle.value, content: opinionContent.value }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 편집하는 기능에 기존 정보를 삭제하는 내용이 존재하지 않는 것 같아요 추가적으로 편집 시 작성된 내용이 없으면 정보를 저장하지 않도록 하는 과정도 필요하다고 생각해요 현재 작성된 내용이 없으면 오류가 발생하는 경우가 있습니다.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 피드백 주신 부분대로 수정하였습니다! |
||
| localStorage.setItem(articleTitle, JSON.stringify(opinion)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 기사의 이름으로 키값으로 설정하는 건 부적절하다고 생각합니다.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그 부분은 미처 고려하지 못했습니다. 기사의 url을 키값으로 설정하는 것이 더 좋아 보여서 이렇게 변경하겠습니다! |
||
| modal.style.display = "none"; | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 코드 단락에는 event.preventDefault() 메소드가 필요합니다.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 메소드는 이벤트 동작과 관련하여 사용되는 메소드로 브라우저의 기본 동작을 실행하지 않도록 합니다. 해당 메소드를 사용함으로써 버튼을 클릭할 때 의도치 않게 페이지가 새로고침되거나 이동하지 못하도록 방지할 수 있기 때문에 필요한 것 같습니다. |
||
|
|
||
| opinionForm.appendChild(editButton); | ||
| } | ||
|
|
||
| if (!deleteButton) { | ||
| deleteButton = document.createElement("button"); | ||
| deleteButton.textContent = "delete"; | ||
| deleteButton.id = "delete-opinion"; | ||
|
|
||
| deleteButton.addEventListener("click", () => { | ||
| localStorage.removeItem(articleTitle); | ||
| modal.style.display = "none"; | ||
| }); | ||
|
|
||
| opinionForm.appendChild(deleteButton); | ||
| } | ||
| } else { | ||
| opinionTitle.value = ""; | ||
| opinionContent.value = ""; | ||
|
|
||
| opinionTitleInput.disabled = false; | ||
| opinionContentTextArea.disabled = false; | ||
| const saveButton = document.getElementById("save-opinion"); | ||
| const editButton = document.getElementById("edit-opinion"); | ||
| const deleteButton = document.getElementById("delete-opinion"); | ||
|
|
||
| saveOpinionButton.textContent = "수정 완료"; | ||
| }); | ||
| saveButton.textContent = "save"; | ||
| saveButton.addEventListener("click", () => { | ||
| const opinion = { title: opinionTitle.value, content: opinionContent.value }; | ||
| localStorage.setItem(articleTitle, JSON.stringify(opinion)); | ||
| modal.style.display = "none"; | ||
| }); | ||
|
|
||
| saveOpinionButton.textContent = "저장"; | ||
| if (editButton) { | ||
| editButton.remove(); | ||
| } | ||
|
|
||
| opinionForm.appendChild(editButton); | ||
| if (deleteButton) { | ||
| deleteButton.remove(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모달 html요소를 직접적으로 html 내에 작성하기 보다는 별도로 js파일을 만들어 작성하는걸 추천드립니다.
html내에는
<div id="root-modal">과 같이 진입점만 존재해도 될 것 같아요