Skip to content
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조 #75

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

dragon-it
Copy link

🎬 2차과제 : API를 활용한 영화검색 사이트 만들기
작성자 : KDT5 이용수 - 4조
제출 날짜 : 5월 7일

결과물 : https://prismatic-chimera-d2347d.netlify.app/
HTML, JS, 활용

필수 요구사항

  • 영화 제목으로 검색이 가능해야 합니다!
  • 검색된 결과의 영화 목록이 출력돼야 합니다!
  • 단일 영화의 상세정보(제목, 개봉연도, 평점, 장르, 감독, 배우, 줄거리, 포스터 등)를 볼 수 있어야 합니다!
  • 실제 서비스로 배포하고 접근 가능한 링크를 추가해야 합니다.

선택 요구사항

  • 한 번의 검색으로 영화 목록이 20개 이상 검색되도록 만들어보세요.
  • 영화 개봉연도로 검색할 수 있도록 만들어보세요.
  • 영화 목록을 검색하는 동안 로딩 애니메이션이 보이도록 만들어보세요.
  • 무한 스크롤 기능을 추가해서 추가 영화 목록을 볼 수 있도록 만들어보세요.
  • 영화 포스터가 없을 경우 대체 이미지를 출력하도록 만들어보세요.
  • 영화 상세정보가 출력되기 전에 로딩 애니메이션이 보이도록 만들어보세요.
  • 영화 상세정보 포스터를 고해상도로 출력해보세요. (실시간 이미지 리사이징)
  • 차별화가 가능하도록 프로젝트를 최대한 예쁘게 만들어보세요.
  • 영화와 관련된 기타 기능도 고려해보세요.

문제점

영화의 목록을 20개 이상 불러오는데 최대치가 넘어가면 검색했던 목록들이 사라지는 문제

Copy link
Member

@iamidlek iamidlek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다.
전체적으로 잘 구현해 주셨습니다.
UI도 깔끔하게 잘 구현해 주신 것 같습니다.
영상이 재생되는 부분과 로딩중 상태 표시가 잘 된 것 같습니다.

commit 나누기가 고려 되면 더욱 좋을 것 같습니다.
core 폴더 내부 dragon.js 는 파일명이 어떤 것을 의미하는지 애매한 것 같습니다.
root 경로에 png, jpg가 있습니다. src > assets 과 같이 디렉토리 내에서 관리 하는 것이 좋을 것 같습니다.
env 설정을 시도하신 것 같은데 apikey가 노출되어 있는 것 같습니다.

@@ -0,0 +1,7 @@
export default function handler(request, response) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test가 끝난 파일은 올리지 않으셔도 좋을 것 같습니다.

}

// 닫기 버튼 정의.
const closeButton = document.querySelector('.close');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

비디오 관련 내용은 Headline과 분리되면 좋을 것 같습니다.


// 포스터 미리보기 대체 이미지
if (movie.Poster === "N/A") {
this.el.style.backgroundImage = `url('https://www.freeiconspng.com/uploads/no-image-icon-8.png')`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class로 css 쪽에서 작업하면 좋을 것 같습니다.

movieStore.subscribe('movies', () => {
this.render()
})
movieStore.subscribe('loading', () => {
Copy link
Member

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 시작과 종료에 따라 재렌더링이 이루어지면 될 것 같습니다.)

Comment on lines +24 to +26
for (let i = morePage + 2; i <= morePage + 4; i += 2) {
await searchMovies(i);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가 load 를 할 때 page 요청이 순차적이지 않습니다.
4, 6 과 같이 + 2 하신 만큼 3, 5 페이지 요청이 누락됩니다.

export default class Home extends Component {
render() {
const headline = new Headline().el
const banner = new Banner().el
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 동일하게 삭제가 필요합니다.

this.el.classList.add('container')
this.el.append(
headline,
banner,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 동일하게 삭제가 필요합니다.



await getMovieDetails(history.state.id)
console.log(movieStore.state.movie)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log 는 확인이 끝나면 제거해 주세요

const bigPoster = movie.Poster.replace('SX300', 'SX700')


<!-- 상세 페이지 내에서의 대체 이미지 -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문법 오류 입니다.
js 에서 주석은 // , /* */ 를 이용해 주세요

class="poster">
</div>
<!-- 상세 페이지 내에서의 대체 이미지 -->

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

불필요한 스페이스 인 것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants