-
Notifications
You must be signed in to change notification settings - Fork 0
KDT5 안중후 2차 과제 (2nd PR) #56
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
base: main
Are you sure you want to change the base?
Conversation
sangheon-kim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
부족한점들 코멘트 달아두었습니다.
| // ################################################################################################ API FETCH | ||
| const getMovieRequest = async () => { | ||
| const url = `https://omdbapi.com/?s=${searchValue}&apikey=7035c60c` | ||
| const res = await fetch(url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
api 따로 분리해서 써주세요 컴포넌트 내부에서 하지말고
| // } | ||
| // }, [toWatch]) | ||
| // ################################################################################################ LOCAL STORAGE | ||
| const saveToLocalStorage = items => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
왜만들었는지 모르겠어요
한줄짜리를 굳이 함수로 호출할거라면 useCallback 처리도 가능하면 해주세요
| p={2}> | ||
| <Stack direction="row"> | ||
| <Button | ||
| startIcon={<TheatersOutlinedIcon />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굳이 StartIcon으로 받은 이유가 뭐에요? children으로 넘기면되는데
| letterSpacing: 13 | ||
| }} | ||
| onClick={() => { | ||
| navigate('/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
익명함수로 쓰시지마시고 위에서 따로 써주세요
| @@ -0,0 +1,73 @@ | |||
| import { useCallback, useEffect, useState } from 'react' | |||
| //영화 목록 좌/우 커서 이동 시 자동 스크롤 | |||
| const clamp = (x, min, max) => Math.min(Math.max(x, min), max) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석이랑 매칭이 안됩니다.
| </> | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fragment를 왜 쓴지 모르겠어요
| placeholder="Search" | ||
| value={props.value} | ||
| onKeyDown={event => { | ||
| if (event.keyCode === 13) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Form태그로 감싸서 onSubmit으로 처리해주세요.
| //포스터 우측 하단 추가/제거 아이콘 컴포넌트 | ||
| const WatchLater = () => { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이전과 동일하게 fragment확인바랍니다.
| import '@fontsource/roboto/500.css' | ||
| import '@fontsource/roboto/700.css' | ||
| import '@fontsource/roboto/900.css' | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 처럼 쓰지 않고, css내부에서 @import를 써도됐는데 이렇게 쓰신 이유를 모르겠습니다.
| { | ||
| path: '/about', | ||
| element: <About /> | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
겹치는 부분에 대해서는 Layout컴포넌트를 만들어서 Outlet을 썼다면 어떨까요
🎬 OMDb API 활용 영화검색 사이트 'CineMap'
배포주소
화면 구성 🪟
기술 스택
Config
Development
Deployment
요구사항
❗ 필수
❔ 선택
추가기능
Watch Later
좌/우 커서 이동시 자동 스크롤
프로젝트 회고
배운점(Insight)
1차 과제에서 README에 대한 피드백을 받았고 이번 과제를 위해 다양한 예시들을 찾아보면서 문서의 중요성 및 작성방법을 배웠습니다.
개인적으로 구글의 모든 제품에 관심이 많아서 1차 과제에서도 Material Design 웹사이트 레이아웃을 클론코딩했습니다. 이번에는 더 나아가 Material Design이 적용된 MUI를 사용해볼 수 있어 몰입해서 프로젝트를 진행할 수 있었고 이전 멘토링 답변에서 들었던 생산성과 자동화를 React + MUI를 통해 느낄 수 있었습니다.
MUI사용으로 인해 더 효율적인 작업이 가능할 걸로 기대했지만 사용법에 익숙해지기까지 많은 시간이 걸린다는 걸 간과해 간단한 스타일 적용에도 어려움을 겪었습니다. 아무리 좋은 기술이더라도 초기에는 학습시간에 대한 기회비용을 고려해야한다는 점을 배웠습니다.
(검색 => 포스터 클릭 => 영화 상세정보 조회)의 과정에서 데이터 전달시 컴포넌트 구성이 (조상 => 부모 => 자식) 구조였다면 props 전달이 용이했겠지만 (부모 => 자식 => 페이지 변경 후 데이터 전달)의 과정이 되었고 결과적으로 useNavigate에서 state를 전달할 수 있는 방법이 있다는 걸 배웠습니다.
문제점(Challenge)
React로 만들어진 해당 프로젝트 내에 사용된 기술들의 작동원리를 JavaScript로 모두 설명할 수 있을지 자문해보면 아직 못할 것 같습니다. 또한 과제 진행 중 마주한 오류들에서도 JS기본의 중요성을 느꼈고 꾸준한 자바스크립트 학습의 필요성을 느꼈습니다.
MUI사용으로 별도의 CSS파일 관리 없이 스타일 작업이 가능했지만 컴포넌트에 직접 작성된 스타일 속성들로 인해 가독성을 해친다는 느낌을 받았습니다.
자동스크롤의 모션이 부드럽지 않지만 라이브러리 사용으로 인해 어떤 방식으로 수정을 해야할지 갈피를 잡지 못해 작동 확인에만 그친 후 추가적인 수정을 하지 못했습니다.
궁금한 점(Questions)
MUI사용시 Layout설정시 App Bar, Stack, Box등 여러 방법의 차이점이 궁금합니다. HTML에서는 1. div + class를 사용시/2. Semantic tags를 사용시 의미적으로 명확한 차이가 있지만 이미 만들어진 컴포넌트를 사용시 어떤 차이점에 주의해야할지 궁금합니다.
처음엔 Home.jsx에 렌더링을 바로 시도했지만 가장 먼저 확인하게 되는 페이지에서 긴 코드를 마주하게 되어 App.jsx로 컴포넌트 분리 후 Home은 App을 import해 홈페이지라는 명시를 위한 용도로만 사용했습니다. 라우터 관리 시에 더 보편적이고 좋은 방식이 어떤건지 궁금합니다.
1.App.jsx(Home)에서 사용한 AppHeader.jsx로 별개의 Component로 분리했지만 App.jsx 내에서 SearchBox.jsx를 AppHeader내부로 적용시키지 못해 컴포넌트 사용이 아닌 AppHeader.jsx내부의 코드를 작성해 감싸 Header를 구성했습니다.
2.1.SearchBox 컴포넌트를 AppHeader 내부에 하나의 컴포넌트로 병합시키는 방법도 고려했지만 Movie/About등 타 페이지의 헤더를 검색창 없이 구현하고 싶었습니다.
2.2 SearchBox가 검색기능로직에 관련한 데이터를 사용하고 있었기 때문에도 분리의 어려움이 있었습니다.)
위와 같은 문제에서 더 좋은 방식이 있는지 궁금합니다.