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 #58

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

Conversation

cuconveniencestore
Copy link

🧞‍♂️ 🎬 영화 검색

주어진 API를 활용해 '완성 예시' 처럼 자유롭게 영화 검색 기능을 구현해보세요!
과제 수행 및 리뷰 기간은 별도 공지를 참고하세요!

🦹🏻 과제 링크

-----------🩵Moviecu🩵-----------

react로 진행하였고 필수 조건 구현 완료 하였고 추가 요소를 진행하였습니다만...

아직 수정중입니다...(노력ing)

🙉 과제 진행 목표

  • 실강을 통해 학습한 react 활용하여 과제 진행하기
  • scss 사용하여 프로젝트 진행해보기
  • 프로젝트 파일 내 디렉토리 정리해보기
  • 진행과정 이해하고 학습하면서 진행하기

❗ 필수

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

❔ 선택

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

추후 보완해야하는 부분

  • view more btn 클릭시 2번쩨 페이지까지만 출력되는 부분 보완해서 모두 볼수 있게 진행
  • 영화 상세정보 페이지 파일에서 fetch함수 활용한 부분 axios로 전환 > api파일로 이동
  • 로딩 페이지 구현

Copy link

@chanuuuuu chanuuuuu left a comment

Choose a reason for hiding this comment

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

React 구현 뿐만 아니라 디자인까지 직접하시느라 고생하셨습니다.👍🏻

React를 사용하는 방법을 배우셨다면, 이제는 연관성이 있는 로직을 컴포넌트에서 분리, 하나의 파일로 모으는 작업(api.jsx), 컴포넌트를 잘게 쪼개어 책임을 분리하는 작업을 진행해보시면 좋을 것 같습니다. 이러한 작업에 있어서 생각해볼만한 부분 코멘트 드렸습니다.

확인해보시고 리플 달아주세요!

<link
rel="preconnect"
href="https://fonts.googleapis.com" />
<link

Choose a reason for hiding this comment

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

preconnect를 사용하여 font 파일을 들고오고 있네요:) 해당 속성이 어떠한 의미로 사용되고 있는지 또 어떤 효과가 있어서 추가한 것인지 설명해주실 수 있을까요?

Copy link
Author

@cuconveniencestore cuconveniencestore May 18, 2023

Choose a reason for hiding this comment

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

사용할 당시에는 그대로 가져와 사용해 인지하지 못했는데 멘토님의 리뷰를 보고 찾아보게되어 리플남깁니다!

rel = preconnect 를 사용하게 되면 페이지가 로드 될 때 외부리소스의 도메인에 미리 연결을 하게 되어, 리소스 로드 시간을 줄여주는 속성 값입니다.

rel = preconnect 속성값을 사용하지 않았을때 웹폰트가 로딩 되기 전까지 텍스트가 나오지 않거나 일부만 나오는 현상이 나타나 폰트가 깜빡이는 듯한 느낌을 줄 수 있는 문제가 있어 이를 방지하고 웹폰트를 유연하게 사용하기위해 쓰입니다! 🥸

import axios from 'axios'

export async function fetchMovies(inputText, selectedType, page) {
const { data } = await axios({

Choose a reason for hiding this comment

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

axios 도입 및 HTTP 요청 로직을 상태관리 로직과 분리한 것은 매우 훌륭합니다. 더 나아가 axios의 get() 메서드 도입을 추가적으로 고려해보면 좋을 것 같아요. 현재 axios()를 통해서 사용하고 있으나, axios에서는 HTTP 요청의 방식인 method(GET)에 대응되는 get()를 제공하고 있기 때문입니다.

이 메서드를 사용해야하는 이유는 해당 메서드를 사용하는 것을 만으로도 해당 요청이 GET 방식이라는 것을 알 수 있으며, 아래의 그림과 같이 '?' 뒤에 들어가는 여러가지 값들을 객체의 형태로 전달할 수 있기 때문입니다. 지금과 같이 url의 문자열에 추가할 수 있지만 나중에 `? 뒤에 추가해야하는 값이 20-30개면 문자열이 너무나도 길어질 수 있겠죠? axios docs 링크를 남겨두었으니 한번 읽어보시는 것을 추천드립니다.

axios.get('https://omdbapi.com', {
    params: {
      apikey: '7035c60c',
      s: inputText,
      type: selectedType, 
      page: page
    }
  })

axios 공식문서

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다!! 문서 참고하여 리팩토링시 수정하겠습니다!

<NavLink
to="/"
className="logo">
<span className="logo--color">Mov</span>iecu <span>!</span>

Choose a reason for hiding this comment

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

BEM 방법론을 잘 적용하고 계시네요👍🏻 여기 logo--color의 클래스명이 해당 태그에 대해서 색을 부여하기 위함으로 보이는데, 혹시 실제 색이 반영되는 내부 span 태그가 아니라 현재 태그에 logo--color의 클래스명을 사용한 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

앗...! 이 부분을 제가 footer 로고 부분을 진행하면서 알게되어 수정했었는데제가 header 로고 부분에는 변경을 못해서.. 다소.... 번거롭게 색상이 변하는 부분이 아닌 그 앞뒤로 색상을 지정해 주었던것 같습니다..!
리팩토링 진행시 변경하도록 하겠습니다! 🥹🫠

@@ -0,0 +1,17 @@
import TheHeader from '~/components/TheHeader'

Choose a reason for hiding this comment

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

파일 경로에 ~를 사용하고 계신데, 혹시 ~의 의미를 알 수 있을까요? 만약 특정 경로를 의미한다면 어떻게 설정하는지 궁금합니다.

Copy link
Author

Choose a reason for hiding this comment

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

프로젝트를 vite react로 진행 하였기 때문에 vite에서 지원하는 (vite.config 폴더 안)alias를 사용해서
path alias: [{ find: '~', replacement: ${__dirname}/src }]}로 절대경로를 설정해 주었습니다!

e.target.src = xboxImg;
};

async function fetchDetailsMovie() {

Choose a reason for hiding this comment

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

해당 HTTP 요청 로직 또한 API 파일로 옮기는 것이 좋을 것 같아요. 따로 컴포넌트 내에 두신 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

앗 넵 따로 컴포넌트 내에 둔 이유는 없고, 마지막 컴포넌트 정리를 진행했어야 했는데 하지 못했던 것 같습니다!
리팩토링시 진행하도록 하겠습니다! 🙌🏼

}

useEffect(() => {
(async () => {
Copy link

@chanuuuuu chanuuuuu May 14, 2023

Choose a reason for hiding this comment

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

(async () => {
   setMovie(await fetchDetailsMovie());
})();

위의 로직을 함수로 새로 만들지 않고 useEffect내에 선언한 이유가 궁금합니다.

++ ( () => {} )()의 문법으로 익명함수를 만들어 즉시 실행하는 방식을 사용한 이유는 아마 useEffect() 내에서 직접적으로 async가 불가능하기 때문이라고 생각하는데요. 해당 비동기 로직을 하나의 메서드로 구현하여 useEffect() 내에서 호출하면 되지 않을까요?🤔

Choose a reason for hiding this comment

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

저는 개인적으로 useEffect() 상태값의 변화에 따라 로직을 호출하는 영역으로, 그 내부에는 최대한 로직의 선언을 줄여야 한다고 생각하는데요. 한 번 해당 useEffect()의 로직들을 분리하여 메서드로 만들고 내부를 간소화하보는 것을 추천드립니다:)

Copy link
Author

Choose a reason for hiding this comment

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

아직 react에 대한 공부가 부족했던것 같습니다🥹
멘토님이 말씀해주신 방법에대해서 공부해서 적용해 보도록 하겠습니다! 감사합니다!

</section>
<section className="data-form">
<ul className="data-form__data">
{movies?.length < 1 && (

Choose a reason for hiding this comment

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

검색 이후에 실제 검색된 결과가 존재하지 않는 경우, 검색값이 존재하지 않는다는 화면이나 알럿이 필요하다고 생각이 듭니다. 실제로 사용자의 관점에서 영화를 검색하였을 때, 변화가 발생하지 않으면 검색된 결과가 없어서 인지 검색이 수행되지 않아서인지 알 수가 없기 때문입니다.

++ 물론 로딩 컴포넌트를 띄워 실제 검색이 되고 있다는 것을 표현할 수 있으나, 검색된 결과가 존재하지 않는다는 것을 명시적으로 제시하는 것이 더 좋을 듯 합니다:)

Copy link
Author

Choose a reason for hiding this comment

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

넵..! 그부분까지 미처 생각을 해보지 못했던것 같습니다! 리팩토링 시 반영하여 보겠습니다! 🤗

one-time donation or become a patron.
</p>
</section>
<section className="search-form">

Choose a reason for hiding this comment

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

search-form 클래스명의 section는 컴포넌트로 분리하여 관리하게 되면, 메인페이지의 역할인 Search 컴포넌트에서 검색 기능을 분리할 수 있을 것으로 보이는데, 어떻게 생각하시나요?🤔

Copy link
Author

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