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조 #65

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

Conversation

7581058
Copy link

@7581058 7581058 commented May 10, 2023

🎬 2차과제 : API를 활용한 영화검색 사이트 만들기

작성자 : KDT5 김다슬 - 4조

결과물

HTML, SCSS, JS, Webpack 활용


필수 요구사항

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

선택 요구사항

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


화면 & 구현한 부분

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

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

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

  4. NotFound 페이지
    image
    등록된 페이지 외의 주소로 접근하면 보여주는 페이지 입니다.


아쉬운 부분

  1. 처음에는 메인 페이지 상단에 영상이 들어간 슬라이드를 구현했었으나, 비디오와 슬라이드에 관련된 지식 부족으로 기존에 구현하려고 했던 각 슬라이드의 영상이 재생 완료 되면 다음 슬라이드로 넘어가는 기능을 구현하지 못했고, 슬라이드 또한 영상은 출력되나 자동 슬라이드가 잘되지 않는 일이 발생했고, 슬라이드가 생성될 때마다 정보를 불러왔었기 때문에 성능 문제가 생겨 슬라이드를 삭제하고 단일 영상으로 대체하게 되었습니다.

  2. 기능 구현에만 급해 정보를 받아온 후 슬라이드들이 생성되고 출력될 때의 속도, 성능 등에 대해 생각하지 못했던 점이 아쉬웠고, 최대한 수정했으나 api 요청, 비동기에 대한 이해가 아직 많이 부족하다는 것을 알게 되었습니다.

  3. 영화를 검색한 후 스크롤을 내리면 무한 스크롤로 검색한 모든 정보를 볼 수 있는데 스크롤을 로딩을 기다리지 않고 급하게 내리거나 하면 검색 목록이 온전하지 않은 부분이 생기는 경우에 대해 해결을 하지 못했습니다.


Sorry, something went wrong.

@7581058 7581058 self-assigned this May 10, 2023
Copy link

@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.

고생하셨습니다.
전체적으로 잘 구현해 주셨습니다.
readme도 잘 작성해 주시고
추가적인 추천 리스트를 보여주는 기능, 영상 및 슬라이드 등
다양한 시도를 해보신 부분이 좋았습니다.

commit을 잘 나누어서 작성해 주시면 좋을 것 같습니다.
MovieDetail 파일의 경우 조금 더 간결한 로직으로
(로딩, 데이터의 여부) 작성 할 수 있으면 좋을 것 같습니다.

movieStore.subscribe('modal', () => {
this.render()
})
movieStore.subscribe('contents', () => {

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

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=${

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=${

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 = {}

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)

Choose a reason for hiding this comment

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

for문 안에서 반복적으로 append 를 하는 것은 좋지 않을 것 같습니다.
주입해야 할 html을 string으로 만들고 한번 주입하는 것이 좋을 것 같습니다.

Suggested change
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)
Copy link

@iamidlek iamidlek May 10, 2023

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(

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)

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')

Choose a reason for hiding this comment

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

로더의 기본으로 hide를 주는 이유가 있을까요?

iamidlek

This comment was marked as duplicate.

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.

None yet

2 participants