-
Notifications
You must be signed in to change notification settings - Fork 0
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
KDT5_양준용_2차 과제 제출 RE_4조 #68
base: main
Are you sure you want to change the base?
Conversation
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.
고생하셨습니다.
전체적으로 잘 구현해 주셨습니다.
UI도 보기 좋게 잘 구현해 주신 것 같습니다.
readme도 잘 작성해 주시고
commit 도 잘 나누어서 작성해 주셔서 좋습니다.
commit message 등 조금 더 고려가 되면 더욱 좋을 것 같습니다.
core 폴더 내부 assets.js, src의 script.js 는 파일명이 어떤 것을 의미하는지 애매한 것 같습니다.
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin /> | ||
<link href="https://fonts.googleapis.com/css2?family=Oswald:wght@600&family=Roboto:wght@400;700&display=swap" rel="stylesheet" /> | ||
|
||
<link rel="stylesheet" href="./src/css/style.css" /> |
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.
reset css 와 겹치는 내용이 많은 것 같습니다.
scss파일과 따로 관리하는 이유도 있으신지 궁금한 부분입니다.
movieStore.subscribe('movies', () => { | ||
this.render() | ||
}) | ||
movieStore.subscribe('loading', () => { |
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.
subscribe로 store의 값이 변화 될 때 재렌더링을 위해 작성하신 부분인데
구독하는 대상이 3개나 있는데 loading 하나로 하여도 충분하지 않을까요?
(data fetch 시작과 종료에 따라 재렌더링이 이루어지면 될 것 같습니다.)
super({ | ||
tagName: 'button' | ||
}) | ||
movieStore.subscribe('pageMax', () => { |
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.
구조상 pageMax 값이 동일하더라도
할당이 이루어져 재렌더링이 일어나지만
의미적으로는 page 의 변화를 바라보고 있어야 할 것 같습니다.
현재 구조에서는 아래처럼 될 것 같아요.
movieStore.subscribe('pageMax', () => { | |
movieStore.subscribe("page", () => { | |
const { page, pageMax } = movieStore.state; | |
if (page === 1) { | |
this.el.classList.remove("hide"); | |
return; | |
} |
<a href="#/search"><button class="btn btn-primary"><i class="fa-solid fa-magnifying-glass"></i></button></a> | ||
` | ||
const inputEl = this.el.querySelector('input') | ||
inputEl.addEventListener('input', () => { |
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.
event 받아서 처리하는 방법도 있을 것 같습니다.
inputEl.addEventListener('input', () => { | |
inputEl.addEventListener("input", (event) => { | |
movieStore.state.searchText = event.target.value; | |
}); |
btnEl.addEventListener('click', () => { | ||
if (movieStore.state.searchText.trim()) { | ||
searchMovies(1) | ||
searchMovies(2) |
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.
1, 2 페이지를 동시에 요청하시는 경우 이하 사항에 대한 처리가 필요할 것 같습니다.
EX) zozo 검색 시 총 결과가 10개 미만이라 2page 요청 이후 not found 처리되는 문제가 있습니다.
|
||
this.el.innerHTML = /* html */ ` | ||
<div | ||
style="background-image: url(${bigPoster});" |
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.
인라인 스타일을 사용하지 않고 구현 할 수 있는 방법을 고려해 보시면 좋을 것 같습니다.
img { | ||
display: block; | ||
width: 100%; | ||
cursor: pointer; |
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.
cursor pointer를 img 태그에 주면
Home 의 BEST SCENE 의 이미지의 경우
클릭 시 아무 동작이 없기 때문에 좋지 않을 것 같습니다.
store.state.message = Error | ||
} | ||
} catch (error) { | ||
console.log('searchMovies error:', error) |
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.
console.error 사용해 보시면 좋을 것 같습니다.
🎬 2차 과제 : API를 활용한 영화 검색 사이트 만들기
결과물
필수 요구사항
선택 요구사항
화면구성
주요기능
1. 한 번의 검색으로 20개 출력
2. 영화 포스터가 없을 경우 대체 이미지를 출력
3. 영화 상세정보 포스터를 고해상도로 출력
어려웠던 점
궁금한 점