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

[2단계 - 상세 정보 & UI/UX 개선하기] 센트(김영우) 미션 제출합니다. #69

Merged
merged 39 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
5aadd5e
feat: 무한 스크롤 기능 구현
kyw0716 Mar 22, 2023
1cf533a
refactor: 로딩 스켈레톤 성능 개선 (요소 추가 방식 => display 변경 방식)
kyw0716 Mar 22, 2023
36ef1e9
feat: 반응형 디자인 적용
kyw0716 Mar 22, 2023
a0c5819
feat: 선택된 영화 정보 불러오는 기능 구현
kyw0716 Mar 22, 2023
e6333f6
feat: 모달 여는 기능 구현
kyw0716 Mar 23, 2023
2045f52
feat: 모달 닫는 기능 구현
kyw0716 Mar 23, 2023
8d03a92
feat: 모달 반응형 디자인 추가
kyw0716 Mar 23, 2023
0ab90a5
feat: 별점 선택 기능 추가
kyw0716 Mar 23, 2023
dc4a7c6
style: 필요없는 import 삭제
kyw0716 Mar 23, 2023
8c453b7
style: 로컬 스토리지 제어 메서드 이름 변경
kyw0716 Mar 24, 2023
8208eed
refactor: 사용하지 않는 메서드 정리
kyw0716 Mar 24, 2023
c2c6fe4
refactor: 유저 사용성 고려하여 모달 내 별 이미지 크기 확대
kyw0716 Mar 24, 2023
5f8fc8e
fix: 별점 선택 오류 수정
kyw0716 Mar 24, 2023
c10c137
style: 모달 내부 텍스트 스타일 수정
kyw0716 Mar 24, 2023
d695e73
docs: 리드미에 배포 링크 추가
kyw0716 Mar 24, 2023
6299cf9
style: 영화 카드 hover 스타일 추가
kyw0716 Mar 24, 2023
1b04be9
refactor: 정보가 없는 경우에 대한 UI 추가
kyw0716 Mar 24, 2023
8ad3bb1
test: 스켈레톤 테스트 추가
kyw0716 Mar 24, 2023
0133ba0
chore: cypress support 폴더 삭제
kyw0716 Mar 24, 2023
f5459d4
refactor: api 에러 처리 추가
kyw0716 Mar 24, 2023
a167d75
refactor: 영화 디테일 모달 띄우는 방식 변경 popstate => click
kyw0716 Mar 26, 2023
2bcf9ed
refactor: 모달 컴포넌트 분리
kyw0716 Mar 26, 2023
c8e1962
feat: hover로 별점 선택하기 기능 추가
kyw0716 Mar 27, 2023
65638a3
feat: 영화 상세정보 모달 컴포넌트 분리
kyw0716 Mar 27, 2023
c194cc6
style: import 컨벤션 추가
kyw0716 Mar 27, 2023
bb01706
refactor: MovieDetailModal 정보 요청 위치 변경
kyw0716 Mar 27, 2023
f1ce9f8
refactor: Header 컴포넌트 이벤트 바인딩 위치 변경
kyw0716 Mar 29, 2023
44cbce4
refactor: api 요청 방식 변경
kyw0716 Mar 29, 2023
54bb3b6
refactor: 직관적이지 않은 에러 메세지 수정
kyw0716 Mar 31, 2023
fdab5de
style: 로컬 스토리지 유틸 함수 early return 추가
kyw0716 Mar 31, 2023
6d76b86
refactor: css에서 !important 제거
kyw0716 Mar 31, 2023
a1b203c
style: map함수 축약
kyw0716 Mar 31, 2023
c86d953
style: 메서드명 수정
kyw0716 Mar 31, 2023
118bff8
style: 이미지 import시 "Image" suffix 추
kyw0716 Mar 31, 2023
ef66def
style: 이미지 import시 "Image" suffix 추가
kyw0716 Mar 31, 2023
c737c26
Merge remote-tracking branch 'origin/step2' into step2
kyw0716 Mar 31, 2023
34e0c0a
refactor: 모달 open시 사용되는 modalType 수정
kyw0716 Mar 31, 2023
c8517f2
refactor: 검색 후 input창 리셋 => 리셋 시키지 않고, input창 클릭시 전체 텍스트 선택으로 수정
kyw0716 Mar 31, 2023
b5f09e8
refactor: 별점 0점 선택 기능 추가
kyw0716 Mar 31, 2023
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# javascript-movie-review

FE 5기 레벨1 영화관 미션
## [배포 링크](https://kyw0716.github.io/javascript-movie-review/)
1 change: 1 addition & 0 deletions cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ export default defineConfig({
setupNodeEvents(on, config) {
// implement node event listeners here
},
supportFile: false,
},
});
6 changes: 4 additions & 2 deletions cypress/e2e/spec.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ describe("영화 리뷰 웹 테스트", () => {
});
});

it("더보기 버튼 클릭시 다음 페이지 정보가 렌더링 된다.", () => {
cy.get(".btn").click();
it("영화 목록을 마지막 요소까지 스크롤시 다음 페이지 정보가 렌더링 된다.", () => {
cy.scrollTo("bottom");

cy.get(".skeleton-container").should("be.visible");

cy.wait("@getPopularMoviesPage2").then((interception) => {
const movieItems = interception.response?.body.results;
Expand Down
Empty file removed cypress/support/commands.ts
Empty file.
Empty file removed cypress/support/e2e.ts
Empty file.
12 changes: 11 additions & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,23 @@
<title>영화 리뷰</title>
</head>
<body>
<div class="modal-section" style="display: none">
<div class="modal-backdrop"></div>
<div class="modal">
<div class="modal-header">
<p class="modal-header--text"></p>
<img class="x-button" alt="close button" />
</div>
<div class="modal-content"></div>
</div>
</div>
<div id="app">
<header></header>
<main>
<section class="item-view">
<h2 class="sub-title">지금 인기 있는 영화</h2>
<ul class="item-list"></ul>
<button class="btn primary full-width">더 보기</button>
<div class="btn"></div>
</section>
</main>
</div>
Expand Down
27 changes: 25 additions & 2 deletions src/App.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import { Header } from "./components/Header";
import { Modal } from "./components/Modal";
import { MovieList } from "./components/MovieList";
import { $ } from "./utils/selector";

export class App {
#header;

Choose a reason for hiding this comment

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

'#header' is declared but its value is never read. 라는 오류를 보여주고 있어요!
어떤 문제가 있는지 한 번 살펴볼까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

혹시 나중에 #header를 사용할지도 모른다고 생각해서 그대로 두었는데 결국 사용되지 않아서 생긴 오류인 것 같습니다! 오류를 해결하자고 그냥 #header를 지우기보다 #header가 무언가 하는 역할이 있도록 수정해보는건 어떨까 하고 #header가 입력값을 반환해주는 메서드를 가지도록 수정해보았습니다!

수정한 커밋: f1ce9f8

Choose a reason for hiding this comment

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

오,, 아래와 바꿔도 상관없을 거라고 생각했는데, 다른 방법을 사용하셨군요~

new Header(
      $header,
      this.onSubmitSearchKeyword.bind(this),
      this.onClickLogoImage.bind(this)
  );

#movieList;
#modal;

constructor() {
const $header = $("header");
const $movieList = $(".item-list");
const $modal = $(".modal-content");

this.#header = new Header(
$header,
devhyun637 marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -17,6 +20,21 @@ export class App {
);

this.#movieList = new MovieList($movieList);

this.#modal = new Modal($modal);

this.bindEvent($movieList);
}

bindEvent($movieList: Element) {

Choose a reason for hiding this comment

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

bindEvent라는 네이밍은 어떤 이벤트인지 감이 잘 안오는거 같아요 :(

Copy link
Member Author

Choose a reason for hiding this comment

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

다시 보니 뭔가 메서드명만 보고 해당 메서드가 어떤 역할을 할 지 바로 알아차리기 힘들어 보이네요😭😭 어떻게 바꿔볼까 고민해보다 이벤트 리스너를 추가해주어 이벤트 리스닝(?)을 시작한다는 의미를 전달하기 위해 startEventListening으로 바꿔보려 했는데 이것도 마음에 들지 않네요 ㅠㅠ 혹시 티케가 생각하는 적합한 메서드명이 있으시다면 공유 부탁드립니다!!

Choose a reason for hiding this comment

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

음.. 가장 간단하게라면 저라면 여러가지 event들이 있어서, bindEvents라는 복수형을 사용할 것 같아요.

Copy link
Member Author

Choose a reason for hiding this comment

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

다소 귀찮을 수 있는 질문이었는데 답변 감사합니다!! 👍👍👍

(이 부분은 혹시 여유가 되신다면 답변 부탁드립니다!! 바쁘시다면 굳이 시간 내셔서 답변하지 않으셔도 괜찮습니다!!!)

혹시 티케가 우테코 미션을 진행할 때 이벤트 리스너를 어떻게 추가하셨었는지 궁급합니다. 저는 1레벨 동안 고민해 본 결과 해당 이벤트의 책임을 가지고 있는 컴포넌트 내에서 모든 이벤트를 한번에 추가해주는 것이 좋을 것이라 판단해 bindEvent라는 메서드를 사용했었는데 티케만의 방법이 있으셨는지 궁금합니다!

$movieList.addEventListener("click", (event: Event) => {
if (!(event.target instanceof HTMLElement)) return;

const movieCard = event.target.closest("li");
const movieId = movieCard?.dataset.movieId;

if (movieId) this.onClickMovieCard(Number(movieId));

Choose a reason for hiding this comment

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

movieCard가 없거나 movieId가 없는 경우에는 사용자는 눌러도 어떠한 결과물을 못받는 걸까요?

Copy link
Member Author

@kyw0716 kyw0716 Mar 31, 2023

Choose a reason for hiding this comment

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

넵!! 클릭한 대상이 영화 카드가 아니라면 아무 동작이 되지 않도록 하기 위해 코드를 위와 같이 작성했었습니다!

Choose a reason for hiding this comment

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

오 사용자는 clickable 한거니깐 선택을 했는데, 어떠한 반응도 없다면 (심지어 마우스도 클릭할 수 있게 cursor형태면) 좋지 않은 UX를 경험할 거 같아요 :(

Copy link
Member Author

Choose a reason for hiding this comment

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

렌더링 된 영화 카드라면 당연히 movieId를 가지고 있을거라 생각해서 movieId가 없을 경우를 따로 생각해보지 않았는데 티케의 리뷰를 보니 혹시나 movieId가 없을 경우엔 사용자 입장에서 굉장히 당황스러울 것 같다는 생각이 드네요!

만약 movieId가 없을 경우엔 alert를 통해 선택한 영화의 상세 정보가 없다는 안내 문구를 띄우는 방식을 적용해보았습니다!

수정한 커밋: c772c19

});
}

onSubmitSearchKeyword(serachKeyword: string) {
Expand All @@ -25,14 +43,19 @@ export class App {
subTitle.innerHTML = `"${serachKeyword}" 검색 결과`;

if (this.#movieList instanceof MovieList)
this.#movieList.reset("search", serachKeyword);
this.#movieList.changeShowTarget("search", serachKeyword);
}

onClickLogoImage() {
const subTitle = $(".sub-title");

subTitle.innerHTML = `지금 인기 있는 영화`;

if (this.#movieList instanceof MovieList) this.#movieList.reset("popular");
if (this.#movieList instanceof MovieList)
this.#movieList.changeShowTarget("popular");
}

onClickMovieCard(movieId: number) {
this.#modal.open("movieDetail", movieId);
}
}
27 changes: 27 additions & 0 deletions src/components/Header/index.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
header h1 {
cursor: pointer;
user-select: none;
font-size: 2rem;
font-weight: bold;
letter-spacing: -0.1rem;
color: #f33f3f;
}

header > .search-box {
background: #fff;
padding: 8px;
border-radius: 4px;
}

header .search-box > input {
border: 0;
}

header .search-box > .search-button {
width: 14px;
border: 0;
text-indent: -1000rem;
background: url("../../../templates/search_button.png") transparent no-repeat
0 1px;
background-size: contain;
}
9 changes: 9 additions & 0 deletions src/components/Header/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import "./index.css";

import logoImage from "../../../templates/logo.png";

import { $ } from "../../utils/selector";

export class Header {
Expand All @@ -12,7 +15,13 @@ export class Header {
this.#$target = $target;

this.render();
this.bindEvent(onSubmitSearchKeyword, onClickLogoImage);
}

bindEvent(

Choose a reason for hiding this comment

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

bindEvent에 있는 이벤트핸들러가 크게 두개인것 같아서 이 둘을 함수로 분리해서 선언해두는 건 어떨까요?

Choose a reason for hiding this comment

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

사용하다보니 느낀 UX인데, 이건 개인 취향이라서 한번 생각해보시라고 제안합니다.

구글에 우리가 어떤 검색어를 입력하면, 검색창에 검색어를 reset하는 경우는 거의 없는데, 여기서는 reset을 해주고 있더라구요! 어떻게 reset해주는 UX를 생각하게 되었는지 궁금합니다~

Copy link
Member Author

Choose a reason for hiding this comment

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

Header 컴포넌트를 수정하면서 bindEvent 부분도 수정되어 App 컴포넌트에서 이벤트 리스너를 추가해주도록 바뀌었습니다!! 혹시 바뀐 방식은 어떤가요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

reset해주는 방식은 제가 영화 검색을 테스트 할 때 검색어를 계속 바꿔가며 입력하다 보니 뭔가 이전 검색어가 남아있는 것이 불편하다고 느껴 바꿔보았습니다! 또, 검색어 입력을 했을 때 리스트 상단에 ~에 대한 검색 결과입니다라는 문구도 생성되어 필요 없지 않을까? 하는 생각에서 reset을 하자고 마음을 먹게 되었습니다.

그 후에 다시 생각해본 적이 없었는데 티케가 이야기 해주셔서 다시 보니 검색 결과를 보며 스크롤을 내리면 어떤 검색어를 입력했었는지 다시 확인하기 어렵지 않을까 하는 생각이 들게 되어 reset이 꼭 필요한 과정이었나? 하는 생각이 들었습니다. 그래서 검색어를 수정하기 편하도록 input을 클릭하면 내부 텍스트가 모두 선택되도록 수정해보았습니다!!

수정한 커밋: c8517f2

Choose a reason for hiding this comment

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

$searchInput.addEventListener("click", function () {
      this.select();
 });

여기서 this.select()는 어떤 역할을 하나요?
또, 왜 App 컴포넌트에서 이벤트 리스너를 추가해주도록 바뀌었는지 알 수 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

this.select()는 입력창을 클릭했을 때 입력창 내에 있는 모든 텍스트가 선택되게 하는 역할을 수행합니다. 검색시 입력했던 검색어가 사라지도록 했던 이유가 짧은 주기로 여러번 검색을 진행할 때 검색어가 남아있는 것이 불편하다고 느껴지는 것이었는데 검색어를 남겨두고도 이러한 부분을 해결할 수 있게 해보고자 this.select()를 사용해보았습니다!

App 컴포넌트에서 이벤트 리스너를 추가해주도록 한 이유는 하위 컴포넌트를 생성할 때 함수를 넘겨주지 않도록 하기 위해서였습니다. 지금은 이벤트 핸들러의 개수가 많이 없지만, 이후 개수가 많이 늘어나게 된다면 늘어날 때마다 하위 컴포넌트에 이를 전달해주어야 하고 이 때문에 유지 보수에 불편함이 생기진 않을까 하는 생각이 들게 되었습니다. 이를 해결하기 위해 App에서 이벤트 핸들러를 선언하는 경우에는 App에서 해당 이벤트에 대한 리스너를 추가하도록 하는 것이 좋다고 판단해 방식을 바꿔봤습니다!!!

onSubmitSearchKeyword: (searchKeyword: string) => void,
onClickLogoImage: () => void
) {
$(".search-box").addEventListener("submit", (event: Event) => {
event.preventDefault();

Expand Down
34 changes: 34 additions & 0 deletions src/components/Modal/MovieDetailModal/Description.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import type { MovieDetail } from "../../../types";

import filledStarImg from "../../../../templates/star_filled.png";

import { getStarSelectContainerTemplate } from "./StarSelect";

export function getDescriptionTemplate(movie: MovieDetail, starRate: number) {
return /*html*/ `
<div class="modal-detail-container">
<div class="modal-movie-detail">
<p class="modal-movie-genre modal-detail--text">
${
movie.genre.length === 0
? `장르 정보 없음`
: movie.genre.join(" ")
}
<span>
<img
src="${filledStarImg}"
alt="별점 ${movie.vote_average}"
/>
${movie.vote_average.toFixed(1)}
</span>
</p>
<p class="modal-movie-description modal-detail--text">
${movie.overview ? movie.overview : "상세 정보 없음"}
</p>
</div>
<div class="modal-star-rate modal-detail--text">
${getStarSelectContainerTemplate(movie.id, starRate)}
</div>
</div>
`;
}
31 changes: 31 additions & 0 deletions src/components/Modal/MovieDetailModal/Image.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
export function getImageContainerTemplate(imagePath: string, title: string) {
return /*html*/ `
<div class="modal-image-container">
${
imagePath
? /*html */
`<img
class="modal-image skeleton"
src="https://image.tmdb.org/t/p/w220_and_h330_face/${imagePath}"
alt="${title} 포스터"
/>`
: /*html */
`<div
class="modal-image center"
style="
background-color:white;
color:black;
display:flex;
justify-content:center;
align-items:center;
font-weight:600;
font-size:24px;
border-radius: 16px;
"
>
<span>No Image</span>
</div>`
}
</div>
`;
}
53 changes: 53 additions & 0 deletions src/components/Modal/MovieDetailModal/StarSelect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import filledStarImg from "../../../../templates/star_filled.png";
import emptyStarImg from "../../../../templates/star_empty.png";

import { $ } from "../../../utils/selector";

const STAR_RATE_STRING = [
"나의 점수는?",
"최악이예요",
"별로예요",
"보통이예요",
"재미있어요",
"명작이예요",
];

export function renderStars(movieId: number, starRate: number) {
const starRateContainer = $(".modal-star-rate");

if (starRateContainer instanceof HTMLElement)
starRateContainer.innerHTML = getStarSelectContainerTemplate(
movieId,
starRate
);
}

export function getStarSelectContainerTemplate(
movieId: number,
starRate: number
) {
const imgArray = getStarTemplate(movieId, starRate);

return /*html*/ `
<span>내 별점</span>
<span class="star-select-container">
${imgArray.join("")}
</span>
<span>${starRate * 2}점</span>
<span class="star-rate-desc">${STAR_RATE_STRING[starRate]}</span>
`;
}

export function getStarTemplate(movieId: number, starRate: number) {
return Array.from(
{ length: 5 },
(_, i) =>
`<img
src="${starRate > i ? filledStarImg : emptyStarImg}"
alt="별점"
class="star-rate-select-img"
data-movie-id="${movieId}"
data-star-rate="${i}"
/>`
);
}
Loading