[1단계 - 영화 목록 불러오기] 야미(이다인) 미션 제출합니다. #5

feb-dain · 2023-03-16T08:05:37Z

안녕하세요 포코!
저번 미션에서 타입 스크립트 사용과 컴포넌트 분리, e2e 테스트를 처음으로 해봐서
이번에는 조금 더 잘하고 싶었는데 아직 갈 길이 먼 것 같습니다😂

영화관 앱 바로가기

구조

assets : 이미지를 모아 놓은 폴더
components
- Header.ts : 헤더 영역 컴포넌트
- headerHandler.ts : 헤더 영역 관련 기능 관리
- MovieList.ts : 메인 영역(영화 목록) 컴포넌트
- movieListHandler.ts : 영화 목록 관련 기능 관리
- NoResultsMessage.ts : 검색후 결과가 없을 때 보여주는 페이지 컴포넌트
- Skeleton.ts : 스켈레톤 컴포넌트
css
- reset.css : 기본 css 속성 초기화
- common.css : 앱 스타일 테마
domain
- movieApi.ts : api 정보를 가져와서 저장하고 관리
- movieStore.ts : 영화 목록 저장, 추가 및 재렌더링 함수 작동
utils
- eventListener.ts : 이벤트 리스너를 간결하게 사용하기 위한 유틸
- selector : 돔 요소를 선택하고 검사하는 것을 간결하게 하기 위한 유틸
type.ts : 영화 정보, api 정보 타입 선언
index.ts : 영화관 앱 실행 기능

웹 컴포넌트 (customElement 사용)

자연스럽게 ui 영역으로 컴포넌트 분리 가능
태그 이름만 봐도 웹 페이지에서 어떤 일을 하고 있는지 알 수 있음
재사용성이 좋음
⇒ 이와 같은 이유로 웹 컴포넌트를 사용하기로 했습니다.
장점이 많고 배워두면 좋을 것 같아 이번 미션에서 웹 컴포넌트를 도입했습니다.

🤓 노력한 점

최대한 오류없이 모든 기능을 구현할 수 있도록 노력했습니다.
함수명, 변수명 등을 보고 어떤 기능을 하는지 알 수 있도록 작명에 신경썼습니다.
함수 분리, 코드 가독성(함수 순서, 개행 등)에 신경 썼습니다.
앱 사용성에 신경을 썼습니다.
- 검색 결과가 없을 경우, 빈 페이지 대신 결과 없음 페이지 렌더링
- 포스터 이미지가 없는 영화일 경우, 대체 이미지 렌더링
  ❗고양이들을 검색하시면 포스터 이미지가 없는 영화를 찾을 수 있습니다.
- 로고 클릭시 처음 화면(인기 있는 영화 목록)으로 이동

🥲 아쉬운 점

시간이 없어 상수화를 진행하지 못했습니다.
잘못된 케이스도 e2e 테스트를 하고 싶었는데 하지 못했습니다.
배포 문제
- api 키를 숨기면서 배포를 하고 싶었는데 시간이 없어 하지 못했습니다.

Co-authored-by: Gabriel Ju Hyun, Yoon <[email protected]>

copy-webpack-plugin을 이용한 이미지 로드 문제 해결

함수명 변경, 전체적인 구조 변경

+ 결과 없음 페이지 css 수정

woowahan-cron · 2023-03-16T13:12:54Z

웹 컴포넌트를 왜 써야하는지까지 명시한 점이 너무 인상깊네요.
리뷰어님한테 좋은 피드백 받으시면서 한 단계 업그레이드해서 레벨1 가뿐하게 마쳐볼까요!
마지막까지 퐈이팅

pocojang

@feb-dain 👋
안녕하세요
저는 리뷰어 포코입니다! 반가워요

처음 만났지만 문서 정리부터 포맷팅까지.. 너무너무 깔끔해서 정말 뿌듯했어요.

대충 눈이 정화되었다는 표현..

피드백

생성자

생성자를 잘 활용하는 것 같아요.
다만 생성자가 이런 일까지..? 싶은게 있어서 아쉬움은 들었어요!

구조

불필요하고 과도한 오버엔지니어링이 전혀 보이지 않아서 정말 좋았습니다.
정말 클린하고 깔끔해서 어찌됐든 잘 읽혀서 좋습니다.

이벤트

가장 아쉬움이 드는 부분이에요.
일단 시도 자체는 좋은데 성급한 최적화로 보이기도 하고..
이 부분을 개선해보는 훈련을 해보시면 좋겠네요

Web Components

학습 측면에서 가장 좋은 시도 중 하나라고 볼 수 있는데요.
아쉽게도 활용이 정말 제한적으로 보여요!

기타

상수화는 차근차근 해보셔도 됩니다
일단 테스트는 시작이라도 했으니 차근차근 개선해보시면 될 것 같아요
api 키를 숨겨야하는 이유만 이해하셔도 충분합니다!
API때문에 camelCase와 snake_case가 혼용되어 필드가 어지러울텐데 개선해보셔도 좋아요 :)

코드량이 많지 않아 피드백 드릴 부분도 많지 않네요!
궁금하신게 있다면 편하게 남겨주시고 다시 리뷰요청 해주세요!

pocojang · 2023-03-18T11:49:52Z

cypress/e2e/movie.spec.cy.ts

@@ -0,0 +1,25 @@
+describe("영화관 앱 테스트.", () => {
+  beforeEach(() => {
+    cy.visit("http://localhost:8080/");


pocojang · 2023-03-18T11:50:23Z

index.html

-<html lang="en">
+<html lang="ko">


pocojang · 2023-03-18T11:52:19Z

src/components/MovieList.ts

+            onerror="
+              this.style.border='1px solid #e2e2e2';
+              this.src='https://www.themoviedb.org/assets/2/v4/glyphicons/basic/glyphicons-basic-38-picture-grey-c2ebdbb057f2a7614185931650f8cee23fa137b93812ccb132b9df511df1cfac.svg';
+            "
+            loading="lazy"


pocojang · 2023-03-18T11:52:49Z

src/components/MovieList.ts

+  renderMovie(movie: IMovie) {
+    return `
+    <li>
+      <a href="#">


하이퍼링크로 #을 지정해놓은 이유가 어떻게 될까요?

템플릿을 그대로 가져왔습니다😂 커서 포인트 대신 하이퍼 링크를 #으로 지정하신 것 같은데, 클릭하면 맨 위로 올라가는 문제 때문에 수정하려고 했으나 시간이 없어서 수정하지 못했습니다!

[cf49660] 불필요한 a태그 삭제했습니다!

<a>를 삭제하라는 것은 아녔지만.. 더 좋은 방법이 있다면 고민해보세요 후후..

pocojang · 2023-03-18T11:53:21Z

src/components/Skeleton.ts

+      </li>
+    `;
+  }
+}


🧻 </li> `; 이렇게 백틱을 한 줄에 붙여써야 한다는 말씀이실까요...?

pocojang · 2023-03-18T12:06:45Z

src/domain/movieApi.ts

+  last_keyword: "",
+
+  showPopularMovies() {
+    fetchMovieInfo("movie/popular", "");


빈문자열을 의미없이 넘기는 것보다는 선택적인 매개변수로 사용하거나 객체를 주고 받는게 낫지 않을까요?

[46d29a8] 선택적 매개변수를 사용했습니다!

pocojang · 2023-03-18T12:07:17Z

src/domain/movieApi.ts

+const fetchMovieInfo = async (endpoint: string, keyword: string) => {
+  const url = buildUrl(endpoint, keyword);
+  const response = await fetch(url);
+  catchError(response.status);


try~catch를 분리해보는 시도는 좋으나 커버하는 범위가 너무 애매한 것 같은데 정말 안전한 로직을 만든다고 생각하고 범위를 넓혀보는게 어떨까요?

[623f7d6] tmdb(API)에서 제공해주는 json을 이용해 더 안전하게 변경했습니다!

pocojang · 2023-03-18T12:08:04Z

src/components/movieListHandler.ts

+  $("#more-button").remove();
+};
+
+export const renderSkeletons = () => {


스스로는 렌더링할 수 없는 문자열뿐이라 render()의 의미가 애매한 것 같은데 어떻게 생각하시나요?

[4834c40] 함수명 변경했습니다! render -> make

pocojang · 2023-03-18T12:09:34Z

src/components/movieListHandler.ts

+import MovieList from "./MovieList";
+
+export const onClickMoreButton = () => {
+  executeEventListener($("#more-button"), "click", async () => {


$("#more-button"), "click"만으로 이벤트에 대한 의미해석은 충분한데요.
함수조차 onClickMoreButton()와 같은 같은 네이밍으로 명시하고 있으니 어떤 일을 일으키는 콜백 함수인지 예측하기 어렵네요ㅠㅠ

[4834c40] 함수명 변경했습니다!

pocojang · 2023-03-18T12:16:45Z

src/components/Header.ts

+    onSubmitSearchBox();
+    onClickLogo();


마치 함수를 호출하고 있는 것 같지만.. 전혀 아니네요
의미와는 다른 사용 방법 같습니다!

[4834c40] 함수명 변경했습니다!

- Type assertion 제거 - `[]` 대신 `Array<>` 사용

+ 함수명/타입명 등 변경

- API가 성공적으로 응답하는지 테스트 - 네트워크 요청을 가로챈 상태에서 테스트

feb-dain · 2023-03-20T08:59:19Z

안녕하세요 포코! 피드백 반영하여 수정 완료했습니다!

영화관 앱 바로가기

❓️ 궁금한 점
intercept를 이용해 네트워크 요청을 가로챈 상태에서 테스트하는 걸 처음 해봤는데, 잘 된 건지 모르겠습니다😂 이렇게 하는 게 맞나요..?

✅ 피드백 반영

[3bc416d] 불필요한 주석 삭제
[d85a393] I- prefix 삭제 및 타입 선언 방식 변경 (Type assertion 제거)
[ae1e790] 이벤트 리스너 유틸 삭제 (확장성 개선후 적용하겠습니다!)
[58c0c35] movieApi 필드 camelCase로 변경
[46d29a8] new URLSearchParams 사용 (이런 게 있는지 몰랐는데 알려주셔서 활용해보았습니다!)
[4834c40] 함수명 변경
[623f7d6] 에러 핸들러 개선

movieStore는 도메인 폴더에서 뺄 지 아니면 로직을 수정할 지 고민중입니다!

+) 사용성

검색 결과가 없을 경우, 빈 페이지 대신 결과 없음 페이지 렌더링
포스터 이미지가 없는 영화일 경우, 대체 이미지 렌더링
- ❗123을 검색하시면 포스터 이미지가 없는 영화를 찾을 수 있습니다.
로고 클릭시 처음 화면(인기 있는 영화 목록)으로 이동

pocojang

@feb-dain

피드백

비동기

비동기는 정말 공부할게 너무나도 많고 어렵습니다.
현업에서 런타임 + 비동기 대응은 프론트엔드 개발자의 정말 엄청난 덕목이라고 볼 수도 있을정도에요.
근데 이 부분에 대해 코드도 적절하게 분리하고 대응도 잘 하신 점 정말 멋졌어요 👍
이제 방법은 moive와 api를.. 이별시켜보는 것도 엄청난 배움이 되실겁니다 😮
또한 더 다양한 엣지케이스도 대응해보시면 좋습니다.

구조

이벤트 부분은 확실히 아직 더 생각해보고 고민해볼 부분이 많을 것 같고요.
음.. 핸들러는 컴포넌트 디렉터리 내부에 있어야할까? 이 부분도 의문이에요!
하지만 그 외에는 깔끔하게 요구사항을 충족하는 적당한 규모와 시도로 보입니다 💯

intercept� 질문

사실 저도 확신이 들지 않아 질문을 받고 한참을 검색해봤었는데요.
결국 우리는 TDD를 공부하는 것이고 Cypress는 도구일뿐이라고 생각해보시는게 좋아요.
그렇다면 내가 지향하는 TDD가 가장 우선순위이며 그 다음으로는 Cypress를 사용하고 있으니 비동기 통신 테스트시 Cypress를 어떻게 사용할까? 고민해보시면 되는거죠 💪

그렇다면 질문에 답이 될 것이라고 믿고요. 그 선택도 제가 봤을때는 잘 선택하신 것 같습니다!
다만 제 개인적인 생각이지만 API테스트는 프론트 개발자가 아닌 백엔드 개발자가 하는게 맞다고 생각합니다 후후... 💭

사용성

그럭저럭 괜찮은 편이네요!
큰 문제는 못느꼈어요. 근데 19금 영화가 유난히 상위권으로 나와서 깜짝 놀라긴했습니다.

한가지 애매한게 있는데요. 검색어를 입려해서 특정 키워드에 맞는 영화 리스트를 본 다음 다시 전체 영화리스트를 볼 수 있는 방법이 없습니다 😭
이점 참고해보시면 좋겠어요 🤔

그럼 2단계에서 다시 만나요~ 👋

feb-dain · 2023-03-20T22:33:52Z

앱 사용성 부분은 pr메시지에만 적어뒀었는데 코멘트에도 다시 추가했습니다! 혹시 오류가 있다면 알려주시길 바랍니다 😊

feb-dain and others added 26 commits March 14, 2023 14:49

docs: 기능 목록과 요구 사항 작성

5db1c55

Co-authored-by: Gabriel Ju Hyun, Yoon <[email protected]>

chore: 폴더 정리 및 프리티어 적용

281a479

fix: 이미지 업로드 문제 해결

fbc7b98

feat: 헤더 컴포넌트화

82760c2

feat: 영화 목록 컴포넌트화

904e6c0

style: export 방법 변경

509f9f0

feat: 영화 아이템 컴포넌트화

cbd6e3d

feat: API 요청 및 API 키 은닉화

440d6fe

feat: API에서 가져온 정보 렌더링

c866b88

feat: 더보기 클릭시 영화 목록 추가 및 렌더링

3947f75

refactor: 영화 목록 관련 함수 분리

f77159e

feat: 인기순으로 영화 정렬

8cec080

fix: 이미지 불러오기 문제 해결

c936bdd

copy-webpack-plugin을 이용한 이미지 로드 문제 해결

feat: 검색 기능 구현 및 정렬 기능 삭제

7197252

feat: 목록 이름 대신 검색 결과 표시하는 기능 추가

cfa5193

feat: 별점을 소수점 1자리까지만 표시는

658cc76

feat: 로고 클릭시 처음 화면으로 돌아가는 기능 구현

b362bed

feat: 요청한 API 상태 코드에 따라 오류 메시지 출력

ae6383e

feat: 검색 결과가 없을 경우 결과 없음 안내 페이지 렌더링

17a4af6

fix: 페이지 초기화 문제 해결

eb55b7c

refactor: any 타입 제거

1efa111

refactor: movie API와 movieStore 분리

e0a59bb

함수명 변경, 전체적인 구조 변경

feat: 스켈레톤 로딩 기능 추가 및 빈 이미지일 경우 대체 이미지 추가

95c26c3

+ 결과 없음 페이지 css 수정

test: e2e 테스트 진행

beea5fc

docs: README 작성

f639cfd

chore: 필요 없는 파일 삭제

2e96a14

pocojang self-assigned this Mar 16, 2023

pocojang self-requested a review March 16, 2023 09:00

pocojang requested changes Mar 18, 2023

View reviewed changes

feb-dain added 9 commits March 19, 2023 18:44

comment: 불필요한 주석 삭제

3bc416d

refactor: I- prefix 삭제 및 타입 선언 방식 변경

d85a393

- Type assertion 제거 - `[]` 대신 `Array<>` 사용

style: css 수정 및 a 태그 삭제

cf49660

remove: 이벤트 리스너 유틸 삭제"

ae1e790

refactor: movieApi 필드 camelCase로 변경

58c0c35

refactor: new URL, new URLSearchParams 사용 및 상수화

46d29a8

refactor: 함수명 변경

4834c40

refactor: 역할 분리 및 에러 핸들러 개선

623f7d6

+ 함수명/타입명 등 변경

test: 테스트 코드 수정 및 추가

cd080ed

- API가 성공적으로 응답하는지 테스트 - 네트워크 요청을 가로챈 상태에서 테스트

pocojang approved these changes Mar 20, 2023

View reviewed changes

pocojang merged commit 1d2bbc3 into woowacourse:feb-dain Mar 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 영화 목록 불러오기] 야미(이다인) 미션 제출합니다. #5

[1단계 - 영화 목록 불러오기] 야미(이다인) 미션 제출합니다. #5

feb-dain commented Mar 16, 2023 •

edited

Loading

woowahan-cron commented Mar 16, 2023

pocojang left a comment

pocojang Mar 18, 2023

pocojang Mar 18, 2023

pocojang Mar 18, 2023

pocojang Mar 18, 2023

feb-dain Mar 19, 2023 •

edited

Loading

feb-dain Mar 20, 2023

pocojang Mar 20, 2023

pocojang Mar 18, 2023

feb-dain Mar 19, 2023 •

edited

Loading

pocojang Mar 18, 2023

feb-dain Mar 20, 2023

pocojang Mar 18, 2023

feb-dain Mar 20, 2023 •

edited

Loading

pocojang Mar 18, 2023

feb-dain Mar 20, 2023

pocojang Mar 18, 2023

feb-dain Mar 20, 2023

pocojang Mar 18, 2023

feb-dain Mar 20, 2023

feb-dain commented Mar 20, 2023 •

edited

Loading

pocojang left a comment

feb-dain commented Mar 20, 2023 •

edited

Loading

		<html lang="en">
		<html lang="ko">

[1단계 - 영화 목록 불러오기] 야미(이다인) 미션 제출합니다. #5

[1단계 - 영화 목록 불러오기] 야미(이다인) 미션 제출합니다. #5

Conversation

feb-dain commented Mar 16, 2023 • edited Loading

구조

웹 컴포넌트 (customElement 사용)

🤓 노력한 점

🥲 아쉬운 점

woowahan-cron commented Mar 16, 2023

pocojang left a comment

Choose a reason for hiding this comment

피드백

생성자

구조

이벤트

Web Components

기타

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feb-dain Mar 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feb-dain Mar 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feb-dain Mar 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feb-dain commented Mar 20, 2023 • edited Loading

pocojang left a comment

Choose a reason for hiding this comment

피드백

비동기

구조

intercept� 질문

사용성

feb-dain commented Mar 20, 2023 • edited Loading

feb-dain commented Mar 16, 2023 •

edited

Loading

feb-dain Mar 19, 2023 •

edited

Loading

feb-dain Mar 19, 2023 •

edited

Loading

feb-dain Mar 20, 2023 •

edited

Loading

feb-dain commented Mar 20, 2023 •

edited

Loading

feb-dain commented Mar 20, 2023 •

edited

Loading