[fix] 사진 모달에서 Safe Area 미적용 및 스크롤 시 헤더 사라짐 버그를 수정할 수 있다#1653
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Warning
|
| Layer / File(s) | Summary |
|---|---|
Global viewport configuration frontend/index.html |
HTML viewport 메타에 viewport-fit=cover 추가하여 모바일 화면 전체를 활용하도록 설정합니다. |
Common Modal scroll-lock & overlay frontend/src/components/common/Modal/Modal.styles.ts, frontend/src/components/common/Modal/Modal.tsx |
Overlay에 overflow: hidden 추가. Modal 열림 시 document.body를 position: fixed로 고정하여 스크롤을 차단하고, 닫을 때 원래 scrollY 위치를 복원하는 방식으로 개선합니다. 키보드 이벤트 정리부의 overflow 해제 로직은 제거하여 스타일 정리 책임을 통합합니다. |
PhotoModal safe-area & flex layout frontend/src/pages/ClubDetailPage/components/PhotoModal/PhotoModal.styles.ts |
ModalContent를 100dvh·position: fixed로 변경. ModalHeader를 절대배치에서 flex 레이아웃으로 전환하고 safe-area-inset-top 반영. ModalBody와 ImageContainer를 flex: 1/overflow: hidden/min-height: 0으로 구성하여 뷰포트 높이에 맞춘 반응형 레이아웃 적용. ThumbnailContainer의 높이와 패딩을 safe-area-inset-bottom으로 조정합니다. |
ClubDetailFooter safe-area padding frontend/src/pages/ClubDetailPage/components/ClubDetailFooter/ClubDetailFooter.styles.ts |
mobile/tablet 미디어 쿼리에서 padding-bottom을 calc(16px/24px + env(safe-area-inset-bottom)) 형태로 변경하여 하단 safe-area를 반영합니다. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
- [fix] MOA-950 사진 모달에서 Safe Area 미적용 및 스크롤 시 헤더 사라짐 버그를 수정할 수 있다 #1652: PhotoModal의 safe-area 패딩(상단/하단), 100dvh 높이 변경, 헤더 위치 고정, Overlay overflow, viewport-fit=cover 적용 등 체크리스트의 모든 과제를 이 PR에서 구현합니다.
Possibly related PRs
- Moadong/moadong#1108: 동일하게
viewport-fit=cover추가 및--rn-safe-top/safe-area-inset 기반 safe-area 처리를 포함합니다. - Moadong/moadong#744: 동일하게 ClubDetailFooter의
ClubDetailFooter.styles.ts를 수정하며 footer 레이아웃 변경 관련 코드 레벨 연관이 있습니다. - Moadong/moadong#554: PhotoModal.styles.ts의 모바일 반응형 스타일링 및 미디어 쿼리 변경과 코드 레벨로 관련됩니다.
Suggested reviewers
- oesnuj
- seongwon030
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 변경의 주요 내용을 명확하게 반영합니다. Safe Area 미적용 및 스크롤 시 헤더 사라짐 버그 수정이라는 핵심 목표가 raw_summary의 여러 파일 변경사항(PhotoModal.styles.ts, Modal.tsx, viewport-fit=cover 추가 등)과 일치합니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/#1652-photo-modal-safe-area-header-scroll-MOA-950
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
✅ UI 변경사항 없음
전체 60개 스토리 · 23개 컴포넌트 |
There was a problem hiding this comment.
Code Review
This pull request improves the mobile layout of the PhotoModal by adopting viewport-fit=cover, dynamic viewport units (100dvh), and safe area insets, alongside refactoring the modal's internal layout to use flexbox instead of absolute positioning. The review feedback suggests two key improvements: adding padding-bottom: env(safe-area-inset-bottom); to ModalContent to prevent the thumbnail area from overlapping with the mobile home indicator, and adding position: relative; to ModalHeader to restore the positioning context for its absolute-positioned child elements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ${media.mobile} { | ||
| width: 100vw; | ||
| height: 100vh; | ||
| height: 100dvh; | ||
| border-radius: 0; | ||
| } |
There was a problem hiding this comment.
모바일 기기(특히 홈 버튼이 없는 iPhone 등)에서는 하단에 홈 인디케이터가 존재하여 모달 하단의 썸네일 영역(ThumbnailContainer)과 겹칠 수 있습니다.
ModalContent에 padding-bottom: env(safe-area-inset-bottom);을 추가하여 하단 Safe Area를 확보하면, 썸네일 목록이 홈 인디케이터에 가려지거나 터치하기 어려워지는 문제를 방지할 수 있습니다.
${media.mobile} {
width: 100vw;
height: 100dvh;
border-radius: 0;
padding-bottom: env(safe-area-inset-bottom);
}| export const ModalHeader = styled.div` | ||
| width: 100%; | ||
| height: 60px; | ||
| flex-shrink: 0; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| padding: 0 32px; | ||
| position: absolute; | ||
| top: 0; | ||
| left: 0; | ||
| z-index: 10; | ||
| background: ${colors.base.white}; | ||
| backdrop-filter: blur(10px); | ||
|
|
||
| ${media.mobile} { | ||
| padding-top: env(safe-area-inset-top); | ||
| height: calc(60px + env(safe-area-inset-top)); | ||
| } | ||
| `; |
There was a problem hiding this comment.
ModalHeader에서 position: absolute;가 제거되면서 기본값인 static 포지션을 가지게 되었습니다.
하지만 내부 자식 요소인 CloseButton과 ImageCounter는 여전히 position: absolute;를 사용하고 있습니다. 상위 요소에 포지셔닝 컨텍스트(relative, absolute 등)가 지정되어 있지 않으면, 이 자식 요소들은 ModalHeader가 아닌 최상위 포지션 컨텍스트인 ModalContent를 기준으로 위치를 계산하게 됩니다.
또한, top 값을 명시하지 않고 브라우저의 기본 static 위치 계산에 의존하는 경우, Safari 등 일부 브라우저에서 수직 정렬이 깨지는 버그가 발생할 수 있습니다.
따라서 ModalHeader에 position: relative;를 명시적으로 추가하여 안전한 포지셔닝 컨텍스트를 제공하는 것을 권장합니다.
| export const ModalHeader = styled.div` | |
| width: 100%; | |
| height: 60px; | |
| flex-shrink: 0; | |
| display: flex; | |
| justify-content: space-between; | |
| align-items: center; | |
| padding: 0 32px; | |
| position: absolute; | |
| top: 0; | |
| left: 0; | |
| z-index: 10; | |
| background: ${colors.base.white}; | |
| backdrop-filter: blur(10px); | |
| ${media.mobile} { | |
| padding-top: env(safe-area-inset-top); | |
| height: calc(60px + env(safe-area-inset-top)); | |
| } | |
| `; | |
| export const ModalHeader = styled.div` | |
| width: 100%; | |
| height: 60px; | |
| position: relative; | |
| flex-shrink: 0; | |
| display: flex; | |
| justify-content: space-between; | |
| align-items: center; | |
| padding: 0 32px; | |
| z-index: 10; | |
| background: ${colors.base.white}; | |
| backdrop-filter: blur(10px); | |
| ${media.mobile} { | |
| padding-top: env(safe-area-inset-top); | |
| height: calc(60px + env(safe-area-inset-top)); | |
| } | |
| `; |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/pages/ClubDetailPage/components/PhotoModal/PhotoModal.styles.ts (1)
17-21:100dvh브라우저 호환성: 타겟이 최신(Chrome/Edge 108+, Firefox 101+, Safari 15.4+)이라면 우려 낮음
dvh는 Baseline Widely Available로(2025-06) 주요 브라우저에서 폭넓게 안정 지원 중이라 호환성 이슈 가능성은 낮습니다. 다만 타겟 브라우저가 더 구형까지 포함되면 아래처럼 fallback을 두는 정도로 정리하면 됩니다.height: 100vh; height: 100dvh;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/ClubDetailPage/components/PhotoModal/PhotoModal.styles.ts` around lines 17 - 21, In the media.mobile CSS block in PhotoModal.styles.ts (the ${media.mobile} rule), add a fallback height before the existing 100dvh so older browsers use 100vh: set height to 100vh first, then set height to 100dvh (i.e., provide the fallback declaration order) to ensure compatibility while keeping the preferred unit; update the ${media.mobile} block that currently contains width: 100vw; height: 100dvh; border-radius: 0 accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@frontend/src/pages/ClubDetailPage/components/PhotoModal/PhotoModal.styles.ts`:
- Around line 17-21: In the media.mobile CSS block in PhotoModal.styles.ts (the
${media.mobile} rule), add a fallback height before the existing 100dvh so older
browsers use 100vh: set height to 100vh first, then set height to 100dvh (i.e.,
provide the fallback declaration order) to ensure compatibility while keeping
the preferred unit; update the ${media.mobile} block that currently contains
width: 100vw; height: 100dvh; border-radius: 0 accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 892d77c0-b61f-40f3-93da-7b5d16574b69
📒 Files selected for processing (2)
frontend/index.htmlfrontend/src/pages/ClubDetailPage/components/PhotoModal/PhotoModal.styles.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/src/pages/ClubDetailPage/components/PhotoModal/PhotoModal.styles.ts`:
- Around line 23-25: The PhotoModal component in PhotoModal.styles.ts uses a
fixed height of 100dvh without accounting for the device's safe area insets. On
mobile devices with home indicators or notches, this causes bottom content
(thumbnails and touch areas) to be obscured or inaccessible. Add bottom
safe-area padding to the modal by incorporating the CSS environment variable for
safe-area-inset-bottom to ensure proper spacing from the device's physical safe
area boundaries.
- Around line 58-59: The safe-area-inset-top value is being applied twice in the
header styles in PhotoModal.styles.ts: once in the height calculation on line 58
and again as padding-top on line 59. This causes the actual header height to
become 60px plus double the safe area value, excessively reducing the content
area. Remove the duplicate padding-top assignment on line 59 since the safe area
offset is already accounted for in the height calculation, or alternatively
adjust the height to use only the base 60px and apply the safe area only through
padding-top, depending on the intended layout behavior.
🪄 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: 35673e97-f89c-4bab-8f60-0bb4a9f2b755
📒 Files selected for processing (1)
frontend/src/pages/ClubDetailPage/components/PhotoModal/PhotoModal.styles.ts
스크롤 문제 때문에 다시 되돌려서 상태를 확인하고자함.
viewport 제거에 따른 헤더 위치가 이상한 문제 해결을 위한 것.
상단 헤더 고정이 되었던 시점인 거 같아서,,, 되돌립니다.
하단 safe-area 이슈로 분리
seongwon030
left a comment
There was a problem hiding this comment.
--rn-safe-top 이 RN에서 상단 여백을 가지는 변수군요 처음 알게 되었습니다.
OS별로 또 다르게 동작하는 부분 처리하신거 좋습니다!
#️⃣연관된 이슈
📝작업 내용
앱 화면
웹 화면
=> 웹뷰로 마이그레이션을 진행하면서 현재 프로덕션에서는 일시적으로 문제가 해결된 것처럼 보이는 상태입니다. 앱에서는 활동사진 헤더가 safe-area에 겹쳐지지 않지만 스크롤 문제가 존재하고, 웹에서는 스크롤 시 헤더가 사라지는 문제가 있었습니다. 또한 동아리 상세페이지에서 스크롤 하기 전 활동 사진 모달에 들어갈 경우 헤더가 사라지는 문제가 있어 수정했습니다. 하단 safe-area 문제는 다른 이슈에서 수정을 진행하겠습니다.
1. 활동사진 모달 헤더 상단 safe-area 대응
모바일 WebView 환경에서 활동사진 모달을 열었을 때 헤더가 Ios 및 android의 safe-area에 가려지는 문제를 수정했습니다.
기존 TopBarWrapper와 동일한 방식인 --rn-safe-top css 변수를 사용합니다.
// ModalHeader ${media.mobile} { height: calc(60px + var(--rn-safe-top, 0px)); padding-top: var(--rn-safe-top, 0px); } // ImageContainer ${media.mobile} { margin-top: calc(60px + var(--rn-safe-top, 0px)); }2. 모달 헤더 레이아웃 개선 - 동아리 이름과 페이지 수 겹침 문제 수정(+ 닫기 버튼 아이콘 교체)
화면 너비가 좁거나 동아리 이름이 긴 경우, 동아리 이름과 페이지 카운터(1 / N)가 겹치는 문제를 수정했습니다.
flex -> grid 레이아웃으로 전환하여 동아리 이름 영역과 카운터 영역을 분리하고, ClubName에 text-overflow: ellipsis를 적용해 긴 이름은 말줄임 처리하도록 하였습니다.
추가로 기존 텍스트 문자(x)를 SVG 아이콘으로 교체하여 자연스럽게 배치되도록 변경했습니다.
🔥트러블 슈팅 (활동 사진 모달 헤더 safe-area 대응 과정)
모달이 모바일에서 화면 전체를 덮고, Webview에서 헤더가 상단바에 가려지는 문제가 있었습니다.
시도 1 iewport-fit=cover + env(safe-area-inset-top) 적용
viewport-fit=cover와 함께 padding-top: env(safe-area-inset-top)을 ModalHeader에 적용했으나, 헤더가 safe-area보다 훨씬 아래로 내려가는 문제가 발생했습니다.
원인을 찾는 과정에서 하단 safe-area가 적용되지 않아 전체 레이아웃이 밀려 보이는 것일 수 있다고 판단해 ClubDetailFooter와 ThumbnailContainer에도 env(safe-area-inset-bottom)을 함께 적용해봤으나, 헤더 위치 문제는 여전히 해결되지 않았습니다.
시도 2 - 상단 safe-area 대응 되돌리기 괒어에서 의도치 않게 정상 동작
문제가 해결되지 않아 상단 safe-area 적용 이전 상태로 되돌린 후, 다시 진행하려 했으나 완전히 롤백되지 않은 상태에서 헤더가 의도한 위치에 오는 현상이 발생했습니다.
어떤 설정이 원인인지 찾기 위해 불필요해 보이는 변경사항을 하나씩 되돌리는 과정에서 헤더가 다시 아래로 내려가는 현상이 반복됐고, 최종적으로 viewport-fit=cover만 있고 env() 없이도 원하는 위치에 헤더가 오는 것을 확인했습니다.
결과는 맞았지만 정확한 원인을 알 수 없는 상태였으며, viewport-fit=cover는 원래 safe-area 영역까지 content를 넓이는 역할을 하는 코드였기 때문에 오히려 이 문제 수정과 맞지 않는 것 같았습니다.
최종 해결 - --rn-safe-top css 변수 적용
env(safe-area-inset-top) 적용 시 헤더과 과도하게 내려갔던 이유는 플랫폼별 동작 차이 때문이었습니다. iOS는 RN이 네이티브 레벨에서 WebView 프레임을 safe-area 아래에서 시작하도록 처리하므로 별도 CSS 보정 없이도 정상 동작합니다.
반면 Android는 WebView가 상태바 뒤까지 확장되어 CSS 보정이 필요한데, env(safe-area-inset-top)을 적용하면 상태바 높이가 이중으로 더해져 헤더가 의도보다 아래로 내려갔습니다. 당시 viewport-fit=cover가 이를 한 단계 올려주면서 우연히 의도한 위치에 오는 것처럼 보였던 것으로 추정됩니다.
--rn-safe-top은 RN이 각 플랫폼의 safe-area 값을 직접 계산해 CSS 변수로 WebView에 주입하는 방식으로, 중복 적용 없이 정확한 값만 전달됩니다. TopBarWrapper가 동일한 방식으로 이미 동작하고 있음을 확인하고 ModalHeader에 동일한 패턴을 적용했으며, viewport-fit=cover 없이도 정상 동작함을 확인했습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
하단 safe-area 문제는 별도의 이슈로 분리
헤더 밀림 문제가 하단 safe-area 적용이 안 된 문제인가 싶어서 하단 safe-area 문제가 있는 일부 페이지에 env(safe-area-inset-bottom)을 적용했으나, 별도 이슈에서 집중적으로 다루기 위해 이번 PR에서 제거했습니다.
Summary by CodeRabbit