-
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조 #65
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.
고생하셨습니다.
전체적으로 잘 구현해 주셨습니다.
readme도 잘 작성해 주시고
추가적인 추천 리스트를 보여주는 기능, 영상 및 슬라이드 등
다양한 시도를 해보신 부분이 좋았습니다.
commit을 잘 나누어서 작성해 주시면 좋을 것 같습니다.
MovieDetail 파일의 경우 조금 더 간결한 로직으로
(로딩, 데이터의 여부) 작성 할 수 있으면 좋을 것 같습니다.
movieStore.subscribe('modal', () => { | ||
this.render() | ||
}) | ||
movieStore.subscribe('contents', () => { |
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.
store의 contents는 boolean으로 명칭이 명확하지 않은 것 같습니다.
데이터 load 와 관련있는 단어로 설정하면 좋을 것 같습니다.
? this.el.classList.remove('hide') | ||
: this.el.classList.add('hide') | ||
|
||
const { movie } = movieStore.state |
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.
movie 뿐만 아니라 store에서 가져와야할 modal, contents를
위에서 한번에 가져오는 것도 좋을 것 같습니다.
posterWrap.innerHTML = /*HTML*/ ` | ||
<div | ||
class="poster" | ||
style=${ |
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.
인라인 스타일 없이 이미지, 대체 이미지 처리가 가능할 것 같습니다.
<div class="poster-wrap"> | ||
<div | ||
class="poster" | ||
style=${ |
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.
동일하게 인라인 스타일 없이 구현 가능할 것 같습니다.
` | ||
|
||
this.el.addEventListener('click', async () => { | ||
movieStore.state.movie = {} |
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.
movieStore.state.~ 부분들을
getMovieDetails 안에서 처리 가능할 것 같습니다.
const options = document.createElement('option') | ||
options.value = i | ||
options.innerText = i | ||
select.append(options) |
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.
for문 안에서 반복적으로 append 를 하는 것은 좋지 않을 것 같습니다.
주입해야 할 html을 string으로 만들고 한번 주입하는 것이 좋을 것 같습니다.
select.append(options) | |
const thisYear = new Date().getFullYear() | |
const endYear = 1985 | |
const select = this.el.querySelector('#year') | |
select.insertAdjacentHTML( | |
'afterbegin', | |
Array.from({ length: thisYear - endYear + 1 }, (_, i) => { | |
if (i) { | |
return /*HTML*/ `<option>${thisYear - i}</option>` | |
} else { | |
return /*HTML*/ `<option>All Year</option><option>${ | |
thisYear - i | |
}</option>` | |
} | |
}).join('') | |
) |
movieStore.state.searchYear = select.value | ||
} | ||
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 처리되는 문제가 있습니다.
const swipercontainer = this.el.querySelector('swiper-container') | ||
|
||
const fetchData = async () => { | ||
const responses = await Promise.all( |
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.
Promise.all 사용 좋습니다.
slide.innerHTML = /*HTML*/ ` | ||
<img class="poster" src="${movie.Poster}" /> | ||
` | ||
swipercontainer.append(slide) |
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.
동일하게 forEach(혹은 map)로는 생성할 요소를 만들고
append는 한번에 하는 것이 좋을 것 같습니다.
export default class TheLoader extends Component { | ||
render() { | ||
this.el.setAttribute('class', 'the-loader') | ||
this.el.classList.add('hide') |
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.
로더의 기본으로 hide를 주는 이유가 있을까요?
🎬 2차과제 : API를 활용한 영화검색 사이트 만들기
결과물
필수 요구사항
선택 요구사항
화면 & 구현한 부분
시작페이지

처음 접속하면 보여지는 페이지이며,
영화와 관련된 기타 기능으로 목록 중 추천 받길 원하는 장르를 선택하면
세션스토리지에 저장해 다음 화면에서 선택한 장르의 영화목록들이 보여지게 됩니다.
메인페이지


시작 페이지에서 버튼을 누르면 나오는 페이지로
상단에는 새로 개봉한 영화(임의 선정)에 대한 예고편과 정보를 보여주고
중앙에는 선택한 장르에 대한 추천 목록을 보여주고 하단에는 검색할 수 있는 검색창이 나타납니다.
영화를 검색할 수 있고, 포스터가 없는 영화는 대체 이미지가 출력됩니다.
무한 스크롤을 통해 검색한 영화에 대한 모든 목록을 조회할 수 있습니다.
상세 정보 출력

상단에 출력된 영상의 more 버튼이나, 추천된 영화 목록을 클릭하거나 검색된 영화를 클릭하면
나타나는 영화에 대한 상세 정보를 모달창으로 보여줍니다.
NotFound 페이지

등록된 페이지 외의 주소로 접근하면 보여주는 페이지 입니다.
아쉬운 부분
처음에는 메인 페이지 상단에 영상이 들어간 슬라이드를 구현했었으나, 비디오와 슬라이드에 관련된 지식 부족으로 기존에 구현하려고 했던 각 슬라이드의 영상이 재생 완료 되면 다음 슬라이드로 넘어가는 기능을 구현하지 못했고, 슬라이드 또한 영상은 출력되나 자동 슬라이드가 잘되지 않는 일이 발생했고, 슬라이드가 생성될 때마다 정보를 불러왔었기 때문에 성능 문제가 생겨 슬라이드를 삭제하고 단일 영상으로 대체하게 되었습니다.
기능 구현에만 급해 정보를 받아온 후 슬라이드들이 생성되고 출력될 때의 속도, 성능 등에 대해 생각하지 못했던 점이 아쉬웠고, 최대한 수정했으나 api 요청, 비동기에 대한 이해가 아직 많이 부족하다는 것을 알게 되었습니다.
영화를 검색한 후 스크롤을 내리면 무한 스크롤로 검색한 모든 정보를 볼 수 있는데 스크롤을 로딩을 기다리지 않고 급하게 내리거나 하면 검색 목록이 온전하지 않은 부분이 생기는 경우에 대해 해결을 하지 못했습니다.