[FIX] 화면 간 쿼리키 동기화, 포커스 레이아웃, 모달 겹침 버그 수정#230
Conversation
- OverlayModal에 Modal 컴포넌트와 동일한 스택 기반 z-index 로직을 적용했습니다 - 정적인 z-40/z-50 클래스를 acquireModalStackIndex 기반 동적 z-index로 교체했습니다
- 제목·서브태스크·메모의 줄바꿈을 wrap-break-word에서 wrap-anywhere로 교체해, 공백 없는 긴 텍스트가 flex 아이템의 자동 최소 너비를 부풀려 카드가 타이머 패널 뒤로 사라지는 문제를 해결했습니다 - 메모 영역에 max-h-[40vh]와 세로 스크롤을 추가해 긴 메모가 페이지 전체 높이를 늘리지 않도록 했습니다
- Home/Today 탭과 사이드바 타이머에서 재생·일시정지·재개·완료·정지 시 집중 페이지 쿼리를 함께 invalidate하도록 했습니다 - 집중 페이지에서 완료·서브태스크 변경 시 Today 탭과 할 일 수정 모달 쿼리를 함께 invalidate하도록 했습니다 - 할 일 수정 모달에서 수정·삭제 시 집중 페이지 쿼리를 함께 invalidate하도록 했습니다
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Walkthrough타이머와 할 일 변경 성공 시 Today·Focus·투두 상세 쿼리 무효화 범위를 확장했습니다. 중첩 모달에는 동적 z-index를 적용했고, FocusTaskItem의 줄바꿈과 서브태스크 스크롤 동작을 조정했습니다. Changes쿼리 캐시 동기화
모달 스택 및 Focus 레이아웃
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
ehye1
left a comment
ehye1
left a comment
There was a problem hiding this comment.
동기화 신경쓰는 게 제일 어려운 것 같아요.
수정 수고하셨습니다 !! 굿뜨
Timo Performance ReportBundle Size — timo-web
Lighthouse — timo-web
Image Optimization — timo-web
측정 커밋: |
![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
🤖 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 `@apps/timo-web/components/layout/sidebar/time/TimerPanel.tsx`:
- Around line 38-41: Extract the duplicated invalidateTodayView and
invalidateFocusTodo query invalidation callbacks into the existing
useTimerQueryInvalidation hook or another shared invalidation hook. In
apps/timo-web/components/layout/sidebar/time/TimerPanel.tsx lines 38-41, remove
the local declarations and consume the shared callbacks; update
apps/timo-web/app/[locale]/(main)/(with-time-sidebar)/home/_hooks/use-home-todos-by-date.ts
lines 54-56 and
apps/timo-web/app/[locale]/(main)/(with-time-sidebar)/today/_hooks/useTodayTodoList.ts
lines 52-55 to use shared invalidateFocusTodo, and
apps/timo-web/app/[locale]/(main)/focus/_hooks/use-focus-session.ts lines 50-51
to use shared invalidateTodayView.
In `@apps/timo-web/components/modal/OverlayModal.tsx`:
- Line 56: Update the useEffect containing setStackIndex and
acquireModalStackIndex so changes to the onExited callback do not rerun the
effect while the modal remains open. Store the latest onExited callback in a ref
or otherwise remove it safely from the effect dependencies, while preserving the
existing modal stack acquisition behavior for genuine open-state changes.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 6e7ee0db-84b5-43d6-881c-fbfe74361e28
📒 Files selected for processing (8)
apps/timo-web/app/[locale]/(main)/(with-time-sidebar)/home/_hooks/use-home-todos-by-date.tsapps/timo-web/app/[locale]/(main)/(with-time-sidebar)/today/_hooks/useTodayTodoList.tsapps/timo-web/app/[locale]/(main)/focus/_components/FocusTaskItem.tsxapps/timo-web/app/[locale]/(main)/focus/_hooks/use-focus-session.tsapps/timo-web/components/layout/sidebar/time/TimerPanel.tsxapps/timo-web/components/modal/OverlayModal.tsxapps/timo-web/hooks/todo-modal/detail/use-delete-todo-submit.tsapps/timo-web/hooks/todo-modal/detail/use-update-todo-submit.ts
| return () => clearTimeout(hideTimer); | ||
| } | ||
|
|
||
| setStackIndex(acquireModalStackIndex()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
useEffect 의존성 배열로 인한 불필요한 스택 인덱스 증가를 방지하세요.
현재 의존성 배열에 onExited가 포함되어 있어, 부모 컴포넌트에서 onExited 콜백의 참조가 바뀔 때마다 useEffect가 재실행될 위험이 있습니다. 모달이 이미 열려있는 상태(isOpen === true)임에도 불구하고 acquireModalStackIndex()가 반복 호출되면, 전역 스택 인덱스가 계속 증가하여 z-index 체계가 꼬이거나 불필요한 리렌더링이 발생할 수 있습니다.
리렌더링 시에도 안전하게 최신 콜백을 참조할 수 있도록 onExited를 useRef로 관리하거나, 의존성 배열에서 안전하게 분리하는 것을 권장합니다. (참고: React 공식 문서 - Effect에서 최신 props 및 state 읽기) 멋진 모달 중첩 로직이 더욱 견고해질 거예요! 😉
💡 제안하는 `useRef` 활용 수정안
const [shouldRender, setShouldRender] = useState(isOpen);
const [isVisible, setIsVisible] = useState(false);
const [stackIndex, setStackIndex] = useState(0);
const dialogRef = useRef<HTMLDivElement>(null);
const wasFloatingLayerOpenRef = useRef(false);
+ const onExitedRef = useRef(onExited);
+ onExitedRef.current = onExited;
useEffect(() => {
if (!isOpen) {
setIsVisible(false);
const hideTimer = setTimeout(() => {
setShouldRender(false);
- onExited?.();
+ onExitedRef.current?.();
}, EXIT_ANIMATION_DURATION);
return () => clearTimeout(hideTimer);
}
setStackIndex(acquireModalStackIndex());
setShouldRender(true);
const showFrame = requestAnimationFrame(() => setIsVisible(true));
return () => cancelAnimationFrame(showFrame);
- }, [isOpen, onExited]);
+ }, [isOpen]);📝 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.
| setStackIndex(acquireModalStackIndex()); | |
| const [shouldRender, setShouldRender] = useState(isOpen); | |
| const [isVisible, setIsVisible] = useState(false); | |
| const [stackIndex, setStackIndex] = useState(0); | |
| const dialogRef = useRef<HTMLDivElement>(null); | |
| const wasFloatingLayerOpenRef = useRef(false); | |
| const onExitedRef = useRef(onExited); | |
| onExitedRef.current = onExited; | |
| useEffect(() => { | |
| if (!isOpen) { | |
| setIsVisible(false); | |
| const hideTimer = setTimeout(() => { | |
| setShouldRender(false); | |
| onExitedRef.current?.(); | |
| }, EXIT_ANIMATION_DURATION); | |
| return () => clearTimeout(hideTimer); | |
| } | |
| setStackIndex(acquireModalStackIndex()); | |
| setShouldRender(true); | |
| const showFrame = requestAnimationFrame(() => setIsVisible(true)); | |
| return () => cancelAnimationFrame(showFrame); | |
| }, [isOpen]); |
🤖 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 `@apps/timo-web/components/modal/OverlayModal.tsx` at line 56, Update the
useEffect containing setStackIndex and acquireModalStackIndex so changes to the
onExited callback do not rerun the effect while the modal remains open. Store
the latest onExited callback in a ref or otherwise remove it safely from the
effect dependencies, while preserving the existing modal stack acquisition
behavior for genuine open-state changes.
- 4개 파일에 중복 정의돼 있던 invalidateTodayView/invalidateFocusTodo를 useTimerQueryInvalidation 훅으로 통합했습니다
2 participants
ISSUE 🔗
close #224
What is this PR? 🔍
개발자 QA에서 발견된 화면 간 동기화 문제 3건을 수정했습니다: 화면 전반의 React Query 쿼리키 미연동, 포커스 세션 메모가 길 때의 레이아웃 붕괴, 모달 중첩 시 오버레이 겹침입니다.
배경
화면 간 쿼리키 동기화
invalidateXxx헬퍼 패턴을 그대로 따라, 누락된 상대 쿼리키(getGetFocusTodoQueryKey,getGetTodayQueryKey,getGetTodoDetailQueryKey)를 해당 mutation의onSuccess에 추가하는 방식으로 수정했습니다. 새로운 추상화나 공용 훅을 만들지 않고, 기존 파일별 로컬 invalidate 함수 컨벤션을 유지했습니다.use-home-todos-by-date.ts,useTodayTodoList.ts: 타이머 재생/일시정지/재개 시getGetFocusTodoQueryKey()를 함께 invalidateuse-focus-session.ts: 완료/정지/서브태스크 변경 시getGetTodayQueryKey()와getGetTodoDetailQueryKey()를 함께 invalidateTimerPanel.tsx(사이드바 타이머): 완료/정지/할 일 상태 변경 시getGetTodayQueryKey()와getGetFocusTodoQueryKey()를 함께 invalidateuse-update-todo-submit.ts,use-delete-todo-submit.ts(수정 모달): 수정/삭제 성공 시getGetFocusTodoQueryKey()를 함께 invalidate포커스 세션 레이아웃
overflow-wrap: break-word는 브라우저가 flex 아이템의 자동 최소 너비(intrinsic min-content)를 계산할 때는 반영되지 않는 스펙 특성이 있습니다. 그래서 공백 없는 긴 메모가 있으면 부모 flex 아이템의 자동 최소 너비가 카드의 의도된 최소 너비(min-w-80)가 아니라 메모 전체 길이만큼 부풀어 올랐고, 그 결과 카드가 타이머 패널 뒤로 완전히 가려지는 현상이 있었습니다.wrap-break-word(overflow-wrap: break-word)에서wrap-anywhere(overflow-wrap: anywhere)로 교체했습니다.anywhere는 자동 최소 너비 계산에도 반영되는 값이라, 메모의 최소 너비 기여도가 낮아지고 카드의 명시적min-w-80이 실제 하한으로 동작합니다. 이 조합을 로컬 정적 HTML로 재현해 여러 뷰포트 폭에서 검증했습니다: 평소엔 자연스럽게 반응형으로 줄어들다가, 320px 바닥에 닿으면 카드와 타이머 패널 전체가 가로 스크롤로 전환됩니다. 메모/서브태스크 영역에는max-h-[40vh] overflow-y-auto를 추가해, 메모가 길어도 페이지 전체 높이가 늘어나지 않고 카드 내부에서만 세로 스크롤되도록 했습니다.min-w-0을 주지 않고, 자식에 명시적min-w-80+wrap-anywhere만 주는 방식)은 원래 #109에서 의도했던 "평소엔 반응형, 바닥 아래는 가로 스크롤" 동작을 그대로 복원한 것입니다. 로그인이 필요한 환경이라 실제 앱에서 라이브 스크린샷은 확보하지 못했고, 실제 소스 클래스와 동일한 컴파일된 Tailwind CSS를 사용한 격리된 재현 HTML로 레이아웃 동작만 검증했습니다.모달 오버레이 겹침
OverlayModal(태그 생성 모달 등에서 쓰는 두 번째 모달 구현체)에도Modal컴포넌트와 동일한 스택 기반 z-index 로직을 적용했습니다.Modal컴포넌트는 이미acquireModalStackIndex()로 중첩된 모달의 오버레이/패널 z-index를 단계별로 올리고 있었지만,OverlayModal은 정적인z-40/z-50클래스만 쓰고 있어서, 이 컴포넌트를 쓰는 모달이 다른 모달 위에 중첩되면 아래 모달의 패널보다도 낮은 z-index로 렌더링돼 겹쳐 보였습니다.Modal.tsx와 동일한BASE_OVERLAY_Z_INDEX/BASE_PANEL_Z_INDEX/MODAL_STACK_Z_INDEX_STEP상수와acquireModalStackIndex()호출을OverlayModal에도 추가하고, 정적z-40/z-50클래스를stackIndex기반 인라인style={{ zIndex }}로 교체했습니다.To Reviewers
포커스 세션 레이아웃 수정은 실제 로그인된 브라우저가 아니라 동일한 컴파일된 Tailwind 클래스를 쓴 격리된 정적 HTML 재현으로 검증했습니다. 실제 앱에서 좁은 화면 + 긴 메모 조합으로 한 번 더 확인해주시면 좋을 것 같습니다.
쿼리키 동기화는 여러 화면(Home/Today/Focus/사이드바/수정 모달)에 걸쳐 있어 변경 지점이 많은데, 모두 기존 파일의 로컬 invalidate 헬퍼 패턴을 그대로 따랐고 새 추상화는 추가하지 않았습니다.
Screenshot 📷
Test Checklist ✔
pnpm check-types통과pnpm lint통과