[2단계 - 상세 정보 & UI/UX 개선하기] 센트(김영우) 미션 제출합니다. #69
Conversation
kyw0716
changed the title
각 요청에 대한 함수 선언 => 요청 함수는 공통으로 사용, 요청 url 생성 함수 추가
There was a problem hiding this comment.
안녕하세요! 센트! 티케입니다. 🧚♀️
드디어 마지막 미션이네요~ 대체적으로 코드가 많이 읽기 편해진 것 같아요.! 마지막인만큼 전체적으로 코드를 보면서 피드백을 드리려고 노력했는데, 1단계에서 넘어갔던 부분이 다시 보여서 리뷰가 조큼 많네요..! 🙇♀️🙇♀️ 코멘트들과 PR 메세지에 대한 피드백 확인해주세요~
PR 메세지에 대한 피드백
클릭보다 마우스를 올렸을 때 별점을 선택할 수 있게 하는 것이 사용성에 더 좋을 것이라 판단해 해당 기능을 추가해보았습니다!
저는 UX에서 '어포던스 디자인'을 제공하는 것과 사용자가 예측할 수 있는 UX를 제공하는 것도 중요하다고 생각해요. 어포던스라고 하면, 어떠한 행동을 유도하는 것을 의미하는데 예를 들어서 예전 아이폰의 '밀어서 잠금해제'를 예를 들 수 있을 것 같아요. (엇,, 혹시 모르는 세대라면 검색해보시면 나올거에요.........) hover의 장점은 제가 굳이 누르지 않아도 별점이 움직이는 거지만, 대부분의 UX에서는 이러한 별점 매기기는 찾기가 어렵기 때문에 사용자는 예측하지 못했던 인터렉션이 일어나서 당황할 수 있어요. (혹시 참고한 프로덕트가 있다면 소개해주세요..!) 실수로 마우스로 hover를 했는데 별점이 매겨질 수 있겠죠. 또 '클릭'이라는 활동을 함으로서 '내가 준 별점을 확정한다.'이런 느낌도 줄 수 있는데 지금은 클릭을 굳이 하지 않아도 별점이 매겨져서 혼란을 줄수도 있구요. 물론 클릭이 무조건적으로 좋은 UX라고는 말은 못하지만, 현재 0점으로 되돌릴 수 없는게 가장 크리티컬 한 것 같습니다 😢
그리고 접근성 측면에서 '키보드로 모든 조작이 가능해야한다.', '모바일로도 동일한 기능을 동작할 수 있어야한다.'가 하나의 중요한 체크리스트입니다. 따라서 mouseover를 사용한다면 키보드로는 어떻게 동작을 하는지, 실제 모바일에서는 정상 동작을 하는지, 그게 아니라면 keyboard, touch 이벤트도 핸들러로 등록해야하는 건 아닌지 꼭 알아봐야 합니다. (물론 modal을 esc로 닫는거 외에는 지금 어떤 것도 접근성을 지킨건 아니고 이번 미션의 중요한 사항은 아니지만 실무에서는 이런 부분까지 고려해야한다는 것을 알려드리기 위한거니 참고하면 좋을 것 같아요~)
혹시 이런건 어떨까요? https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/range 별표 UX를 연결하는건 도전적인 과제가 될 거에요. 저도 예전에 input type="ragne"를 활용하여 별점 매기기 UX를 만든적이 있는데, 크로스브라우징까지 체크하면서 만드느라 눈물을 흘렸던 기억이 있네요,, 나중에 여유가 있다면 관련 키워드로 검색해보면 좋을 것 같아요~
추가적으로 기존에는 전체 스켈레톤 리스트를 추가했다 지웠다 하는 방식으로 로딩 처리를 해주었지만, 이번엔 미리 스켈레톤 리스트를 추가하거나 지우지 않고 스켈레톤 리스트의 display 속성을 변경하는 방식으로 로딩 처리를 진행해보았습니다!
저는 이 방식이 너무 마음에 들어요! 사용자가 검정색 화면을 보지 않아도 되어서 좋네요 💯
src/index.js
Outdated
There was a problem hiding this comment.
처음 주어졌을 때 index.js로 주어져서 변경 없이 그대로 진행했었습니다!!
| @@ -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; | |||
There was a problem hiding this comment.
'#header' is declared but its value is never read. 라는 오류를 보여주고 있어요!
어떤 문제가 있는지 한 번 살펴볼까요?
There was a problem hiding this comment.
혹시 나중에 #header를 사용할지도 모른다고 생각해서 그대로 두었는데 결국 사용되지 않아서 생긴 오류인 것 같습니다! 오류를 해결하자고 그냥 #header를 지우기보다 #header가 무언가 하는 역할이 있도록 수정해보는건 어떨까 하고 #header가 입력값을 반환해주는 메서드를 가지도록 수정해보았습니다!
수정한 커밋: f1ce9f8
There was a problem hiding this comment.
오,, 아래와 바꿔도 상관없을 거라고 생각했는데, 다른 방법을 사용하셨군요~
new Header(
$header,
this.onSubmitSearchKeyword.bind(this),
this.onClickLogoImage.bind(this)
);
src/App.ts
Outdated
| this.bindEvent($movieList); | ||
| } | ||
|
|
||
| bindEvent($movieList: Element) { |
There was a problem hiding this comment.
bindEvent라는 네이밍은 어떤 이벤트인지 감이 잘 안오는거 같아요 :(
There was a problem hiding this comment.
다시 보니 뭔가 메서드명만 보고 해당 메서드가 어떤 역할을 할 지 바로 알아차리기 힘들어 보이네요😭😭 어떻게 바꿔볼까 고민해보다 이벤트 리스너를 추가해주어 이벤트 리스닝(?)을 시작한다는 의미를 전달하기 위해 startEventListening으로 바꿔보려 했는데 이것도 마음에 들지 않네요 ㅠㅠ 혹시 티케가 생각하는 적합한 메서드명이 있으시다면 공유 부탁드립니다!!
There was a problem hiding this comment.
음.. 가장 간단하게라면 저라면 여러가지 event들이 있어서, bindEvents라는 복수형을 사용할 것 같아요.
There was a problem hiding this comment.
다소 귀찮을 수 있는 질문이었는데 답변 감사합니다!! 👍👍👍
(이 부분은 혹시 여유가 되신다면 답변 부탁드립니다!! 바쁘시다면 굳이 시간 내셔서 답변하지 않으셔도 괜찮습니다!!!)
혹시 티케가 우테코 미션을 진행할 때 이벤트 리스너를 어떻게 추가하셨었는지 궁급합니다. 저는 1레벨 동안 고민해 본 결과 해당 이벤트의 책임을 가지고 있는 컴포넌트 내에서 모든 이벤트를 한번에 추가해주는 것이 좋을 것이라 판단해 bindEvent라는 메서드를 사용했었는데 티케만의 방법이 있으셨는지 궁금합니다!
src/App.ts
Outdated
| const movieCard = event.target.closest("li"); | ||
| const movieId = movieCard?.dataset.movieId; | ||
|
|
||
| if (movieId) this.onClickMovieCard(Number(movieId)); |
There was a problem hiding this comment.
movieCard가 없거나 movieId가 없는 경우에는 사용자는 눌러도 어떠한 결과물을 못받는 걸까요?
There was a problem hiding this comment.
넵!! 클릭한 대상이 영화 카드가 아니라면 아무 동작이 되지 않도록 하기 위해 코드를 위와 같이 작성했었습니다!
There was a problem hiding this comment.
오 사용자는 clickable 한거니깐 선택을 했는데, 어떠한 반응도 없다면 (심지어 마우스도 클릭할 수 있게 cursor형태면) 좋지 않은 UX를 경험할 거 같아요 :(
There was a problem hiding this comment.
렌더링 된 영화 카드라면 당연히 movieId를 가지고 있을거라 생각해서 movieId가 없을 경우를 따로 생각해보지 않았는데 티케의 리뷰를 보니 혹시나 movieId가 없을 경우엔 사용자 입장에서 굉장히 당황스러울 것 같다는 생각이 드네요!
만약 movieId가 없을 경우엔 alert를 통해 선택한 영화의 상세 정보가 없다는 안내 문구를 띄우는 방식을 적용해보았습니다!
수정한 커밋: c772c19
src/components/MovieList/index.ts
Outdated
| }); | ||
| } | ||
| } | ||
|
|
||
| onClickMoreButton() { | ||
| fetchNextPage() { |
There was a problem hiding this comment.
리뷰를 읽어보고 어떤 부분을 추상화 할 수 있을지 고민해보았는데 제가 생각할 수 있는 선에서 추상화 할 수 있는 부분은 this.#state의 page 프로퍼티를 증가 시키는 부분을 메서드로 분리하는 정도가 있을 것이라고 생각했습니다. 혹시 티케가 생각한 추상화 할 수 있는 부분들에는 어떤 것이 있는지 궁금합니다!! 꽤 오래 고민했는데도 저 부분 이외에는 더 떠오르는 것이 없네요😭😭
src/components/Modal/index.ts
Outdated
| }); | ||
| } | ||
|
|
||
| open(modalType: "movieDetail" | string, movieId: number | undefined) { |
There was a problem hiding this comment.
movieDetail과 string이라.. 조금 modalType이 애매한 것 같아요
There was a problem hiding this comment.
일단 이후 무언가 들어올 것으로 예상하고 타입을 지정했었는데 다시 보니 왜 저렇게 했나 싶은 생각이 드네요😭 string을 빼주고 "movieDetail"만 남겨주었습니다!
수정한 커밋: 34e0c0a
There was a problem hiding this comment.
modelType을 선언해두고, "movieDetail" 대신 타입을 사용해도 좋을 것 같네용 :)
There was a problem hiding this comment.
타입을 따로 선언할 생각을 못했네요😅😅 모달내에서 사용될 타입을 정의하는 파일을 따로 만들어 티케가 리뷰해주신 내용을 적용해봤습니다!
수정한 커밋: 521b1e9
|
티케!! 항상 정성어린 피드백 감사해요👍👍 리뷰 남겨주신 부분들 최대한 반영하려고 노력해봤습니다!! (css 관련 피드백은 제가 아직 익숙치 않아서 잘 반영하지 못한 것 같네요 ㅠㅠ) PR 메세지에 대한 피드백에서 여쭤보신
이 부분에 대해 해명(?)을 해보자면, 사실 제가 원했던 것은 클릭한 이후 드래그를 했을 때 별점이 올라가는 것이었습니다.... 하지만 개발 하면서 클릭 후 드래그 하는 이벤트를 어떻게 다뤄야 할지 막막해서 일단 수정한 커밋: b5f09e8 이야기 해주신 추가적인 도전과제도 방학 동안에 시간 내서 한번 고민해보도록 하겠습니다! 상세한 리뷰 다시 한번 감사드립니다!!! |
There was a problem hiding this comment.
센트~! 미션하느라 고생많았습니다!
드디어 Level1이 끝이네요! 마지막 미션은 기간적 여유가 별로 없어서(?) 다소 아쉬운 부분이 많이 느껴지는데, 방학때 한번 다시 살펴보거나, 혹은 level2 할 때 좀더 신경써서 해주시면 좋을 것 같아요! 특히 API 요청,응답 관련해서 추상화를 어떻게 하면 좋을지, 어떻게 오류처리를 해주면 좋을지 등 매우 어려운 부분이지만 이런 부분도 고민을 많이 해보시는 걸 추천드려요! 코멘트를 몇개 남겼는데 확인해주시고, 질문이 있다면 이메일 알람은 오니깐 코멘트 이어서 남겨주셔도 좋고, DM 주셔도 좋습니다~
즐거운 방학 보내세요! 💌
src/components/Header/index.ts
Outdated
|
|
||
| bindEvent( |
There was a problem hiding this comment.
$searchInput.addEventListener("click", function () {
this.select();
});
여기서 this.select()는 어떤 역할을 하나요?
또, 왜 App 컴포넌트에서 이벤트 리스너를 추가해주도록 바뀌었는지 알 수 있을까요?
src/components/MovieList/index.ts
Outdated
| } | ||
|
|
||
| reset(state: showType, searchKeyword?: string) { | ||
| changeShowTarget(state: showType, searchKeyword?: string) { |
There was a problem hiding this comment.
오, 저는 요청/응답 부분은 바뀐 방향이 더 복잡해지지 않았나라는 생각이 드네요..!
getUrl을 request를 호출하는 쪽에서 알아야하는지에 대한 의문도 있고,
getUrl 자체도 너무 복잡하기도 하고요..!
요청/응답에 대한 부분과 이에 관한 에러핸들링은 좀 더 고민해보시면 좋을 것 같습니다.
중복 처리를 하려다보니 너무 크게 함수를 묶은건 아닌지 다시 고려해봐도 좋을 것 같구요 :)
하다가 너무 어렵거나, 아니면 잘 모르겠으면 고민한 이후에 코멘트 남겨주시거나 DM주시면 같이 고민해볼 수 있을것 같아요~
안녕하세요 티케!! 지난번에 남겨주신 꼼꼼한 리뷰 감사드립니다👍👍
배포 링크: https://kyw0716.github.io/javascript-movie-review/
이번 단계 에서는 지난번에 구현했던 내용들에 다음의 요구 사항을 추가로 구현해보았습니다!
리뷰에 앞서 제 코드를 이해하시기 편하게 새로 추가한 컴포넌트와 새롭게 추가한 기능을 어떤 방식으로 구현했는지 간략히 설명 드리겠습니다!
추가한 컴포넌트 목록
모달을 재사용할 수 있도록 모달의 기능을 추상화하여 컴포넌트로 만들어보았습니다!
Modal컴포넌트는 자신을 열고 닫는 기능만을 가지고, 내부는 다른 컴포넌트를 통해 채울 수 있도록 코드를 작성해보았습니다.Modal 컴포넌트
역할
모달의 전체적인 구조를 만들어주는 역할을 수행합니다. 모달을 열고 닫는 기능을 수행하고, 내부를 어떤 요소로 채울지 선택할 수 있는 기능을 포함하고 있습니다.
메서드 목록
bindEvent
esc키 입력을 통해 모달을 닫는 기능, 배경을 클릭해 모달을 닫는 기능, 닫기 버튼을 클릭해 모달을 닫는 기능, 별을 클릭해 별점을 등록하는 기능 등의 이벤트를 바인딩 해주는 메서드open
모달을 여는 기능을 수행하는 메서드
close
모달을 닫는 기능을 수행하는 메서드
MovieDetailModal 컴포넌트
역할
모달 내부를 영화 상세정보로 채워주는 역할을 수행합니다.
메서드 목록
bindEvent
mouseover이벤트를 통해 별점을 선택하는 기능을 바인딩 해주는 메서드render
영화 상세정보 모달 내부를 렌더링 해주는 메서드
getMovieDetailTemplate
영화 상세정보 모달 템플릿을 반환하는 메서드
반응형 디자인 적용
css에 미디어 쿼리를 이용해 데스크톱 모니터, 테블릿, 모바일 각각에 맞는 디자인을 적용해주는 방식으로 반응형 디자인을 적용시켜주었습니다!
상세 정보 모달 구현
뒤로 가기를 통해 모달을 닫을 수 있도록 하라는 요구사항이 있어 window에popstate이벤트를 통해 모달을 열고 닫을 수 있도록 모달을 구현해 보았습니다. 각 영화 카드는a태그로 감싸져 있어 클릭했을 시 해시를 통해 영화의 고유 아이디를url에 추가해주고, 이를 가져와 영화의 디테일 정보를 요청해 반환된 값으로 모달 내부의 상세 정보를 채워주었습니다!요구 사항이 조금 변경되어 굳이
popstate를 사용하지 않아도 돼서 영화 카드를 클릭했을 시 클릭된 영화의 id를 받아와 그 값으로 api에 요청을 발생시켜 반환 값으로 상세 정보 모달 내부를 채워주었습니다!사용자 별점 기능 구현
사용자가 각 영화에 별점을 남긴 것을 저장할 수 있도록
localStorage를 활용했습니다. 각 영화의 고유 아이디와 사용자가 남긴 별점을 key, value 쌍으로localStorage에 저장하는 방식을 사용했습니다!무한 스크롤 기능 구현
무한 스크롤 기능을 구현하기 위해
intersection observerAPI를 활용해보았습니다. 뷰포트에 기존에 있던 더보기 버튼 부분이 보여지게 되면 요청을 넣어 반환 값으로 화면을 채우는 방식을 사용했습니다!추가적으로 기존에는 전체 스켈레톤 리스트를 추가했다 지웠다 하는 방식으로 로딩 처리를 해주었지만, 이번엔 미리 스켈레톤 리스트를 추가하거나 지우지 않고 스켈레톤 리스트의
display속성을 변경하는 방식으로 로딩 처리를 진행해보았습니다!