[OMF-342] 이미지 문항 핀치 줌 및 전체화면 뷰어 개선#163
Conversation
- 핀치 투 줌(pinch-to-zoom) 및 확대/축소 기능 추가 - 전체화면 모달 UI 개선 (배경색, 라운드, 닫기 버튼 스타일) - 확대 전 힌트 오버레이 표시 후 핀치 시 자동 소멸 - 인라인 스타일을 Tailwind 클래스로 전환
|
Note
|
| Layer / File(s) | Summary |
|---|---|
줌 설정 및 훅 의존성 src/features/survey/components/SurveyImage.tsx |
React import이 useCallback, useEffect, useRef, useState 훅을 포함하도록 확장되었습니다. 줌 범위를 설정하는 MIN_SCALE, MAX_SCALE 상수와 스케일 값을 제한하는 clamp 헬퍼 함수가 추가되었습니다. |
전체 화면 줌 상태 및 터치 핸들러 src/features/survey/components/SurveyImage.tsx |
전체 화면 줌 상태가 도입됨: scale, isPinching, showHint 플래그와 핀치 거리 추적을 위한 ref가 추가되었습니다. resetZoom, handleClose 및 터치 이벤트 핸들러(handleTouchStart, handleTouchMove, handleTouchEnd)가 핀치 제스처를 감지하고 스케일을 업데이트합니다. Effect는 전체 화면에서 기본 touchmove 스크롤을 방지하고 전체 화면 진입 시 힌트를 표시합니다. |
버튼 및 스타일 변형 업데이트 src/features/survey/components/SurveyImage.tsx |
확장 버튼 스타일이 스타일 상수 방식에서 유틸리티 클래스 기반 className으로 변경되었습니다. VARIANT_STYLES 내 square 변형 스타일 블록 구조가 조정되었습니다. |
전체 화면 포탈 마크업 및 줌/힌트 오버레이 src/features/survey/components/SurveyImage.tsx |
전체 화면 포탈 JSX가 재작성됨: 오버레이 컨테이너와 닫기 버튼 스타일이 업데이트되었습니다. 이미지 래퍼가 이제 ref와 터치 이벤트 핸들러를 수용합니다. 동적 transform: scale 이 적용되며 핀치 중에는 트랜지션이 비활성화됩니다. showHint 플래그가 true일 때 조건부 힌트 오버레이가 렌더링됩니다. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
✨Feat
Suggested reviewers
- chldsbdud
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | 제목은 PR의 주요 변경사항인 이미지 문항의 핀치 줌 및 전체화면 뷰어 개선을 명확하게 반영하고 있습니다. |
| 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. |
✏️ 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
feature/OMF-342
Tip
💬 Introducing Slack Agent: The best way for teams to turn conversations into code.
Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
- Generate code and open pull requests
- Plan features and break down work
- Investigate incidents and troubleshoot customer tickets together
- Automate recurring tasks and respond to alerts with triggers
- Summarize progress and report instantly
Built for teams:
- Shared memory across your entire org—no repeating context
- Per-thread sandboxes to safely plan and execute work
- Governance built-in—scoped access, auditability, and budget controls
One agent for your entire SDLC. Right inside Slack.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/features/survey/components/SurveyImage.tsx (2)
119-133: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win버튼 클릭 핸들러에 Sentry
ui.click스팬이 누락되었습니다.전체 보기 진입 버튼(Line 122
onClick)과 닫기 버튼(Line 149onClick=handleClose) 모두 UI 인터랙션 추적이 빠져 있습니다. 코딩 가이드라인에 따라Sentry.startSpan({ op: 'ui.click', ... })로 감싸고, 필요한 컨텍스트는span.setAttribute()로 첨부해 주세요. 더불어 파일 상단의 Sentry import도 추가가 필요합니다.♻️ 제안 예시
+import * as Sentry from '@sentry/react'; import { adaptive } from "@toss/tds-colors";<button type="button" aria-label="이미지 크게 보기" - onClick={() => setIsFullscreen(true)} + onClick={() => + Sentry.startSpan( + { op: 'ui.click', name: 'SurveyImage open fullscreen' }, + (span) => { + span?.setAttribute('variant', variant); + setIsFullscreen(true); + }, + ) + } className="absolute bottom-2 right-2 ..."- const handleClose = useCallback(() => { - setIsFullscreen(false); - setShowHint(false); - resetZoom(); - }, [resetZoom]); + const handleClose = useCallback(() => { + Sentry.startSpan( + { op: 'ui.click', name: 'SurveyImage close fullscreen' }, + (span) => { + span?.setAttribute('finalScale', scale); + setIsFullscreen(false); + setShowHint(false); + resetZoom(); + }, + ); + }, [resetZoom, scale]);As per coding guidelines: "In component action handlers, use
Sentry.startSpanwithop: 'ui.click'to create spans for button clicks and other UI interactions, and usespan.setAttribute()to add metrics" and "Import Sentry usingimport * as Sentry from '@sentry/react'".Also applies to: 146-164
🤖 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 `@src/features/survey/components/SurveyImage.tsx` around lines 119 - 133, Add Sentry UI click spans around the fullscreen-enter and close button handlers in SurveyImage.tsx: import Sentry with "import * as Sentry from '@sentry/react'" at the top, then wrap the onClick that calls setIsFullscreen(true) (the inline handler using setIsFullscreen) and the onClick that invokes handleClose in Sentry.startSpan({ op: 'ui.click', description: '...'}), call span.setAttribute(...) to attach relevant context (e.g., action: 'open_fullscreen' or 'close_fullscreen', imageId or src), and ensure you call span.finish() after the handler completes (or in finally) so the span always closes.
136-144:⚠️ Potential issue | 🟠 Major | ⚡ Quick win전체화면 모달에 키보드 닫기(Escape) 지원이 누락되었습니다.
role="dialog"+aria-modal="true"로 모달임을 명시했지만 키보드 사용자(예: 외장 키보드 연결 환경, 데스크톱 웹뷰)는 Escape로 닫을 수 없고, 포커스가 모달 외부에 머무를 수 있습니다. 최소한 Escape 키 핸들러를 추가하고, 가능하면 모달 진입 시 포커스를 닫기 버튼으로 이동시켜 주세요.🛠️ 제안 패치
useEffect(() => { if (!isFullscreen) return; - setShowHint(true); - }, [isFullscreen]); + setShowHint(true); + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === "Escape") handleClose(); + }; + window.addEventListener("keydown", onKeyDown); + return () => window.removeEventListener("keydown", onKeyDown); + }, [isFullscreen, handleClose]);🤖 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 `@src/features/survey/components/SurveyImage.tsx` around lines 136 - 144, Add keyboard Escape support and initial focus for the fullscreen modal in the SurveyImage component: create a ref (e.g., closeButtonRef) for the modal close button and, inside a useEffect that runs when isFullscreen changes, if isFullscreen and typeof document !== "undefined" focus closeButtonRef.current and attach a keydown listener on document that calls the component's existing close handler (e.g., closeFullscreen / onClose / toggleFullscreen — use the exact function name used in this file) when event.key === "Escape"; remove the listener and blur/cleanup on unmount or when isFullscreen becomes false. Ensure the createPortal modal markup uses the referenced close button element so focus-able action exists and that the effect only runs when isFullscreen is true.
🧹 Nitpick comments (1)
src/features/survey/components/SurveyImage.tsx (1)
103-106: ⚡ Quick win힌트가 사용자가 핀치하지 않으면 사라지지 않습니다.
showHint는 전체화면 진입 시true로 설정되고,handleTouchStart에서 두 손가락 터치가 시작될 때만false로 바뀝니다. 따라서 사용자가 한 손가락만 사용하거나 잠시 보기만 한 채로 머무르면 안내 오버레이가 계속 노출됩니다. 일정 시간 후 자동으로 숨기거나, 최초 터치(1손가락 포함) 시 숨기는 것이 자연스럽습니다.♻️ 자동 사라짐 예시
useEffect(() => { if (!isFullscreen) return; - setShowHint(true); + setShowHint(true); + const timer = window.setTimeout(() => setShowHint(false), 2500); + return () => window.clearTimeout(timer); }, [isFullscreen]);Also applies to: 189-203
🤖 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 `@src/features/survey/components/SurveyImage.tsx` around lines 103 - 106, The hint overlay is set true on entering fullscreen (useEffect that sets showHint) but only cleared on two-finger pinch in handleTouchStart, so it never hides for single-finger touches or passive viewing; update the logic so showHint is cleared either on the first touch of any type (handleTouchStart/handlePointerDown) or after a short timeout (e.g., 3s) when isFullscreen becomes true; modify the useEffect that sets showHint and the touch handlers (handleTouchStart) to call the same hideHint routine (or start a timer) and apply the same fix to the duplicate hint logic used elsewhere (the other showHint/useEffect block around the second occurrence).
🤖 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 `@src/features/survey/components/SurveyImage.tsx`:
- Around line 188-203: The hint text promises panning but the touch handler only
updates scale, so either implement pan or change the hint; to implement pan, add
translateX and translateY state in the SurveyImage component and update them
inside handleTouchMove when touches.length === 1 by computing clientX/clientY
deltas (store previous touch point to compute delta), then include
translate(...) together with scale(...) in the image style/transform;
alternatively, if you prefer the simpler fix, update the hint (showHint text) to
instruct only pinch-to-zoom (e.g., "두 손가락으로 확대해 보세요!") and ensure any duplicated
hint usages (the other hint at lines ~72-86) are changed consistently.
- Around line 72-86: In the SurveyImage component's touch handler, remove the
ineffective e.preventDefault() call from the useCallback function named
handleTouchMove; keep the two-finger distance calculation and scale update logic
(references: handleTouchMove, lastTouchDistRef, setScale, MIN_SCALE, MAX_SCALE)
unchanged, since native touchmove with passive:false (the addEventListener
handler) is already responsible for blocking scroll—just delete the
e.preventDefault() line to avoid relying on React's passive synthetic event
behavior.
---
Outside diff comments:
In `@src/features/survey/components/SurveyImage.tsx`:
- Around line 119-133: Add Sentry UI click spans around the fullscreen-enter and
close button handlers in SurveyImage.tsx: import Sentry with "import * as Sentry
from '@sentry/react'" at the top, then wrap the onClick that calls
setIsFullscreen(true) (the inline handler using setIsFullscreen) and the onClick
that invokes handleClose in Sentry.startSpan({ op: 'ui.click', description:
'...'}), call span.setAttribute(...) to attach relevant context (e.g., action:
'open_fullscreen' or 'close_fullscreen', imageId or src), and ensure you call
span.finish() after the handler completes (or in finally) so the span always
closes.
- Around line 136-144: Add keyboard Escape support and initial focus for the
fullscreen modal in the SurveyImage component: create a ref (e.g.,
closeButtonRef) for the modal close button and, inside a useEffect that runs
when isFullscreen changes, if isFullscreen and typeof document !== "undefined"
focus closeButtonRef.current and attach a keydown listener on document that
calls the component's existing close handler (e.g., closeFullscreen / onClose /
toggleFullscreen — use the exact function name used in this file) when event.key
=== "Escape"; remove the listener and blur/cleanup on unmount or when
isFullscreen becomes false. Ensure the createPortal modal markup uses the
referenced close button element so focus-able action exists and that the effect
only runs when isFullscreen is true.
---
Nitpick comments:
In `@src/features/survey/components/SurveyImage.tsx`:
- Around line 103-106: The hint overlay is set true on entering fullscreen
(useEffect that sets showHint) but only cleared on two-finger pinch in
handleTouchStart, so it never hides for single-finger touches or passive
viewing; update the logic so showHint is cleared either on the first touch of
any type (handleTouchStart/handlePointerDown) or after a short timeout (e.g.,
3s) when isFullscreen becomes true; modify the useEffect that sets showHint and
the touch handlers (handleTouchStart) to call the same hideHint routine (or
start a timer) and apply the same fix to the duplicate hint logic used elsewhere
(the other showHint/useEffect block around the second occurrence).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 300626c1-94ac-44f0-91c7-50f7a0fe0e00
📒 Files selected for processing (1)
src/features/survey/components/SurveyImage.tsx
| const handleTouchMove = useCallback((e: React.TouchEvent) => { | ||
| e.preventDefault(); | ||
| if (e.touches.length === 2) { | ||
| const ddx = e.touches[0].clientX - e.touches[1].clientX; | ||
| const ddy = e.touches[0].clientY - e.touches[1].clientY; | ||
| const newDist = Math.sqrt(ddx * ddx + ddy * ddy); | ||
|
|
||
| if (lastTouchDistRef.current !== null) { | ||
| const ratio = newDist / lastTouchDistRef.current; | ||
| setScale((prev) => clamp(prev * ratio, MIN_SCALE, MAX_SCALE)); | ||
| } | ||
|
|
||
| lastTouchDistRef.current = newDist; | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Are React 18 synthetic onTouchMove listeners passive at the root, making e.preventDefault() ineffective?
💡 Result:
Yes—React’s synthetic onTouchMove/onTouchStart/onWheel handlers are effectively registered as passive at the root in the common intervention case, so calling e.preventDefault inside those handlers will not prevent the default behavior. React explicitly describes this behavior: the “Keep onTouchStart, onTouchMove, and onWheel passive” change states that e.preventDefault in these handlers stays broken “due to Chrome’s intervention,” even though React moved from document-level listeners to root listeners [1]. The corresponding code change emulates passive listeners on roots for touchstart/touchmove/wheel (sets isPassiveListener = true for those event names) [2]. For React 18 specifically, community discussion confirms the intent and that there isn’t a React-native API to mark these listeners as non-passive; the issue notes that even onWheel/onTouchMove handlers “are passive” so preventDefault doesn’t work, and suggests manual addEventListener workarounds instead [3]. Related: React’s SyntheticEvent wrapper still exposes preventDefault [4], but whether it has effect is constrained by the browser’s passive listener semantics (the problem is at listener registration, not at the SyntheticEvent API surface) [1][2][4]. Workaround (if you truly need preventDefault): attach native listeners yourself with { passive: false } (for example via useEffect + addEventListener on the element), because React doesn’t provide an escape hatch for non-passive React synthetic touch/wheel handlers [3][5].
Citations:
- 1: Keep onTouchStart, onTouchMove, and onWheel passive facebook/react#19654
- 2: facebook/react@dd651df05
- 3: React 18 not passive wheel / touch event listeners support facebook/react#22794
- 4: https://legacy.reactjs.org/docs/events.html
- 5: https://stackoverflow.com/questions/76406592/how-to-do-passivefalse-event-listeners-in-react
🏁 Script executed:
# Check the full file content around the mentioned lines to verify the workaround
head -n 110 src/features/survey/components/SurveyImage.tsx | tail -n 40Repository: On-Survey/Frontend
Length of output: 1174
React 합성 이벤트의 e.preventDefault()는 실제로 작동하지 않습니다.
React 18에서 onTouchMove 합성 이벤트 리스너는 루트에서 passive로 등록되므로, Line 73의 e.preventDefault() 호출은 스크롤을 차단하지 못합니다. 실제 스크롤 차단은 Lines 94–101의 네이티브 addEventListener("touchmove", ..., { passive: false })로 구현된 핸들러가 담당합니다. 합성 이벤트 핸들러에서 불필요한 e.preventDefault() 호출을 제거하세요.
♻️ 제안 패치
const handleTouchMove = useCallback((e: React.TouchEvent) => {
- e.preventDefault();
if (e.touches.length === 2) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleTouchMove = useCallback((e: React.TouchEvent) => { | |
| e.preventDefault(); | |
| if (e.touches.length === 2) { | |
| const ddx = e.touches[0].clientX - e.touches[1].clientX; | |
| const ddy = e.touches[0].clientY - e.touches[1].clientY; | |
| const newDist = Math.sqrt(ddx * ddx + ddy * ddy); | |
| if (lastTouchDistRef.current !== null) { | |
| const ratio = newDist / lastTouchDistRef.current; | |
| setScale((prev) => clamp(prev * ratio, MIN_SCALE, MAX_SCALE)); | |
| } | |
| lastTouchDistRef.current = newDist; | |
| } | |
| }, []); | |
| const handleTouchMove = useCallback((e: React.TouchEvent) => { | |
| if (e.touches.length === 2) { | |
| const ddx = e.touches[0].clientX - e.touches[1].clientX; | |
| const ddy = e.touches[0].clientY - e.touches[1].clientY; | |
| const newDist = Math.sqrt(ddx * ddx + ddy * ddy); | |
| if (lastTouchDistRef.current !== null) { | |
| const ratio = newDist / lastTouchDistRef.current; | |
| setScale((prev) => clamp(prev * ratio, MIN_SCALE, MAX_SCALE)); | |
| } | |
| lastTouchDistRef.current = newDist; | |
| } | |
| }, []); |
🤖 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 `@src/features/survey/components/SurveyImage.tsx` around lines 72 - 86, In the
SurveyImage component's touch handler, remove the ineffective e.preventDefault()
call from the useCallback function named handleTouchMove; keep the two-finger
distance calculation and scale update logic (references: handleTouchMove,
lastTouchDistRef, setScale, MIN_SCALE, MAX_SCALE) unchanged, since native
touchmove with passive:false (the addEventListener handler) is already
responsible for blocking scroll—just delete the e.preventDefault() line to avoid
relying on React's passive synthetic event behavior.
| {/* Hint */} | ||
| {showHint && ( | ||
| <div className="absolute bottom-10 flex items-center gap-2 px-5 py-3 rounded-full pointer-events-none bg-[#B0B8C1]"> | ||
| <Asset.Icon | ||
| frameShape={Asset.frameShape.CleanW24} | ||
| backgroundColor="transparent" | ||
| name="icon-x-mono" | ||
| color={adaptive.grey600} | ||
| name="icon-arrow-maximum-mono" | ||
| color="#ffffff" | ||
| aria-hidden={true} | ||
| ratio="1/1" | ||
| /> | ||
| </button> | ||
| </div> | ||
| <span className="text-white text-sm font-medium whitespace-nowrap"> | ||
| 확대하고, 손으로 움직여보세요! | ||
| </span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
힌트 문구와 실제 구현이 일치하지 않습니다.
힌트 오버레이의 문구는 "확대하고, 손으로 움직여보세요!"인데, handleTouchMove는 두 손가락의 거리(scale)만 갱신할 뿐 한 손가락 팬(pan/translate) 동작은 구현되어 있지 않습니다. 확대 후 이미지의 가려진 영역을 이동시킬 수단이 없어 사용자가 안내대로 시도해도 동작하지 않습니다.
다음 중 하나로 정렬해 주세요.
- 팬(이동) 기능까지 구현 (예:
translateX/Y상태를 추가하고touches.length === 1일 때clientX/Y델타로 갱신,transform에translate(...)합성). - 또는 팬을 구현하지 않을 경우 힌트 문구를 확대 동작만 안내하도록 수정 (예: "두 손가락으로 확대해 보세요!").
Also applies to: 72-86
🤖 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 `@src/features/survey/components/SurveyImage.tsx` around lines 188 - 203, The
hint text promises panning but the touch handler only updates scale, so either
implement pan or change the hint; to implement pan, add translateX and
translateY state in the SurveyImage component and update them inside
handleTouchMove when touches.length === 1 by computing clientX/clientY deltas
(store previous touch point to compute delta), then include translate(...)
together with scale(...) in the image style/transform; alternatively, if you
prefer the simpler fix, update the hint (showHint text) to instruct only
pinch-to-zoom (e.g., "두 손가락으로 확대해 보세요!") and ensure any duplicated hint usages
(the other hint at lines ~72-86) are changed consistently.
|
흠... |
2 participants
📌 주요 변경 사항
🔗 관련 이슈
Jira 링크: https://onsurvey.atlassian.net/browse/OMF-342
✅ 리뷰 요청 사항 (Need Review)
🎨 스크린샷 (선택)
Summary by CodeRabbit
릴리스 노트
신기능
스타일