-
Notifications
You must be signed in to change notification settings - Fork 0
Kdt5 임승이 과제 제출 (re) #78
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
chanuuuuu
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.
끝까지 버그 수정하시느라 고생하셨습니다👍🏻 강의를 듣고 구현까지 따라하신 것 훌륭합니다. 단순히 구현을 따라는 것에서 더 나아가 실제 컴포넌트가 어떠한 방식으로 교체되는지, 로직의 분리는 어떻게 해야하는지 고민하시면서 진행하시면 좋겠습니다.
추가적으로 생각해볼만한 것을 코멘트 드렸습니다. 한번 보시고 리팩토링에 한 번 적용해보시기 바랍니다. 끝까지 화이팅!🙇🏻♂️
| @@ -0,0 +1,5 @@ | |||
| .parcel-cache/ | |||
| dist/ | |||
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.
dist directory의 내용이 포함되어있습니다. gitignore가 제대로 반영되고 있는지 확인 부탁드립니다:)
| } | ||
| <div class='the-loader hide'></div> | ||
| ` | ||
| const moviesEl = this.el.querySelector('.movies') |
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 특이성에 따르면 구현하신 페이지 내에서 MovieList 컴포넌트는 하나만 사용되므로 클래스명이 아닌 ID를 통해서 요소를 조회하는 것이 더 효율적일 것으로 보입니다. 이와 마찬가지로 아래의 loadEl 또한 ID를 통해서 조회하는 방식을 검토할 수 있을 것 같아요! 클래스명을 사용하여 조회해야한다면 그 이유도 설명해주실 수 있을까요?🤔
| function routeRender(routes) { | ||
| // 접속할 때 해시 모드가 아니면(해시가 없으면) /#/로 리다이렉트! | ||
| if (!location.hash) { | ||
| history.replaceState(null, '', '/#/') // (상태, 제목, 주소) |
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.
https://kdt-5-m2-1.vercel.app/#/abouthttps://kdt-5-m2-1.vercel.app/#/movie?id=tt0948470https://kdt-5-m2-1.vercel.app/#/love
와 같이 모든 페이지가#을 포함하고 있습니다.movie라는 단어가 해당 페이지에 대한 의미를 내포하듯, URL에서 모든 문자는 의미를 가져야하는데요.#는 어떠한 의미를 가지고 있는지 설명해주실 수 있을까요?
| ] | ||
| } | ||
| }) | ||
| window.addEventListener('popstate', () => { |
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.
popstate 이벤트를 사용하였네요. 해당 이벤트는 어떠한 경우에 발생하게 되는지, 그리고 왜 이벤트를 사용하게 되었는지 설명해주실 수 있을까요?🤔
src/store/movie.js
Outdated
| } else { | ||
| store.state.message = Error | ||
| } | ||
| page++ |
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.
여기 변경하고 있는 page라는 변수는 searchMovies()의 인자로 사용되고 있는 page를 말하고 있습니다. 기본적으로 함수의 인자로 전달되는 값을 변경하는 것을 지양해야합니다. 의도하지 않는 변경이 발생할 수 있기 때문입니다. 만약 인자로 전달한 값을 변경하기 위해서는 함수의 return을 통해 변경된 값을 반환하고, 그 값을 이용하는 방식으로 구현하는 것을 추천드립니다.
let page = 1;
function getNextPage(value) {
return value + 1; // 새로운 값을 반환
}
page = getNextPage(page);
chanuuuuu
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를 활용하여 영화 검색 기능을 구현해보는 과제
과제 결과
🔗 https://movieapp-ofryw9uf2-doitidey.vercel.app
❗ 필수 기능
❔ 선택 기능
리뷰
사용한 기술
HTML5,JavaScript,SCSS배운점
느낀점
React로 시도하다가 JS로 다시 구현했는데, 역시 React를 이해하기 위해서는 JS 공부가 필수적이며
저는 아직도 JS에 대한 기본 지식이 아직 부족하구나 느낄 수 있었습니다..... 🙂
많은 기능들을 모두 추가하지는 못했지만, 남은 시간 동안 할 수 있는 만큼은 구현해보고자 했습니다.
SCSS를 도입해보았습니다만 html에 css로 변환후 link하여 파일 갯수가 늘어나는 단점이 있습니다.
-> JS에 link 할 수 있을 것 같아 좀 더 알아보려 합니다.
스켈레톤 UI를 추가하다가 완전히 구현하지 못해 제거하여 push했습니다.
리팩토링을 진행하게 된다면 해당 기능을 다시 추가해보고 싶습니다.
전체적으로 급하게 진행하다보니 코드를 다시 한 번 보며 다시 이해하는 시간을 가져야 할 것 같습니다.