-
Notifications
You must be signed in to change notification settings - Fork 8
[Front_end]강성훈 10주차 과제 제출합니다. #7
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
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.
getData와 getSearchData 함수가 거의 동일한 구조이니, 파라미터를 받아 처리하는 공통 API 호출 함수로 통합하는 방향도 있을 것 같네요!
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.
감사합니다
| options2 | ||
| ); | ||
| const data2 = await response2.json(); | ||
| const $data2 = data2.results; |
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.
처음에 배울 때 DOM 선택자에 사용하는 것으로 배웠는데 그렇게 사용하다 보니까 약간 반복되는 내용에 쓰게 된 것 같아요
| background-color: antiquewhite; | ||
| align-items: center; | ||
| justify-content: space-between; | ||
| background-color: #000; |
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.
background-color가 두 번 선언되었네요. 혹시 우선순위를 조정하려고 하셨나요?
수정할 필요가 있어보입니다!
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 복붙해서 그런것 같습니다
| data.forEach((item) => { | ||
| const box = document.createElement('div'); | ||
| box.classList.add('section__main--box'); | ||
|
|
||
| const img = makeImage(item.poster_path); | ||
| const title = makeTitle(item.title); | ||
| const rate = makeRate(item.vote_average); | ||
|
|
||
| box.append(img, title, rate); | ||
| $movies.append(box); | ||
| }); |
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.
데이터가 많아질 경우, DOM을 계속 추가하면 성능 저하 이슈가 발생할 수 있을 것 같습니다. 이럴 경우, 어떻게 해결하면 좋을까요? 한 번 고민해봐도 좋을 것 같아요!
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.
grid와 flex를 같이 쓴 이유가 있을까요? 하나를 선택하여 일관되게 짜면 좋을 것 같아요. 만약 혼용이 불가피하다면, grid로 큰 레이아웃을 잡고 내부 요소를 flex로 조정해봅시다.
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 사용하여 시작하면 바로 인기 영화 뜨게 하기
검색 API 사용하여 영화를 검색할 수 있음
검색 할 때 찾는 것이 없다면 따로 처리함
검색 했을 때 영화 개수에 따라 더 보기 버튼 유무 설정
미디어 쿼리를 사용해서 모바일, 태블릿, 데스크톱 에서 모두 사용할 수 있게 만듦
미디어 쿼리를 사용해서 모바일과 태블릿,데스크톱에서 다르게 설정함