Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions REQUIREMENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- [v] 영화 목록의 1페이지를 불러오며 더보기 버튼을 누르면 그 다음의 영화 목록을 불러 올 수 있다.
- 단, 페이지 끝에 도달한 경우에는 더보기 버튼을 화면에 출력하지 않는다.
- ⚠️ 인기순은 TMDB에서 제공하는 API의 속성 이름을 나타내는 것이므로 별도로 받은 데이터를 정렬하지 않습니다.
- [v] 영화 목록 아이템에 대한 Skeleton UI를 구현한다.
- Skeleton UI는 템플릿으로 제공되는 파일 이외로 자유롭게 구현할 수 있다.
9 changes: 9 additions & 0 deletions cypress.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { defineConfig } from "cypress";

export default defineConfig({
e2e: {
setupNodeEvents(on, config) {
// implement node event listeners here
},
},
});
45 changes: 45 additions & 0 deletions cypress/e2e/index.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
describe("메인 페이지", () => {
beforeEach(() => {
cy.intercept(
{
method: "GET",
url: /^https:\/\/api\.themoviedb\.org\/3\/movie\/popular*/,
},
(req) => {
req.reply({
fixture: "popular-movies.json",
delayMs: 500,
});
}
).as("getPopularMovies");

cy.visit("http://localhost:5173");
});

it("영화 목록 API를 호출하면 20개의 영화가 불러와진다.", () => {
cy.wait("@getPopularMovies").then((interception) => {
const popularMovies = interception.response.body.results;
expect(popularMovies).to.have.length(20);

const popularMovieItems = cy.get("ul.thumbnail-list > li");
popularMovieItems.should("have.length", 20);
});
});

it("더보기 버튼 클릭 시 스켈레톤이 보였다가 사라지고, 새 영화 카드 20개가 추가로 보인다.", () => {
cy.get("ul.thumbnail-list > li").should("have.length", 20);

cy.get("button").contains("더 보기").click();

cy.get("ul.thumbnail-list > li.card-skeleton").should(
"have.length.greaterThan",
0
);

cy.wait("@getPopularMovies");

cy.get("ul.thumbnail-list > li.card-skeleton").should("have.length", 0);

cy.get("ul.thumbnail-list > li").should("have.length", 40);
});
});
327 changes: 327 additions & 0 deletions cypress/fixtures/popular-movies.json

Large diffs are not rendered by default.

25 changes: 25 additions & 0 deletions cypress/support/commands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// ***********************************************
// This example commands.js shows you how to
// create various custom commands and overwrite
// existing commands.
//
// For more comprehensive examples of custom
// commands please read more here:
// https://on.cypress.io/custom-commands
// ***********************************************
//
//
// -- This is a parent command --
// Cypress.Commands.add('login', (email, password) => { ... })
//
//
// -- This is a child command --
// Cypress.Commands.add('drag', { prevSubject: 'element'}, (subject, options) => { ... })
//
//
// -- This is a dual command --
// Cypress.Commands.add('dismiss', { prevSubject: 'optional'}, (subject, options) => { ... })
//
//
// -- This will overwrite an existing command --
// Cypress.Commands.overwrite('visit', (originalFn, url, options) => { ... })
20 changes: 20 additions & 0 deletions cypress/support/e2e.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// ***********************************************************
// This example support/e2e.js is processed and
// loaded automatically before your test files.
//
// This is a great place to put global configuration and
// behavior that modifies Cypress.
//
// You can change the location of this file or turn off
// automatically serving support files with the
// 'supportFile' configuration option.
//
// You can read more here:
// https://on.cypress.io/configuration
// ***********************************************************

// Import commands.js using ES2015 syntax:
import "./commands";

// Alternatively you can use CommonJS syntax:
// require('./commands')
3 changes: 2 additions & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<link rel="stylesheet" href="styles/index.css" />
<title>영화 리뷰</title>
</head>
<body>
<div id="app"></div>
<script type="module" src="/src/main.js"></script>
<script type="module" src="src/main.js"></script>
</body>
</html>
62 changes: 62 additions & 0 deletions src/apis/getPopularMovies.js
Copy link
Contributor

Choose a reason for hiding this comment

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

영화 리뷰 헤더 영역의 배너 포스터 이미지는 아래의 링크로 불러와지도록 하면 더욱 멋질 것 같아요!

const 배너URL = "https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/";

Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
export async function getPopularMovies(params) {
const { language = "ko-KR", page = 1 } = params;

const url = new URL("https://api.themoviedb.org/3/movie/popular");
url.searchParams.set("language", language);
url.searchParams.set("page", page);
Comment on lines +4 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

제공되는 객체를 잘 활용하셨네요! 👍


const response = await fetch(url.toString(), {
headers: {
accept: "application/json",
Authorization: `Bearer ${import.meta.env.VITE_TMDB_API_KEY}`,
},
});
Comment on lines +4 to +13
Copy link
Author

Choose a reason for hiding this comment

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

현재는 api server url을 환경변수로 다룰 필요가 없을 것 같아서 그렇게 다루진 않았고, api 함수가 딱 하나라서 fetch와 관련해서도 별도 추상화를 하지는 않았어요!

Copy link
Contributor

Choose a reason for hiding this comment

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

좋습니다. 그런데 헤더 이미지를 변경하시게 된다면 new URL에 선언하신 하드 코딩된 날것의 URL주소는 별도의 상수로 선언하고 관리하시는 게 편하실지도 모르겠어요 ㅎ


if (!response.ok) {
const error = await response.json();
throw new Error(error.status_message || "일시적으로 에러가 발생했어요.");
Copy link
Contributor

Choose a reason for hiding this comment

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

이 구문에서 어떤 에러가 발생할 수 있을까요?
그리고 어떻게 사용자에게 오류 메시지를 보여주면 좋을까요?

}

const popularMovies = await response.json();

return {
...popularMovies,
results: popularMovies.results.map(toClientEntity),
};
}

function toClientEntity(popularMovies) {
Copy link
Author

Choose a reason for hiding this comment

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

외부로 export 되지 않고 모듈에서만 사용되는 함수명은 너무 이상하지만 않다면 합의만 되면 괜찮다고 생각해서 요정도로 지어봤어요.
파생값을 만들어주기 위한 역할을 할 수도 있지만, 현재는 snake_case를 camelCase로 변환하는 역할을 담당하고 있어요.

Copy link
Contributor

Choose a reason for hiding this comment

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

네넵~! 아무래도 미션에서 사용하는 컨벤션을 준수하시려는 의도로 보였습니다. 어떤 방향이든 좋아요!

const {
adult,
backdrop_path,
genre_ids,
id,
original_language,
original_title,
overview,
popularity,
poster_path,
release_date,
title,
video,
vote_average,
vote_count,
} = popularMovies;

return {
adult,
backdropPath: backdrop_path,
genreIds: genre_ids,
id,
originalLanguage: original_language,
originalTitle: original_title,
overview,
popularity,
posterPath: poster_path,
releaseDate: release_date,
title,
video,
voteAverage: vote_average,
voteCount: vote_count,
};
}
23 changes: 23 additions & 0 deletions src/components/Banner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export function createBanner() {
const banner = document.createElement("div");
banner.classList.add("background-container");

banner.innerHTML = /* HTML */ `
<div class="overlay" aria-hidden="true"></div>
<div class="top-rated-container">
<h1 class="logo">
<img src="./images/logo.png" alt="MovieList" />
</h1>
<div class="top-rated-movie">
<div class="rate">
<img src="./images/star_empty.png" class="star" />
<span class="rate-value">9.5</span>
</div>
<div class="title">인사이드 아웃2</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

인사이드 아웃2는 더미데이터를 의도해서 넣어두신 걸까용?

<button class="primary detail">자세히 보기</button>
</div>
</div>
`;

return banner;
}
42 changes: 42 additions & 0 deletions src/components/MovieCard.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { formatMovieRate } from "../domain/formatMovieRate.js";
import { getImageUrl } from "../domain/getImageUrl.js";

export function createMovieCard({ title, imageFileName, rate }) {
const li = document.createElement("li");
const div = document.createElement("div");
div.classList.add("item");

div.innerHTML = /* HTML */ `
<img class="thumbnail" src=${getImageUrl(imageFileName)} alt=${title} />
<div class="item-desc">
<p class="rate">
<img src="./images/star_empty.png" class="star" /><span
>${formatMovieRate(rate)}</span
>
</p>
<strong>${title}</strong>
</div>
`;

li.append(div);

return li;
}

export function createMovieCardSkeleton() {
Copy link
Author

Choose a reason for hiding this comment

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

리액트에서는 MovieCard 라는 컴포넌트가 있다고 하면, MovieCard.Skeleton = MovieCardSkeleton과 같이 표현하는 걸 좋아하는데 여기서는 스켈레톤 관련된 코드를 MovieCard라는 파일안에 같이 두는 방식으로 작업했어요.

const li = document.createElement("li");
li.classList.add("card-skeleton");

const div = document.createElement("div");
div.classList.add("item");

div.innerHTML = /* HTML */ `
<div class="thumbnail-skeleton"></div>
<div class="rate-skeleton"></div>
<div class="title-skeleton"></div>
`;

li.append(div);

return li;
}
10 changes: 10 additions & 0 deletions src/domain/formatMovieRate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* rate는 1~10 사이의 값이 들어옵니다.
* 소수점 1자리까지 보여줍니다.
* @example
* const movieRate = formatMovieRate(7.654);
* console.log(movieRate); // 7.6
*/
export function formatMovieRate(rate) {
return rate.toFixed(1);
}
Comment on lines +8 to +10
Copy link
Author

Choose a reason for hiding this comment

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

이 함수는 동작 자체는 단순하지만, 영화 서비스에서 평점은 다양한 곳에서 표현될 것으로 예상이 되어서 별도 함수로 분리해두었어요

3 changes: 3 additions & 0 deletions src/domain/getImageUrl.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function getImageUrl(posterPath) {
return `https://media.themoviedb.org/t/p/w440_and_h660_face/${posterPath}`;
}
Comment on lines +1 to +3
Copy link
Author

Choose a reason for hiding this comment

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

요건 네이밍을 조금 더 좁히면 좋겠다는 생각이 들어요.
현재는 크게 상관은 없지만, 조금만 요구사항이 추가되어도 바로 재사용이 불가능 할 것 같아요.

tmdb에서 제공하는 스펙을 봐야겠지만 width랑 height를 받을 수 있다면 받아보면 좋을 것 같네요. + face같이 이미지 타입을 결정하는 값도 같이? (face가 아직 뭔지는 모르겠어요)

120 changes: 88 additions & 32 deletions src/main.js
Original file line number Diff line number Diff line change
@@ -1,35 +1,91 @@
import image from "../templates/images/star_filled.png";

console.log("npm run dev 명령어를 통해 영화 리뷰 미션을 시작하세요");

console.log(
"%c" +
" _____ ______ ________ ___ ___ ___ _______ \n" +
"|\\ _ \\ _ \\|\\ __ \\|\\ \\ / /|\\ \\|\\ ___ \\ \n" +
"\\ \\ \\\\\\__\\ \\ \\ \\ \\|\\ \\ \\ \\ / / | \\ \\ \\ __/| \n" +
" \\ \\ \\\\|__| \\ \\ \\ \\\\\\ \\ \\ \\/ / / \\ \\ \\ \\ \\_|/__ \n" +
" \\ \\ \\ \\ \\ \\ \\ \\\\\\ \\ \\ / / \\ \\ \\ \\ \\_|\\ \\ \n" +
" \\ \\__\\ \\ \\__\\ \\_______\\ \\__/ / \\ \\__\\ \\_______\\ \n" +
" \\|__| \\|__|\\|_______|\\|__|/ \\|__|\\|_______| \n" +
" \n" +
" \n" +
" \n" +
" ________ _______ ___ ___ ___ _______ ___ __ \n" +
"|\\ __ \\|\\ ___ \\ |\\ \\ / /|\\ \\|\\ ___ \\ |\\ \\ |\\ \\ \n" +
"\\ \\ \\|\\ \\ \\ __/|\\ \\ \\ / / | \\ \\ \\ __/|\\ \\ \\ \\ \\ \\ \n" +
" \\ \\ _ _\\ \\ \\_|/_\\ \\ \\/ / / \\ \\ \\ \\ \\_|/_\\ \\ \\ __\\ \\ \\ \n" +
" \\ \\ \\\\ \\\\ \\ \\_|\\ \\ \\ / / \\ \\ \\ \\ \\_|\\ \\ \\ \\|\\__\\_\\ \\ \n" +
" \\ \\__\\\\ _\\\\ \\_______\\ \\__/ / \\ \\__\\ \\_______\\ \\____________\\\n" +
" \\|__|\\|__|\\|_______|\\|__|/ \\|__|\\|_______|\\|____________|",
"color: #d81b60; font-size: 14px; font-weight: bold;"
);

addEventListener("load", () => {
import { getPopularMovies } from "./apis/getPopularMovies.js";
import { createBanner } from "./components/Banner.js";
import {
createMovieCard,
createMovieCardSkeleton,
} from "./components/MovieCard.js";
import { getImageUrl } from "./domain/getImageUrl.js";

addEventListener("load", async () => {
const app = document.querySelector("#app");
const buttonImage = document.createElement("img");
buttonImage.src = image;

if (app) {
app.appendChild(buttonImage);
}
const wrap = document.createElement("div");
wrap.id = "wrap";
app.append(wrap);

const header = document.createElement("header");
wrap.append(header);

const banner = createBanner();
header.append(banner);

const container = document.createElement("div");
container.classList.add("container");
wrap.append(container);

const main = document.createElement("main");
container.append(main);

const section = document.createElement("section");
main.append(section);

const title = document.createElement("h2");
title.textContent = "지금 인기 있는 영화";
section.append(title);
Comment on lines +12 to +34
Copy link
Author

Choose a reason for hiding this comment

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

요렇게 기본 템플릿을 만들어주는 부분들 코드가 조금 불편한 것 같아요.
특정 단위로 묶어야 될 것 같다는 생각이 들었어요

Comment on lines +12 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

아래와 같이 선언부와 DOM 조작부를 따로 나누어서 어떤 요소를 선언했는지 명확하게 알면 더 좋지 않을까요?

Suggested change
const wrap = document.createElement("div");
wrap.id = "wrap";
app.append(wrap);
const header = document.createElement("header");
wrap.append(header);
const banner = createBanner();
header.append(banner);
const container = document.createElement("div");
container.classList.add("container");
wrap.append(container);
const main = document.createElement("main");
container.append(main);
const section = document.createElement("section");
main.append(section);
const title = document.createElement("h2");
title.textContent = "지금 인기 있는 영화";
section.append(title);
const wrap = document.createElement("div");
const header = document.createElement("header");
const container = document.createElement("div");
const main = document.createElement("main");
const section = document.createElement("section");
const title = document.createElement("h2");
wrap.id = "wrap";
app.append(wrap);
wrap.append(header);
header.append(createBanner());
container.classList.add("container");
wrap.append(container);
container.append(main);
main.append(section);
title.textContent = "지금 인기 있는 영화";
section.append(title);

이것마저도 너무 지저분하다는 느낌이 드신다면, 큰 틀을 구성해 주는 함수를 만들어봐도 좋을 것 같아요 :)
예를 들어, createWrap 함수를 만들고 그 안에 header, container, footer를 담는 식으로 말이에요!

function createAppLayout() {
    const elements = {
      wrap: document.createElement("div"),
      header: document.createElement("header"),
      container: document.createElement("div"),
      main: document.createElement("main"),
      section: document.createElement("section")
    };

    // 구조 설정
    elements.wrap.id = "wrap";
    elements.container.classList.add("container");

    // DOM 트리 구성
    app.append(elements.wrap);
    elements.wrap.append(elements.header, elements.container);
    elements.container.append(elements.main);
    elements.main.append(elements.section);

    return elements;
  }


let page = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

이벤트 리스너에 page를 let으로 선언하기보다, 글로벌 스코프에 하나의 객체를 선언하고 앱 내에서 구동될 객체 정보를 정의해 보는 것도 괜찮지 않을까요?

const appState = {
    page: 1,
    // 향후 추가될 상태들
    // filter: 'all',
    // sortBy: 'popularity' 
};

const response = await getPopularMovies({ page });

const ul = document.createElement("ul");
ul.classList.add("thumbnail-list");
section.append(ul);

const cards = response.results.map(({ title, posterPath, voteAverage }) =>
createMovieCard({ title, imageFileName: posterPath, rate: voteAverage })
);

const firstMoviePosterPath = response.results[0].posterPath;
header.style.backgroundImage = `url(${getImageUrl(firstMoviePosterPath)})`;
ul.append(...cards);

const loadMoreButton = document.createElement("button");
loadMoreButton.textContent = "더 보기";
loadMoreButton.classList.add("load-more-button");
loadMoreButton.addEventListener("click", async () => {
const cardSkeleton = Array.from({ length: ITEM_PER_PAGE }, () =>
createMovieCardSkeleton()
);
ul.append(...cardSkeleton);

page = page + 1;

try {
const response = await getPopularMovies({
language: "ko-KR",
page,
});
if (response.total_pages === page) {
loadMoreButton.classList.add("hide");
}

const cards = response.results.map(({ title, posterPath, voteAverage }) =>
createMovieCard({ title, imageFileName: posterPath, rate: voteAverage })
);

removeSkeleton();
ul.append(...cards);
} catch (error) {
removeSkeleton();
// TODO: Toast나 Alert로 표현하기
window.alert(error.message);
}
Comment on lines +62 to +81
Copy link
Contributor

Choose a reason for hiding this comment

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

버튼 이벤트리스너는 버튼을 클릭했을 떄 어떤 행동을 할 것인가를 기대하는 것인데, 지금 너무 많은 작업을 하다 보니까 한눈에 보기가 어렵습니다ㅠ.
모든 작업 내역을 담기보다는 별도의 함수로 빼는 작업이 필요하지 않을까요?
예를 들어, 영화 목록을 불러오는 작업의 내역은 별도의 함수로 분리할 수도 있지 않을까요?

async function loadMoreMovies() {
    try {
      showLoadingSkeleton();
      const newMovies = await fetchNextPage();
      hideLoadMoreButtonIfLastPage(newMovies);
      displayNewMovies(newMovies);
    } catch (error) {
      handleLoadError(error);
    } finally {
      hideLoadingSkeleton();
    }
  }

현재 리스너가 담당하는 작업을 제가 정리해 봤어요. 어떻게 하면 좀 더 간소화할 수 있을까요?

  1. 스켈레톤 UI 작업
  2. 페이지 상태 변경
  3. API 호출
  4. 마지막 페이지 처리
  5. 영화 카드 생성
  6. DOM 조작
  7. 에러 처리

});

section.append(loadMoreButton);
});

function removeSkeleton() {
ul.querySelectorAll(".card-skeleton").forEach((el) => el.remove());
Copy link
Contributor

Choose a reason for hiding this comment

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

사용성에서 피드백 드렸지만, 더보기 버튼을 누르고 서버에 다음 페이지는 요청하고 있는데, 스켈레톤이 다음 페이지 콘텐츠로 교체되지 않는 부분도 꼭 점검 부탁드려요!

}

const ITEM_PER_PAGE = 20;
Loading