-
Notifications
You must be signed in to change notification settings - Fork 88
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
[2단계 - 상세 정보 & UI/UX 개선하기] 초코(강다빈) 미션 제출합니다. #171
Changes from 1 commit
58a3ce0
5661a8a
022dbf2
f3135ea
52f9c3a
553471b
a9e23b3
a12194e
40846a2
00f7a69
dab4109
c67458d
716b456
4e07ad5
5057566
2bbc6f4
da29495
f74a933
0b51801
568b3c4
e2b5ccc
5622b4c
0696960
04750f0
1ca0616
97c0763
98ee73b
22dc6ad
b09291f
3755207
8cf6096
fa81b18
6d4fef8
4273068
a817135
a96aac3
11532ab
02db706
75004f9
28af29a
88c2cc7
acda9b6
b27638f
ea1b8a3
7b4d067
2094ff8
83c4cd4
24db597
9cb0f27
e2f6ecd
5399034
64dd479
065f2c6
a6111ca
cefc87f
f109faa
2cc746c
1b4ea09
0fbc146
1c2c72e
af8d8e8
b1e8768
28af320
5569bdb
ccb81ff
afa3b94
0148e13
7207d52
5fb1acf
73a10cf
2d77149
c1a406b
46943e3
15111d6
2dc4077
779cebb
485910f
7e3748e
de041e3
dc31890
8209206
1c9ff19
6c861dd
4eaa732
6707341
b64fdfc
6864ff6
4dac6b6
0330025
973037a
9709602
a1a8443
9858971
8cc5075
06cad8e
48726d8
5cdec22
4e169ca
bff9d97
c846053
4542fb2
722cb70
2fadb45
cb164ab
fa80a28
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 |
---|---|---|
|
@@ -5,6 +5,13 @@ import NoImage from '../images/no-image.png'; | |
import StarFilled from '../images/star_filled.png'; | ||
import StarEmpty from '../images/star_empty.png'; | ||
|
||
interface ModalContent { | ||
title: string; | ||
posterPath: any; | ||
genres: string; | ||
voteAverage: string; | ||
overview: string; | ||
} | ||
export default class Modal { | ||
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 클래스의 역할(모달 생성, 오픈, 클로즈, 영화 데이터 넣어주기, vote 영역 관리) 이 너무 방대한 것 같습니다. 이를 리팩토링한다면 모달 오픈, 클로즈 / 영화 데이터 넣어주기 + vote 관리 로 생각 중인데, 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. 테스트를 기준으로 분리를 많이 하는거 같아요. 컴포넌트 단위의 테스트를 진행할 때 Testable 한 지를 기준으로 삼아보셔도 좋을거 같아요 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이 하는 역할이 많기 때문에 Modal에 대한 테스트를 진행하기 위해서는 굉장히 많은 코드가 작성되어야 할텐데요. 말씀해주신 내용처럼 vote 영역을 따로 분리한다면 테스트를 작성할 때 vote 영역에서는 vote에 대한 동작만 테스트를 진행하면 되겠죠. 이런 식으로 컴포넌트 내에서도 여러 컴포넌트로 나뉠 수 있고 그에 따른 역할과 책임이 있을텐데 이 부분을 고려해서 분리해보시면 좋을거 같아요 ㅎㅎ 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. 오😯 테스트를 고려해서 나누어 본다. 그동안에는 관심사를 기반으로 분리하다고 했을 때 확 와닿지 않는 느낌이었는데 좋네요! 감사합니다. |
||
#isOpen = false; | ||
|
||
|
@@ -29,14 +36,28 @@ export default class Modal { | |
this.#removeExistingModal(); | ||
|
||
const movieDetail = await this.#fetchMovieDetail(); | ||
const modalContent = this.#prepareModalContent(movieDetail); | ||
|
||
const modalElement = this.#createModalElement(movieDetail); | ||
const modalElement = this.#createModalElement(modalContent); | ||
document.body.appendChild(modalElement); | ||
this.#modalElement = modalElement; | ||
|
||
this.#setupModalEventListener(this.#modalElement); | ||
} | ||
|
||
#prepareModalContent(movieDetail: any) { | ||
const { title, poster_path, genres, vote_average, overview } = movieDetail; | ||
|
||
return { | ||
title, | ||
posterPath: poster_path ? `https://image.tmdb.org/t/p/w500${poster_path}` : NoImage, | ||
genres: genres.map((genre: any) => genre.name).join(', '), | ||
|
||
voteAverage: vote_average.toFixed(2), | ||
overview: overview || '해당 영화의 줄거리 정보가 없습니다.', | ||
}; | ||
} | ||
|
||
get Element() { | ||
return this.#modalElement; | ||
} | ||
|
@@ -56,19 +77,17 @@ export default class Modal { | |
} | ||
|
||
// eslint-disable-next-line max-lines-per-function | ||
#createModalElement(movieDetail: any) { | ||
#createModalElement(content: ModalContent) { | ||
const { title, posterPath, genres, voteAverage, overview } = content; | ||
|
||
const modalElement = document.createElement('div'); | ||
modalElement.classList.add('modal', 'modal--open'); | ||
|
||
const posterPath = movieDetail.poster_path ? `https://image.tmdb.org/t/p/w500${movieDetail.poster_path}` : NoImage; | ||
const genres = movieDetail.genres.map((genre: any) => genre.name).join(', '); | ||
const overview = movieDetail.overview ? movieDetail.overview : '해당 영화의 줄거리 정보가 없습니다.'; | ||
|
||
const modalHTML = /* html */ ` | ||
<div class="modal-backdrop"></div> | ||
<div class="modal-container"> | ||
<div class="modal-header"> | ||
<h3 class="detail-title text-detail-title">${movieDetail.title}</h3> | ||
<h3 class="detail-title text-detail-title">${title}</h3> | ||
<button class="modal-close-button"></button> | ||
</div> | ||
<div class="modal-body"> | ||
|
@@ -79,19 +98,15 @@ export default class Modal { | |
<p class="detail-genres text-detail-contents">${genres}</p> | ||
<p class="detail-vote_average text-detail-contents"> | ||
<img src=${StarFilled} alt="별점" class="star-start" /> | ||
${movieDetail.vote_average.toFixed(2)} | ||
${voteAverage} | ||
</p> | ||
</div> | ||
<p class="detail-overview text-detail-contents">${overview}</p> | ||
</div> | ||
<div class="my-vote"> | ||
<p class="my-vote-title text-detail-vote">내 별점</p> | ||
<div class="my-vote-body"> | ||
<button><img src=${StarEmpty} /></button> | ||
<button><img src=${StarEmpty} /></button> | ||
<button><img src=${StarEmpty} /></button> | ||
<button><img src=${StarEmpty} /></button> | ||
<button><img src=${StarEmpty} /></button> | ||
${Array(5).fill(`<button><img src=${StarEmpty} /></button>`).join('')} | ||
</div> | ||
<p class="my-vote-number text-detail-vote-contents">0</p> | ||
<p class="my-vote-description text-detail-vote-contents">남겨주세요</p> | ||
|
@@ -118,48 +133,35 @@ export default class Modal { | |
}); | ||
} | ||
|
||
// eslint-disable-next-line max-lines-per-function | ||
async openModal() { | ||
if (this.#isOpen) return; | ||
|
||
document.body.style.overflow = 'hidden'; | ||
await this.generateModal(); | ||
|
||
this.#VoteHandler(); | ||
|
||
document.addEventListener('keydown', this.#handleKeyDown); | ||
|
||
this.#isOpen = true; | ||
} | ||
|
||
// eslint-disable-next-line max-lines-per-function | ||
#VoteHandler() { | ||
const savedVotes = localStorage.getItem('myVoteResult'); | ||
if (savedVotes) { | ||
const savedVotesJSON = JSON.parse(savedVotes); | ||
this.#myVoteResult = savedVotesJSON; | ||
} | ||
const voteForMovie = savedVotes ? JSON.parse(savedVotes)[this.#movieId] : null; | ||
|
||
const voteForMovie = this.#myVoteResult[this.#movieId]; | ||
if (voteForMovie) { | ||
this.#updateVoteStar(voteForMovie); | ||
this.#updateVoteText(voteForMovie); | ||
} | ||
} | ||
|
||
// eslint-disable-next-line max-lines-per-function | ||
#updateVoteStar(voteForMovie: number) { | ||
const starButtons = this.#modalElement?.querySelectorAll('.my-vote-body button img'); | ||
|
||
if (starButtons) { | ||
starButtons.forEach((starButton, index) => { | ||
if (index < voteForMovie / 2) { | ||
starButton.setAttribute('src', StarFilled); | ||
} else { | ||
starButton.setAttribute('src', StarEmpty); | ||
} | ||
}); | ||
} | ||
if (!starButtons) return; | ||
|
||
starButtons.forEach((starButton, index) => { | ||
starButton.setAttribute('src', index < voteForMovie / 2 ? StarFilled : StarEmpty); | ||
}); | ||
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. 코드가 훨씬 깔끔해졌네요 👍 👍 👍 |
||
} | ||
|
||
#updateVoteText(voteForMovie: number) { | ||
|
@@ -204,7 +206,6 @@ export default class Modal { | |
} | ||
} | ||
|
||
// eslint-disable-next-line max-lines-per-function | ||
handleStarClick(starIndex: number) { | ||
const myVoteNumber = this.#modalElement?.querySelector('.my-vote-number'); | ||
const myVoteDescription = this.#modalElement?.querySelector('.my-vote-description'); | ||
|
@@ -214,17 +215,23 @@ export default class Modal { | |
const starButtons = this.#modalElement?.querySelectorAll('.my-vote-body button img'); | ||
if (!starButtons) return; | ||
|
||
this.updateStarButtons(starButtons, starIndex); | ||
this.updateVoteInfo(myVoteNumber, myVoteDescription, starIndex); | ||
} | ||
|
||
updateStarButtons(starButtons: NodeListOf<Element>, starIndex: number) { | ||
starButtons.forEach((starButton, index) => { | ||
if (index <= starIndex) { | ||
starButton.setAttribute('src', StarFilled); | ||
} else { | ||
starButton.setAttribute('src', StarEmpty); | ||
} | ||
starButton.setAttribute('src', index <= starIndex ? StarFilled : StarEmpty); | ||
}); | ||
} | ||
|
||
updateVoteInfo(myVoteNumber: Element, myVoteDescription: Element, starIndex: number) { | ||
const newMyVoteNumber = myVoteNumber as HTMLElement; | ||
const newMyVoteDescription = myVoteDescription as HTMLElement; | ||
|
||
const myVoteKey = (starIndex + 1) * 2; | ||
myVoteNumber.textContent = myVoteKey.toString(); | ||
myVoteDescription.textContent = VOTE[myVoteKey]; | ||
newMyVoteNumber.textContent = myVoteKey.toString(); | ||
newMyVoteDescription.textContent = VOTE[myVoteKey]; | ||
|
||
this.#myVoteResult[this.#movieId] = myVoteKey; | ||
localStorage.setItem('myVoteResult', JSON.stringify(this.#myVoteResult)); | ||
|
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.
오 이제 잘 보이네요 ㅎㅎ
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.
이미지 닫는 태그가 소스에 딱 붙어있어서 그랬더라구요