Skip to content

[2단계 - 상세 정보 & UI/UX 개선하기] 해니 미션 제출합니다.#315

Open
janghw0126 wants to merge 80 commits intowoowacourse:janghw0126from
janghw0126:step2
Open

[2단계 - 상세 정보 & UI/UX 개선하기] 해니 미션 제출합니다.#315
janghw0126 wants to merge 80 commits intowoowacourse:janghw0126from
janghw0126:step2

Conversation

@janghw0126
Copy link
Copy Markdown
Member

@janghw0126 janghw0126 commented Apr 13, 2026

🎯 미션 소개

  • API 연동을 통한 비동기 통신 학습
  • 사용자 시나리오 기반 E2E 테스트

🕵️ 셀프 리뷰(Self-Review)

제출 전 체크 리스트

  • 기능 요구 사항을 모두 구현했고, 정상적으로 동작하는지 확인했나요?
  • 기본적인 프로그래밍 요구 사항(코드 컨벤션, 에러 핸들링 등)을 준수했나요?
  • 테스트 코드는 모두 정상적으로 실행되나요? (필요 시, API Mocking 등)
  • (해당하는 경우) 배포한 데모 페이지에 정상적으로 접근할 수 있나요?

리뷰 요청 & 논의하고 싶은 내용

안녕하세요, 수야! 이번에도 step-2 리뷰를 맡아주셔서 감사합니다. 😊
step-1에서 꼼꼼하게 리뷰해 주신 덕분에 리팩토링하면서 정말 많이 고민하고 성장할 수 있었어요. 이번에도 잘 부탁드려요!

1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점

  1. 단일 책임 원칙을 최대한 지키고 싶어서, 각각의 코드와 함수가 하나의 일만 하도록 구조에 대한 고민을 많이 했어요. step-1에서 리팩토링하고 고민하는 데 시간이 많이 걸렸는데, 그 덕분인지 step-2에서는 코드를 많이 수정하지 않고도 자연스럽게 작성할 수 있었습니다.

  2. 무한 스크롤을 어떻게 구현할지 많이 고민했어요. 처음에 생각한 방법은 두 가지였는데요, 조금 당연하고 조금은 멍청한? ㅎㅎ 고민이었을 수도 있어요! 하나는 화면 끝에 닿을 때마다 20개씩 불러와서 렌더링하는 방식이고, 나머지는 영화 목록을 전부 다 불러온 뒤 스크롤에 따라 화면에 보여주는 방식이었습니다. 트레이드오프를 생각해보니, 전체 데이터를 선 로딩하면 스크롤이 매끄럽고 네트워크 요청이 초기에만 발생하는 장점이 있었어요. 하지만 인기 영화는 수백~수천 페이지가 존재할 텐데, 초기 로딩이 느리고 메모리를 많이 사용하게 되고, 사용자가 보지 않을 데이터까지 미리 받아오는 게 비효율적이라는 생각이 들었습니다. 그래서 스크롤 시 API를 요청하는 방식을 선택했어요.

  3. 무한 스크롤 구현 방법으로 scroll 이벤트와 Intersection Observer 중 어떤 걸 써야 할지 고민했어요. scroll + debounce 방식은 스크롤할 때마다 수백 번 실행되어 성능이 떨어진다는 걸 알게 됐고, Intersection Observer는 스크롤 이벤트를 직접 듣지 않고 브라우저가 요소의 가시성 변화를 감지할 때 콜백을 실행하는 방식이라 더 효율적일 것 같아 이 방법을 채택했습니다.

  4. localStorage 구현 방식과 안전하게 갈아끼울 수 있는 구조에 대해서도 많이 고민했는데요, 궁금한 점이 있어서 아래 2)번 질문에 함께 남겨두겠습니다!

2) 이번 리뷰를 통해 논의하고 싶은 부분

질문이 조금 길어요.. 이번 미션을 하면서 고민이 많았거든요!

  1. 이번 미션의 목표를 제대로 잡지 못했어요. 레벨 1이 끝나가면서 체력적으로도 지쳤고, 미션을 통해 얻어가고 싶은 게 뭔지에 대한 고민도 깊어졌습니다. 지금까지 확실하게 얻은 게 있는지 불확실한 느낌에 방황하기도 했고요. 그러다가 2일 전에 정신을 차리고 어영부영 구현을 시작했는데, 돌이켜보면 미션마다 목표를 제대로 못 잡아서 그랬던 것 같아요. 그런데 저는 우테코를 통해 프론트를 처음 접하는 상황이에요. 뭘 얻어가야 할지조차 잘 모르는 상태에서 목표를 어떻게 명확하게 정할 수 있을까요? 수야는 이런 상황에서 목표를 어떻게 잡으시나요?

  2. 제 코드를 다시 보면, 처음부터 끝까지 혼자 짤 수 있을까 싶은 느낌이 들어요. 설계에 대한 고민은 많이 했지만, 모르는 부분은 AI의 도움을 많이 받았거든요. 저는 웹을 거의 2년 전에 공부하고 손을 놨기에 사실상 처음이나 다름없어요. 그래서 ai가 내용을 알려준 것이지 제가 스스로 습득한 게 아닌 것 같다는 생각이 들었습니다. 아직 몸에 해당 지식들이 돌아다니지 않는 느낌이랄까요.. 이럴 때 어떻게 해야 제 지식으로 체화할 수 있을까요? 그냥 반복이 부족한 건지, 머리로 아는 것과 손이 아는 것 사이의 간격이 아직 좁혀지지 않는데 어떻게 해결할 수 있을까요?

  3. 보여지는 것에 집착하는 것 같아요. "남들한테 이렇게 보이겠지?" 하면서 정작 제가 얻어가야 할 것보다 남을 위해 보여주려고 코드를 짜는 느낌이 듭니다. 그러다 보니 체화가 잘 안 되는 것 같기도 하고요. 어떻게 해야 제 학습에 집중할 수 있을까요? 다른 크루들과 비교하지 않고 자신만의 길을 가는 방법이 궁금해요. 레벨 1이 끝나면서 이 고민이 해결되어야 레벨 2에서 더 성장할 수 있을 것 같아 여쭤봅니다.

  4. localStorage 구현 방식에 대해 많이 고민했어요. 이번에는 영화마다 고유한 키를 만들어 별점을 저장하고, 모달이 열릴 때 해당 키로 localStorage를 조회해서 저장된 값이 있으면 별 UI를 복원하는 방식으로 구현했습니다. 사용자가 별을 클릭하면 즉시 덮어써서 새로고침 후에도 유지되게 했고요. 그런데 요구사항에 "서버 API로 교체할 때 안전하게 갈아끼울 수 있는 방법을 고민하고 적용하라"는 내용이 있었어요. 현재는 모달 내부에 localStorage 로직이 직접 들어가 있는데, 서버 API로 교체할 때 모달 코드를 수정하지 않아도 되는 구조가 어떤 건지 궁금합니다. 저장소를 추상화하는 레이어를 두는 방식인가요? 감이 잘 잡히지 않아 여쭤봐요..!!

코드를 구현하면서 생긴 궁금한 점은 아래 코멘트로 남겨두었어요!


✅ 리뷰어 체크 포인트

  • 비동기 통신 과정에서 발생할 수 있는 예외(네트워크, 서버 오류 등)를 고려했는가?
  • 비동기 로직에서 콜백 지옥 없이, 적절히 async/await 또는 Promise를 활용했는가?
  • 역할과 책임에 따라 파일/모듈을 분리했는가? (UI, 비즈니스 로직, API 호출 등)

DongEun02 and others added 30 commits March 31, 2026 15:37
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: less <lejin0906@gmail.com>
Co-authored-by: Henny <janghw232625@gmail.com>
Co-authored-by: less <lejin0906@gmail.com>
Co-authored-by: Henny <janghw232625@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
Co-authored-by: janghw0126 <janghw232625@gmail.com>
Co-authored-by: lee-eojin <lejin0906@gmail.com>
- render() 호출 시 생성되는 .close-modal 버튼에 직접 click 리스너를 연결.
- 기존에는 document.body 이벤트 위임에만 의존했으나, 버블링 과정의 불확실성으로 닫기 버튼이 동작하지 않는 문제가 있었음.
- MovieList에서 updateMoreButton 메서드 제거
- movieState에 isLoading, hasMore 상태 추가
- scroll-sentinel 요소를 활용해 화면 끝 도달 시 다음 페이지 자동 로드
- isLoading, hasMore 상태로 중복 요청 방지 및 마지막 페이지 감지
- 마지막 페이지 도달 시 observer.disconnect()로 감시 종료
- rootMargin 200px 설정으로 바닥 도달 전 미리 로드
- main.css: #wrap min-width 제거, .container 패딩 추가, 헤더·로고·검색창·타이틀 브레이크포인트별 크기/위치 조정
- thumbnail.css: 썸네일 고정 크기 → width: 100% / aspect-ratio 유동 크기로 변경, 그리드 컬럼 수 브레이크포인트 정비
- modal.css: 모달 너비 min() 함수로 반응형 처리, 태블릿(1024px)·모바일(800px)에서 세로 스택 레이아웃 적용
- 태블릿: 모달을 화면 하단 고정, 세로 정렬
- 모바일: 포스터 이미지 숨김, 전체 요소 세로/중앙 정렬
- 모바일: 별점 영역 세로 배치를 위해 stars-row/rating-text 래퍼 추가
- 모바일: 닫기 버튼과 제목 겹침 수정
- 모달 열기(포스터/제목 클릭), 닫기(버튼/ESC) 테스트 추가
- 별점 부여 후 새로고침 시 유지 여부 테스트 추가
- 더보기 버튼 → 무한 스크롤 방식으로 테스트 변경
- beforeEach에 검색 입력 활성화 확인 추가
- MovieList에 appendSkeletons / removeSkeletons 메서드 추가
- renderMoreMovies에서 API 호출 전후로 스켈레톤을 붙이고 제거 (에러 시에도 제거)
- 스켈레톤 shimmer 애니메이션 CSS 추가
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 13, 2026

전체 요약 (Walkthrough)

이 풀 리퀘스트는 영화 리뷰 애플리케이션에 영화 상세 정보 모달, 무한 스크롤, 별점 평가 기능을 추가하는 변경사항을 포함합니다. 새로운 MovieDetailModal 컴포넌트는 포스터, 제목, 장르, 평점, 줄거리를 표시하며 localStorage에 사용자 평점을 저장합니다. IntersectionObserver를 활용한 무한 스크롤이 기존의 "더 보기" 버튼을 대체합니다. 상태 관리 체계가 movieStateisLoadinghasMore 필드를 추가하여 확장되었으며, 전체 UI 레이아웃과 반응형 디자인이 개선되었습니다. E2E 테스트도 모달 열기/닫기, 별점 지속성, 무한 스크롤 동작을 검증하도록 업데이트되었습니다.

코드 리뷰 예상 소요 시간

🎯 4 (Complex) | ⏱️ ~45 분

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed 제목은 한국어로 작성되었으며 '2단계 - 상세 정보 & UI/UX 개선하기'라는 구체적인 미션 단계와 주요 변경 사항을 명확하게 나타냅니다. 변경 세트의 주요 특징(모달, 별점 시스템, 무한 스크롤, 반응형 개선)과 잘 부합합니다.
Description check ✅ Passed PR 설명이 템플릿 구조를 충실히 따르고 있으며, 셀프 리뷰 체크리스트 항목들을 모두 체크했고 구체적인 구현 고민 과정과 논의 사항이 상세히 기술되어 있습니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (3)
src/features/View/MovieList.ts (1)

41-52: 스켈레톤 생성 경로를 하나로 모아두는 편이 안전합니다.

지금은 appendSkeletons().skeleton-item을 붙이고 있어서, 정리 규칙이 renderSkeleton()과 갈립니다. renderSkeleton()도 같은 경로를 타게 만들지 먼저 점검해보세요. 이런 종류는 나중에 empty/error 경로에서 제거 로직이 다시 분기되기 쉽습니다. As per coding guidelines, "Promote modular thinking—breaking problems into reusable components."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/View/MovieList.ts` around lines 41 - 52, The skeleton element
class is applied only in appendSkeletons, splitting creation and cleanup logic;
centralize skeleton creation by moving the "skeleton-item" class assignment into
the MovieSkeleton.render() (or a new helper like
renderSkeletonElement/createSkeleton) so both appendSkeletons and any other
caller use the same creation path, then update removeSkeletons to rely on the
same ".skeleton-item" selector; reference MovieSkeleton.render(),
appendSkeletons(), and removeSkeletons() when making this change.
src/styles/modal.css (1)

32-35: 모달 전용 스타일이라면 hr 범위를 좁히는 편이 안전합니다.

전역 hr를 덮어쓰면 다른 화면의 구분선까지 같이 바뀝니다. .modal hr처럼 스코프를 제한해 두면 사이드 이펙트를 줄일 수 있습니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/styles/modal.css` around lines 32 - 35, 현재 전역 hr 규칙이 모든 구분선을 변경하고 있어 사이드
이펙트가 발생할 수 있습니다; 범위를 제한하려면 전역 hr 대신 모달 컴포넌트의 루트 클래스(예: .modal 또는
.modal-content)를 접두사로 사용해 규칙을 스코프하세요(참고: 현재 대상 선택자명은 hr, 변경할 위치는
src/styles/modal.css의 hr 규칙).
cypress/e2e/spec.cy.ts (1)

21-35: 배경 클릭 닫기 시나리오도 같이 묶어두면 좋겠습니다.

README-step2.md Line 22-25에는 backdrop 클릭으로 모달을 닫는 요구사항이 있는데, 현재 E2E는 닫기 버튼과 ESC만 확인합니다. 이 케이스 하나만 더 있으면 회귀를 막기 쉬워집니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cypress/e2e/spec.cy.ts` around lines 21 - 35, The test suite is missing a
scenario that verifies clicking the modal backdrop closes the modal as required;
add a new test in cypress/e2e/spec.cy.ts (alongside the existing tests "닫기 버튼 클릭
시 모달이 닫힌다" and "ESC 키 입력 시 모달이 닫힌다") that opens the modal by clicking the first
".thumbnail" (use cy.get(".thumbnail-list
li").first().find(".thumbnail").click()), asserts ".modal-background" has class
"active", then simulates a backdrop click (cy.get(".modal-background").click()
or click a specific backdrop element if present) and finally asserts
".modal-background" does not have class "active" to cover the backdrop-close
requirement from README-step2.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cypress/e2e/spec.cy.ts`:
- Around line 38-53: The test opens the first movie card then reloads but
re-opens whatever is first after reload, which can be a different movie; capture
the initially opened movie's identifier (e.g., read the first `.thumbnail-list
li`'s `data-id` or the clicked `.thumbnail`'s `data-id` into a variable/alias)
before clicking or reloading, then after cy.reload() locate and click the same
movie by selecting the element with that `data-id` (e.g.,
`[data-id="${savedId}"]`) instead of blindly clicking the first `.thumbnail-list
li`, and then assert `.rating-label` and `.rating-score` as before to ensure you
reopened the same movie.

In `@src/features/eventHandler.ts`:
- Around line 58-60: The current logic increments movieState.page before calling
renderMoreMovies, which will skip a page if the fetch fails; change the flow so
the page is only incremented on successful load: call
renderMoreMovies(movieState.page + 1, movieState.searchQuery) or let
renderMoreMovies return a success promise and increment movieState.page inside
the success path (and set movieState.isLoading appropriately), and on failure
roll back any tentative increment or avoid changing movieState.page; update the
block that checks entry.isIntersecting / movieState.isLoading /
movieState.hasMore and adjust where movieState.page is mutated so it only
changes after a successful renderMoreMovies completion.
- Around line 63-64: 현재 movieState.hasMore가 false일 때 즉시 observer.disconnect()하면
이후 탐색이나 새 검색 시에도 무한스크롤이 재등록되지 않으므로 observer.disconnect()를 이 위치에서 제거하거나 상태만 검사하도록
변경하세요: 대신에 콜백 내에서 if (!movieState.hasMore) return; 로 추가해 더 이상의 요청을 막고 sentinel
관찰은 유지하거나, 만약 disconnect()를 유지하려면 검색/홈 전환 시 상태 초기화와 함께 loadInfiniteScroll()를 다시
호출하도록 리셋 로직(예: resetMovieState() 후 loadInfiniteScroll())을 구현해 재등록이 보장되게 만드세요; 관련
식별자: movieState.hasMore, observer.disconnect(), loadInfiniteScroll(), sentinel.

In `@src/features/movieController.ts`:
- Around line 77-83: The renderMovieDetailModal function can be overwritten by
slower earlier responses; add a request-scoping guard: either track a
module-scoped lastRequestedId (set it to id at start and only call
movieDetailModal.render(data) if id === lastRequestedId) or use an
AbortController with getMovieDetail to cancel previous fetches before starting a
new one; update renderMovieDetailModal to set/reset the guard or abort previous
controller before awaiting getMovieDetail, and only call
movieDetailModal.render(data) when the response corresponds to the latest
request.

In `@src/features/movieModel.ts`:
- Around line 41-45: The getMovieDetail function currently uses the boxed type
`Number` and lacks an explicit return type; change the parameter from `id:
Number` to the primitive `id: number` and annotate the function to return a
Promise of the MovieDetail type (e.g., Promise<MovieDetail>), and ensure the
JSON response is typed as MovieDetail (cast or type the result of
response.json()) so the function signature and returned value use the
MovieDetail type throughout.

In `@src/features/movieState.ts`:
- Around line 4-10: The current state (isLoading, page, searchQuery, hasMore)
can't prevent out-of-order responses from overwriting newer results — add a
request identity to the state (e.g., activeRequestId or activeAbortController)
and update reset() to increment/replace it; when starting a fetch in the
controller (functions in movieController.ts such as the fetch/load handler),
generate a new id or AbortController, store it on the state, attach it to the
network call, and on response only commit results if the response's id matches
state.activeRequestId (or if the signal wasn't aborted); also ensure previous
in-flight requests are aborted or ignored when resetting/searching to prevent
stale updates.

In `@src/features/View/MovieDetailModal.ts`:
- Around line 35-69: The modal currently injects untrusted fields (title,
genres, overview, release_date, poster_path) directly into innerHTML in
MovieDetailModal which opens XSS; refactor to build the DOM with
document.createElement calls (or a safe templating/sanitizer) instead of using
innerHTML: create the modal container, image element (set src from
THUMB_NAIL_URL + data.poster_path via setAttribute), create text nodes and
assign textContent for title (.modal-title), release date/genres
(.modal-release-date-and-genres) by joining genre names safely, overview
(.modal-overview), and other text fields; for dynamic stars generate img
elements with class "modal-star" and set dataset.index and src attributes
programmatically; ensure any HTML attributes use safe escaping and do not
concatenate untrusted strings into raw HTML.
- Around line 52-63: The star widgets are non-interactive images and
inaccessible to keyboard/screen-reader users; replace the <img
class="modal-star"> elements in the "star-rating"/"stars-row" with interactive
controls (preferably real radio buttons or button elements acting as radio role)
so they can receive focus and be activated via Enter/Space, give the container
role="radiogroup" and each control role="radio" (or use native input
type="radio") with aria-checked and an accessible name (e.g., aria-label="1
star", "2 stars") derived from data-index, add keyboard handlers to toggle
selection on Enter/Space and update the "rating-label" and "rating-score" text
as well as the aria-checked states, and ensure visible focus styles; apply the
same changes to the other instance referenced (lines ~80-95).

In `@src/styles/main.css`:
- Around line 134-138: The .search-form rule inside the media query uses top:
36px but lacks a position value so the offset has no effect; update the
.search-form selector in that media query (or globally) to include an
appropriate position (e.g., position: relative or position: absolute depending
on desired behavior) so the top: 36px offset is applied, and ensure this change
aligns with surrounding layout (choose relative if you want it offset within its
normal flow, absolute if you want it positioned against a containing block).

In `@src/styles/modal.css`:
- Around line 225-231: The CSS assumes a container with an inner <img> (uses
rules like .modal-image img) but the MovieDetailModal component renders a single
<img class="modal-image">; update the styles in modal.css (both the block at the
first diff and the later block around lines 296-301) to target the actual
element selector .modal-image (remove or duplicate rules that target
.modal-image img) so all size, border-radius and responsive rules apply to the
rendered <img class="modal-image">; alternatively, if you prefer the container
pattern, change MovieDetailModal to render a wrapper (e.g., <div
class="modal-image"><img /></div>) consistently — but ensure selector and markup
match (.modal-image or .modal-image img) across both CSS blocks.

---

Nitpick comments:
In `@cypress/e2e/spec.cy.ts`:
- Around line 21-35: The test suite is missing a scenario that verifies clicking
the modal backdrop closes the modal as required; add a new test in
cypress/e2e/spec.cy.ts (alongside the existing tests "닫기 버튼 클릭 시 모달이 닫힌다" and
"ESC 키 입력 시 모달이 닫힌다") that opens the modal by clicking the first ".thumbnail"
(use cy.get(".thumbnail-list li").first().find(".thumbnail").click()), asserts
".modal-background" has class "active", then simulates a backdrop click
(cy.get(".modal-background").click() or click a specific backdrop element if
present) and finally asserts ".modal-background" does not have class "active" to
cover the backdrop-close requirement from README-step2.md.

In `@src/features/View/MovieList.ts`:
- Around line 41-52: The skeleton element class is applied only in
appendSkeletons, splitting creation and cleanup logic; centralize skeleton
creation by moving the "skeleton-item" class assignment into the
MovieSkeleton.render() (or a new helper like
renderSkeletonElement/createSkeleton) so both appendSkeletons and any other
caller use the same creation path, then update removeSkeletons to rely on the
same ".skeleton-item" selector; reference MovieSkeleton.render(),
appendSkeletons(), and removeSkeletons() when making this change.

In `@src/styles/modal.css`:
- Around line 32-35: 현재 전역 hr 규칙이 모든 구분선을 변경하고 있어 사이드 이펙트가 발생할 수 있습니다; 범위를 제한하려면
전역 hr 대신 모달 컴포넌트의 루트 클래스(예: .modal 또는 .modal-content)를 접두사로 사용해 규칙을 스코프하세요(참고:
현재 대상 선택자명은 hr, 변경할 위치는 src/styles/modal.css의 hr 규칙).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: aa40a346-a9b8-42f2-b765-248bd96fb472

📥 Commits

Reviewing files that changed from the base of the PR and between b27966b and e569dcc.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (16)
  • README-step1.md
  • README-step2.md
  • cypress/e2e/spec.cy.ts
  • index.html
  • src/features/View/Header.ts
  • src/features/View/MovieCard.ts
  • src/features/View/MovieDetailModal.ts
  • src/features/View/MovieList.ts
  • src/features/eventHandler.ts
  • src/features/movieController.ts
  • src/features/movieModel.ts
  • src/features/movieState.ts
  • src/styles/main.css
  • src/styles/modal.css
  • src/styles/thumbnail.css
  • types/types.ts

Comment on lines +38 to +53
it("별점 부여 후 새로고침 시 별점이 유지된다", () => {
cy.get(".thumbnail-list li").first().find(".thumbnail").click();
cy.get(".modal-background").should("have.class", "active");

cy.get(".modal-star[data-index='4']").click();
cy.get(".rating-label").should("have.text", "재미있어요");
cy.get(".rating-score").should("have.text", "(8/10)");

cy.get(".close-modal").click();
cy.reload();
cy.get(".thumbnail-list li").should("have.length.greaterThan", 0);

cy.get(".thumbnail-list li").first().find(".thumbnail").click();
cy.get(".modal-background").should("have.class", "active");
cy.get(".rating-label").should("have.text", "재미있어요");
cy.get(".rating-score").should("have.text", "(8/10)");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

새로고침 뒤에 같은 영화를 다시 열었다는 보장이 없습니다.

평점은 movieId로 저장되는데, 검증은 다시 “첫 번째 카드”를 여는 방식이라 인기순 응답 순서가 바뀌면 기능이 정상이어도 테스트가 깨집니다. 처음 선택한 카드의 data-id를 저장해서 같은 영화를 다시 열도록 맞춰 두면 훨씬 안정적입니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cypress/e2e/spec.cy.ts` around lines 38 - 53, The test opens the first movie
card then reloads but re-opens whatever is first after reload, which can be a
different movie; capture the initially opened movie's identifier (e.g., read the
first `.thumbnail-list li`'s `data-id` or the clicked `.thumbnail`'s `data-id`
into a variable/alias) before clicking or reloading, then after cy.reload()
locate and click the same movie by selecting the element with that `data-id`
(e.g., `[data-id="${savedId}"]`) instead of blindly clicking the first
`.thumbnail-list li`, and then assert `.rating-label` and `.rating-score` as
before to ensure you reopened the same movie.

Comment on lines +58 to +60
if (entry.isIntersecting && !movieState.isLoading && movieState.hasMore) {
movieState.page += 1;
renderMoreMovies(movieState.page, movieState.searchQuery);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

페이지 번호를 먼저 올리면 실패한 페이지를 건너뜁니다.

지금은 요청 전에 movieState.page += 1을 해버려서, renderMoreMovies()가 실패해도 상태가 롤백되지 않습니다. 네트워크 오류 한 번이면 다음 스크롤에서 page 3을 요청하게 되어 page 2가 비게 됩니다. 페이지 증가는 성공 시점으로 옮기거나 실패 복구 흐름이 필요합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/eventHandler.ts` around lines 58 - 60, The current logic
increments movieState.page before calling renderMoreMovies, which will skip a
page if the fetch fails; change the flow so the page is only incremented on
successful load: call renderMoreMovies(movieState.page + 1,
movieState.searchQuery) or let renderMoreMovies return a success promise and
increment movieState.page inside the success path (and set movieState.isLoading
appropriately), and on failure roll back any tentative increment or avoid
changing movieState.page; update the block that checks entry.isIntersecting /
movieState.isLoading / movieState.hasMore and adjust where movieState.page is
mutated so it only changes after a successful renderMoreMovies completion.

Comment on lines +63 to +64
// 마지막 페이지면 관찰 중단
if (!movieState.hasMore) observer.disconnect();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

여기서 observer를 끊으면 이후 목록에서도 무한 스크롤이 살아나지 않습니다.

한 번 hasMore = false가 된 뒤 disconnect()되면, 홈으로 돌아오거나 새 검색을 해도 loadInfiniteScroll()가 다시 실행되지 않아 sentinel을 더 이상 감시하지 않습니다. 이 위치에서는 관찰을 유지하고 상태만 가드할지, 아니면 상태 초기화와 함께 재등록할지 한 번 정리해 보세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/eventHandler.ts` around lines 63 - 64, 현재 movieState.hasMore가
false일 때 즉시 observer.disconnect()하면 이후 탐색이나 새 검색 시에도 무한스크롤이 재등록되지 않으므로
observer.disconnect()를 이 위치에서 제거하거나 상태만 검사하도록 변경하세요: 대신에 콜백 내에서 if
(!movieState.hasMore) return; 로 추가해 더 이상의 요청을 막고 sentinel 관찰은 유지하거나, 만약
disconnect()를 유지하려면 검색/홈 전환 시 상태 초기화와 함께 loadInfiniteScroll()를 다시 호출하도록 리셋 로직(예:
resetMovieState() 후 loadInfiniteScroll())을 구현해 재등록이 보장되게 만드세요; 관련 식별자:
movieState.hasMore, observer.disconnect(), loadInfiniteScroll(), sentinel.

Comment on lines +77 to +83
export async function renderMovieDetailModal(id: number) {
try {
movieDetailModal.reset();
// 영화 상세 정보 API 요청
const data = await getMovieDetail(id);
// 영화 상세 정보 모달 렌더링 함수 호출
movieDetailModal.render(data);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

연속 클릭 시 늦게 온 이전 응답이 최신 모달을 덮어쓸 수 있습니다.

상세 요청이 겹치면 마지막으로 클릭한 영화가 아니라, 가장 늦게 끝난 응답이 render()될 수 있습니다. 이 구간은 “마지막으로 요청한 id만 반영한다”는 가드나 AbortController 같은 취소 흐름을 두는 편이 안전합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/movieController.ts` around lines 77 - 83, The
renderMovieDetailModal function can be overwritten by slower earlier responses;
add a request-scoping guard: either track a module-scoped lastRequestedId (set
it to id at start and only call movieDetailModal.render(data) if id ===
lastRequestedId) or use an AbortController with getMovieDetail to cancel
previous fetches before starting a new one; update renderMovieDetailModal to
set/reset the guard or abort previous controller before awaiting getMovieDetail,
and only call movieDetailModal.render(data) when the response corresponds to the
latest request.

Comment on lines +41 to +45
export async function getMovieDetail(id: Number){
try{
const response = await fetch(`${BASE_URL}/movie/${id}?api_key=${API_KEY}&language=ko-KR`);
if (!response.ok) throw new Error(`API 요청 실패: ${response.status}`);
return response.json();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

TypeScript에서 함수 파라미터 타입으로 Number대신number를 쓰는 이유는 무엇인가요? Number객체 타입과number 원시 타입의 차이도 함께 확인해주세요.

💡 Result:

TypeScript에서 함수 파라미터 타입으로 Number 대신 number를 사용하는 이유는 number가 JavaScript의 원시(primitve) 숫자 타입을 나타내기 때문입니다. Number는 원시 값을 래핑하는 객체 타입으로, JavaScript에서 거의 사용되지 않으며 TypeScript에서도 이를 권장하지 않습니다. 공식 TypeScript 문서와 핸드북에서 명시적으로 number, string, boolean 등의 소문자 원시 타입을 사용하라고 안내하며, 대문자 Number 등의 타입은 "non-primitive boxed objects"로 거의 적절히 사용되지 않는다고 경고합니다. Number 객체 타입과 number 원시 타입의 차이: - number: JavaScript의 기본 숫자 원시 타입 (예: 42). 모든 숫자 연산이 효율적으로 동작하며, 런타임에서 primitive 값입니다. - Number: new Number(42)로 생성되는 객체 인스턴스. typeof가 "object"로 반환되며, primitive와 다르게 동작할 수 있습니다 (예: === 비교 실패). TypeScript에서 number는 Number로 암시적 변환 가능하지만, 반대는 불가능하며, 객체 메서드(toFixed 등) 접근 시 문제 발생 가능. 함수 파라미터에서 number를 사용하면 primitive 숫자만 예상하고 타입 체크가 엄격해지며, 대부분의 JS/TS 코드와 호환됩니다. Number를 사용하면 불필요한 객체 래퍼링으로 인한 성능 저하와 버그 위험이 있습니다.

Citations:


함수 파라미터와 반환 타입을 명시적으로 지정하세요.

id: Number 대신 원시 타입 number를 사용하고, 반환 타입을 MovieDetail로 명시하세요. TypeScript 공식 문서에서는 Number 같은 박스형 타입 대신 소문자 원시 타입(number, string, boolean)을 사용하도록 명확히 권장하며, 이렇게 하면 방금 추가한 MovieDetail 타입의 이점을 이 함수 경계에서도 활용할 수 있고 컴파일 단계에서 타입 불일치를 더 빠르게 감지할 수 있습니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/movieModel.ts` around lines 41 - 45, The getMovieDetail function
currently uses the boxed type `Number` and lacks an explicit return type; change
the parameter from `id: Number` to the primitive `id: number` and annotate the
function to return a Promise of the MovieDetail type (e.g.,
Promise<MovieDetail>), and ensure the JSON response is typed as MovieDetail
(cast or type the result of response.json()) so the function signature and
returned value use the MovieDetail type throughout.

Comment on lines +4 to +10
isLoading: false,
hasMore: true,
reset() {
this.page = 1;
this.searchQuery = "";
this.isLoading = false;
this.hasMore = true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

이 상태만으로는 오래된 응답이 최신 화면을 덮어쓸 수 있습니다.

isLoading은 중복 스크롤 요청은 막아주지만, 서로 다른 검색/초기화 요청 간의 완료 순서 역전은 못 막습니다. 지금 src/features/movieController.ts:32-74 흐름이면 이전 요청이 늦게 돌아왔을 때 최신 결과를 덮어쓸 수 있어서, “현재 유효한 요청”을 식별하거나 취소할 수단을 상태에 둘지 먼저 정해보는 게 좋겠습니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/movieState.ts` around lines 4 - 10, The current state
(isLoading, page, searchQuery, hasMore) can't prevent out-of-order responses
from overwriting newer results — add a request identity to the state (e.g.,
activeRequestId or activeAbortController) and update reset() to
increment/replace it; when starting a fetch in the controller (functions in
movieController.ts such as the fetch/load handler), generate a new id or
AbortController, store it on the state, attach it to the network call, and on
response only commit results if the response's id matches state.activeRequestId
(or if the signal wasn't aborted); also ensure previous in-flight requests are
aborted or ignored when resetting/searching to prevent stale updates.

Comment on lines +35 to +69
this.div.className = "modal-background";
this.div.innerHTML = /*html*/ `
<div class="modal">
<div class="modal-container">
<img class="modal-image" src="${THUMB_NAIL_URL}${data.poster_path}" alt="${data.title}">
<div class="modal-description">
<button class="close-modal">
<img src="${closeImg}" alt="닫기">
</button>
<h2 class="modal-title">${data.title}</h2>
<p class="modal-release-date-and-genres">${data.release_date.slice(0, 4)} · ${data.genres.map((genre) => genre.name).join(", ")}</p>
<div class="average">
<p class="modal-rating-text">평균</p>
<img class="average-star" src="${starFilledImg}">
<p class="modal-rating">${data.vote_average.toFixed(1)}</p>
</div>
<hr>
<div class="modal-user-rating">
<p class="modal-user-rating-text">내 별점</p>
<div class="star-rating">
<div class="stars-row">
${[1, 2, 3, 4, 5].map((i) => `<img class="modal-star" data-index="${i}" src="${starEmptyImg}">`).join("")}
</div>
<div class="rating-text">
<span class="rating-label"></span>
<span class="rating-score"></span>
</div>
</div>
</div>
<hr>
<p class="modal-overview-title">줄거리</p>
<p class="modal-overview">${data.overview}</p>
</div>
</div>
</div>`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

외부 API 문자열을 innerHTML로 바로 주입하고 있습니다.

title, genres, overview는 신뢰되지 않은 데이터인데 그대로 HTML에 삽입되어 XSS 경로가 열립니다. 이 구간은 DOM 노드를 만들고 textContent로 채우거나, 최소한 sanitize 단계를 둔 뒤 렌더링하는 편이 안전합니다.

🧰 Tools
🪛 ast-grep (0.42.1)

[warning] 35-68: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: this.div.innerHTML = /html/ <div class="modal"> <div class="modal-container"> <img class="modal-image" src="${THUMB_NAIL_URL}${data.poster_path}" alt="${data.title}"> <div class="modal-description"> <button class="close-modal"> <img src="${closeImg}" alt="닫기"> </button> <h2 class="modal-title">${data.title}</h2> <p class="modal-release-date-and-genres">${data.release_date.slice(0, 4)} · ${data.genres.map((genre) => genre.name).join(", ")}</p> <div class="average"> <p class="modal-rating-text">평균</p> <img class="average-star" src="${starFilledImg}"> <p class="modal-rating">${data.vote_average.toFixed(1)}</p> </div> <hr> <div class="modal-user-rating"> <p class="modal-user-rating-text">내 별점</p> <div class="star-rating"> <div class="stars-row"> ${[1, 2, 3, 4, 5].map((i) =>).join("")} </div> <div class="rating-text"> <span class="rating-label"></span> <span class="rating-score"></span> </div> </div> </div> <hr> <p class="modal-overview-title">줄거리</p> <p class="modal-overview">${data.overview}</p> </div> </div> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)


[warning] 35-68: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.div.innerHTML = /html/ <div class="modal"> <div class="modal-container"> <img class="modal-image" src="${THUMB_NAIL_URL}${data.poster_path}" alt="${data.title}"> <div class="modal-description"> <button class="close-modal"> <img src="${closeImg}" alt="닫기"> </button> <h2 class="modal-title">${data.title}</h2> <p class="modal-release-date-and-genres">${data.release_date.slice(0, 4)} · ${data.genres.map((genre) => genre.name).join(", ")}</p> <div class="average"> <p class="modal-rating-text">평균</p> <img class="average-star" src="${starFilledImg}"> <p class="modal-rating">${data.vote_average.toFixed(1)}</p> </div> <hr> <div class="modal-user-rating"> <p class="modal-user-rating-text">내 별점</p> <div class="star-rating"> <div class="stars-row"> ${[1, 2, 3, 4, 5].map((i) =>).join("")} </div> <div class="rating-text"> <span class="rating-label"></span> <span class="rating-score"></span> </div> </div> </div> <hr> <p class="modal-overview-title">줄거리</p> <p class="modal-overview">${data.overview}</p> </div> </div> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/View/MovieDetailModal.ts` around lines 35 - 69, The modal
currently injects untrusted fields (title, genres, overview, release_date,
poster_path) directly into innerHTML in MovieDetailModal which opens XSS;
refactor to build the DOM with document.createElement calls (or a safe
templating/sanitizer) instead of using innerHTML: create the modal container,
image element (set src from THUMB_NAIL_URL + data.poster_path via setAttribute),
create text nodes and assign textContent for title (.modal-title), release
date/genres (.modal-release-date-and-genres) by joining genre names safely,
overview (.modal-overview), and other text fields; for dynamic stars generate
img elements with class "modal-star" and set dataset.index and src attributes
programmatically; ensure any HTML attributes use safe escaping and do not
concatenate untrusted strings into raw HTML.

Comment on lines +52 to +63
<div class="modal-user-rating">
<p class="modal-user-rating-text">내 별점</p>
<div class="star-rating">
<div class="stars-row">
${[1, 2, 3, 4, 5].map((i) => `<img class="modal-star" data-index="${i}" src="${starEmptyImg}">`).join("")}
</div>
<div class="rating-text">
<span class="rating-label"></span>
<span class="rating-score"></span>
</div>
</div>
</div>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

별점이 키보드 사용자에게는 조작 불가합니다.

클릭 가능한 요소가 <img>뿐이라 포커스 이동, Enter/Space 입력, 현재 선택값의 접근 가능한 이름이 모두 빠져 있습니다. 모달의 핵심 액션이라 버튼/라디오 같은 인터랙티브 요소로 바꿔 두는 편이 좋겠습니다.

Also applies to: 80-95

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/View/MovieDetailModal.ts` around lines 52 - 63, The star widgets
are non-interactive images and inaccessible to keyboard/screen-reader users;
replace the <img class="modal-star"> elements in the "star-rating"/"stars-row"
with interactive controls (preferably real radio buttons or button elements
acting as radio role) so they can receive focus and be activated via
Enter/Space, give the container role="radiogroup" and each control role="radio"
(or use native input type="radio") with aria-checked and an accessible name
(e.g., aria-label="1 star", "2 stars") derived from data-index, add keyboard
handlers to toggle selection on Enter/Space and update the "rating-label" and
"rating-score" text as well as the aria-checked states, and ensure visible focus
styles; apply the same changes to the other instance referenced (lines ~80-95).

Comment on lines +134 to 138
@media screen and (min-width: 769px) and (max-width: 1080px) {
.search-form {
width: 325px;
top: 36px;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

CSS에서 position: static인 요소에 top 값을 주면 실제 레이아웃에 반영되나요?

💡 Result:

아니요, CSS에서 position: static인 요소에 top 값을 설정해도 실제 레이아웃에 반영되지 않습니다. position: static은 기본 위치 지정 방식으로, top, bottom, left, right 등의 offset 속성들이 무시됩니다. 이러한 속성들은 position: relative, absolute, fixed, sticky와 같은 positioned 요소에서만 작동합니다.

Citations:


🏁 Script executed:

rg '\.search-form\s*\{' -A 20 src/styles/

Repository: woowacourse/javascript-movie-review

Length of output: 2391


top: 36px;.search-formposition 설정 없이는 적용되지 않습니다.

.search-formposition 속성이 없어서 이 오프셋 규칙은 레이아웃에 영향을 주지 않습니다. 세로 위치 조정이 필요하다면 먼저 적절한 position 값(예: relative, absolute 등)을 설정해야 합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/styles/main.css` around lines 134 - 138, The .search-form rule inside the
media query uses top: 36px but lacks a position value so the offset has no
effect; update the .search-form selector in that media query (or globally) to
include an appropriate position (e.g., position: relative or position: absolute
depending on desired behavior) so the top: 36px offset is applied, and ensure
this change aligns with surrounding layout (choose relative if you want it
offset within its normal flow, absolute if you want it positioned against a
containing block).

Comment on lines +225 to +231
.modal-image {
width: 400px;
height: 600px;
flex-shrink: 0;
opacity: 1;
border-radius: 16px;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

이미지 셀렉터가 현재 마크업과 어긋나 있습니다.

src/features/View/MovieDetailModal.ts Line 39는 <img class="modal-image"> 하나만 렌더링하는데, 여기 스타일은 .modal-image.modal-image img 전제를 섞어 쓰고 있습니다. 그래서 데스크톱/태블릿의 일부 이미지 크기 규칙이 실제로는 적용되지 않습니다. 클래스 위치를 하나로 통일해 두는 게 좋겠습니다.

Also applies to: 296-301

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/styles/modal.css` around lines 225 - 231, The CSS assumes a container
with an inner <img> (uses rules like .modal-image img) but the MovieDetailModal
component renders a single <img class="modal-image">; update the styles in
modal.css (both the block at the first diff and the later block around lines
296-301) to target the actual element selector .modal-image (remove or duplicate
rules that target .modal-image img) so all size, border-radius and responsive
rules apply to the rendered <img class="modal-image">; alternatively, if you
prefer the container pattern, change MovieDetailModal to render a wrapper (e.g.,
<div class="modal-image"><img /></div>) consistently — but ensure selector and
markup match (.modal-image or .modal-image img) across both CSS blocks.

Copy link
Copy Markdown
Member Author

@janghw0126 janghw0126 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요, 수야!
추가로 이번 미션을 진행하면서 궁금했던 점들을 질문드려요! ☺️

@@ -27,13 +46,57 @@ export function initEvents() {

await renderSearchResults(movieState.page, movieState.searchQuery);
Copy link
Copy Markdown
Member Author

@janghw0126 janghw0126 Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

옵저버 패턴을 적용하면 상태가 변경될 때 구독된 렌더 함수가 자동으로 실행되기 때문에, 이벤트 핸들러에서 렌더 함수를 직접 호출하지 않아도 된다고 알고 있어요. 코드 결합도는 낮아지지만, 상태가 바뀌면 어디선가 렌더링이 일어난다는 흐름이 암묵적으로 되어서 코드를 처음 보는 사람이 흐름을 추적하기 어려워질 수 있다고 느꼈습니다.

실무에서는 이 트레이드오프를 어떤 기준으로 판단하시나요? 특히 팀 프로젝트나 규모가 커질수록 옵저버 방식이 오히려 유지보수를 어렵게 만드는 상황도 있는지 궁금합니다.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상태를 변경하는 쪽이 렌더 함수까지 직접 책임지고 있어서, 상태가 바뀌어도 렌더 호출을 빠뜨리면 UI가 반영되지 않는 구조입니다. 지금은 상태 변경 경로가 세 곳뿐이라 관리가 가능하지만, 이 결합도가 실질적인 문제로 드러나는 시점이 어디인지 감이 잘 오지 않아요.

예를 들어 상태를 변경하는 지점이 늘어나거나, 동일한 상태 변화에 반응해야 하는 UI가 여러 곳이 될 때가 그 시점인지, 아니면 더 이른 시점부터 분리를 고려해야 하는지 궁금합니다.

const saved = localStorage.getItem(`rating-${movieId}`);
if (saved) this.#updateStars(stars, labelEl, scoreEl, Number(saved));

stars.forEach((star) => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MovieDetailModal 안의 별점 클릭 핸들러가 현재 두 가지 일을 동시에 하고 있습니다. 원래는 이벤트 등록을 eventHandler.ts에서 일괄 관리하려 했는데, 별점 이벤트는 모달이 열릴 때만 DOM에 존재하기 때문에 eventHandler.ts에서 처리하기 어려워 지금처럼 MovieDetailModal 안에 구현하게 됐어요.

이런 경우처럼 특정 컴포넌트가 렌더링된 이후에만 등록할 수 있는 이벤트는 어디에서 관리하는 게 적절한가요? eventHandler.ts에서 일괄 관리한다는 원칙을 지키려면 어떤 구조로 설계해야 하는지 궁금합니다.

opacity: 1;
}

@media screen and (max-width: 800px) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반응형 구현을 할 때 AI를 활용하면 금방 되는데, 직접 하려고 하면 어디서부터 시작해야 할지 막막합니다. AI가 만들어준 코드는 동작하지만 왜 그렇게 작성했는지 완전히 이해하지 못한 채 넘어가는 경우가 많은 것 같아요. 미션 구현과 제출을 우선시하다 보니 그냥 넘어가게 되는 것 같기도 합니다.

AI 없이 직접 작성해보려 하면 시간이 오래 걸리고 막히는 지점이 많아서, AI 활용과 직접 체득 사이에서 어떻게 균형을 잡아야 할지 고민입니다.

반응형뿐만 아니라 다른 개념들도 제대로 내 것으로 만들기 위한 학습 방법이 있다면 조언 부탁드립니다!

@janghw0126 janghw0126 changed the title [2단계 - 영화 목록 불러오기] 해니 미션 제출합니다. [2단계 - 상세 정보 & UI/UX 개선하기] 해니 미션 제출합니다. Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants