Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Event Constants frontend/src/constants/eventName.ts |
USER_EVENT에 FESTIVAL_POPUP_CLICKED: 'Festival Popup Clicked' 추가 |
Popup Component frontend/src/pages/MainPage/components/Popup/Popup.tsx |
팝업 이미지(AppDownload → FestivalImage) 교체, handleFestival 추가로 클릭 시 FESTIVAL_POPUP_CLICKED 추적 및 /festival-introduction로 네비게이션 처리; useNavigate 및 FestivalImage import 추가 |
Popup Storybook frontend/src/pages/MainPage/components/Popup/Popup.stories.tsx |
Storybook에 INITIAL_VIEWPORTS/defaultViewport: 'iphone6' 추가하고 데코레이터에서 MemoryRouter로 래핑; 모바일 시뮬레이션용 window resize 로직 제거 |
Sequence Diagram(s)
sequenceDiagram
participant User as 사용자
participant Popup as Popup 컴포넌트
participant Const as eventName 상수
participant Analytics as Analytics 서비스
participant Router as 라우터
User->>Popup: 팝업 이미지 클릭
Popup->>Const: FESTIVAL_POPUP_CLICKED 참조
Popup->>Analytics: track(FESTIVAL_POPUP_CLICKED)
Popup->>Router: navigate('/festival-introduction')
Router-->>User: /festival-introduction 로 이동
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20분
Possibly related PRs
- [feature] 메인화면 컴포넌트 스토리북 추가 #1128 — Popup 컴포넌트 및 스토리에 대한 변경을 포함, 동일 파일군과 연관됨
- [fix] 이미지 미로드 시 early return #1184 — Popup.tsx의 이미지/로딩 관련 변경과 충돌 가능성 있음(이미지 로드/early-return 관련)
- [feature] 메인화면에 앱 출시 팝업을 추가한다 #1010 —
eventName.ts및 Popup 관련 이벤트 추적 변경을 다루는 PR로 강한 코드 수준 연관성 있음
Suggested reviewers
- lepitaaar
- oesnuj
- suhyun113
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목 '[feature] 동소한 팝업 추가'는 앱다운로드 팝업을 동소한 팝업으로 변경하는 주요 변경사항을 명확하게 요약하고 있습니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#1300-festival-popup-MOA-728
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}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/MainPage/components/Popup/Popup.tsx (1)
45-54:⚠️ Potential issue | 🟠 Major
popupType값이 일관되지 않아 분석 데이터가 부정확해집니다.팝업이 동소한(festival) 팝업으로 변경되었지만,
MAIN_POPUP_VIEWED와MAIN_POPUP_NOT_SHOWN이벤트의popupType이 여전히'app_download'로 설정되어 있습니다. 이로 인해 분석 데이터가 실제 팝업 유형과 일치하지 않게 됩니다.
handleClose,handleDontShowAgain함수 내의popupType값도 동일한 문제가 있습니다 (Lines 69-72, 78-81).🔧 제안하는 수정 사항
if (isMobile && !isHidden && imageLoaded) { setIsOpen(true); trackEvent(USER_EVENT.MAIN_POPUP_VIEWED, { - popupType: 'app_download', + popupType: 'festival', }); } else { trackEvent(USER_EVENT.MAIN_POPUP_NOT_SHOWN, { - popupType: 'app_download', + popupType: 'festival', }); }
handleClose와handleDontShowAgain함수에도 동일하게 적용:const handleClose = ( action: 'close_button' | 'backdrop_click' = 'close_button', ) => { trackEvent(USER_EVENT.MAIN_POPUP_CLOSED, { - popupType: 'app_download', + popupType: 'festival', action: action, }); // ... }; const handleDontShowAgain = () => { trackEvent(USER_EVENT.MAIN_POPUP_CLOSED, { - popupType: 'app_download', + popupType: 'festival', action: 'dont_show_again', }); // ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/MainPage/components/Popup/Popup.tsx` around lines 45 - 54, The popupType string is hardcoded as 'app_download' causing analytics to mislabel festival popups; update the popupType to the correct value (e.g., 'festival') wherever events are fired: in the isMobile branch that fires USER_EVENT.MAIN_POPUP_VIEWED and USER_EVENT.MAIN_POPUP_NOT_SHOWN, and inside the handleClose and handleDontShowAgain functions so their trackEvent calls use the same 'festival' popupType string, ensuring all USER_EVENT.* calls related to this popup are consistent.
🧹 Nitpick comments (1)
frontend/src/pages/MainPage/components/Popup/Popup.tsx (1)
86-93: 롤백을 위해 미사용 코드가 유지되고 있음을 확인합니다.
handleDownload함수와 관련 import들(AppDownloadImage,detectPlatform,getAppStoreLink)이 현재 사용되지 않지만, PR 목표에 따르면 3월 5일 이후 롤백 예정이므로 유지하는 것이 적절합니다.롤백 시 원활한 복원을 위해 TODO 주석을 추가하는 것을 고려해 보세요.
💡 롤백 관련 주석 추가 제안
+ // TODO: 3월 5일 이후 handleFestival 제거하고 handleDownload로 롤백 예정 const handleDownload = () => { const storeLink = getAppStoreLink();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/MainPage/components/Popup/Popup.tsx` around lines 86 - 93, The unused popup download logic (handleDownload) and related imports (AppDownloadImage, detectPlatform, getAppStoreLink) are intentionally retained for an upcoming rollback; add a clear TODO comment near the handleDownload function and the related imports indicating these are preserved for rollback after 2026-03-05 (or the specified date), include owner/contact and a short reason so future reviewers know to keep them until the rollback window passes, and ensure the comment references the function name handleDownload and the imports AppDownloadImage, detectPlatform, getAppStoreLink so they’re easy to locate later.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/src/pages/MainPage/components/Popup/Popup.tsx`:
- Around line 45-54: The popupType string is hardcoded as 'app_download' causing
analytics to mislabel festival popups; update the popupType to the correct value
(e.g., 'festival') wherever events are fired: in the isMobile branch that fires
USER_EVENT.MAIN_POPUP_VIEWED and USER_EVENT.MAIN_POPUP_NOT_SHOWN, and inside the
handleClose and handleDontShowAgain functions so their trackEvent calls use the
same 'festival' popupType string, ensuring all USER_EVENT.* calls related to
this popup are consistent.
---
Nitpick comments:
In `@frontend/src/pages/MainPage/components/Popup/Popup.tsx`:
- Around line 86-93: The unused popup download logic (handleDownload) and
related imports (AppDownloadImage, detectPlatform, getAppStoreLink) are
intentionally retained for an upcoming rollback; add a clear TODO comment near
the handleDownload function and the related imports indicating these are
preserved for rollback after 2026-03-05 (or the specified date), include
owner/contact and a short reason so future reviewers know to keep them until the
rollback window passes, and ensure the comment references the function name
handleDownload and the imports AppDownloadImage, detectPlatform, getAppStoreLink
so they’re easy to locate later.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
frontend/src/assets/images/popup/festival.pngis excluded by!**/*.png
📒 Files selected for processing (2)
frontend/src/constants/eventName.tsfrontend/src/pages/MainPage/components/Popup/Popup.tsx
lepitaaar
approved these changes
Mar 3, 2026
Contributor
lepitaaar
left a comment
lepitaaar
left a comment
There was a problem hiding this comment.
3월 5일 이후는 수동으로 삭제하시나요?
Member
Author
api가 아니기에 수동 삭제 할 것 같네요 |
🎨 UI 변경사항을 확인해주세요
8개 스토리 변경 · 전체 58개 스토리 · 23개 컴포넌트 |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/MainPage/components/Popup/Popup.stories.tsx (1)
16-23:⚠️ Potential issue | 🟡 Minor스토리 문서 설명을 현재 팝업 목적에 맞게 갱신해주세요.
docs.description.component가 아직 “앱 다운로드 팝업”으로 되어 있어, 현재 동소한(축제) 팝업 동작과 문서가 불일치합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/MainPage/components/Popup/Popup.stories.tsx` around lines 16 - 23, Update the story description in Popup.stories.tsx by changing docs.description.component from the outdated "앱 다운로드 팝업" text to a concise description matching the current festival/동소한 popup behavior (e.g., mention it's the 모바일 축제/동소한 팝업 shown on mobile); locate the docs.description.component entry in the story config (near viewport/INITIAL_VIEWPORTS and defaultViewport: 'iphone6') and replace the string so the story docs accurately describe the popup's current purpose.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/src/pages/MainPage/components/Popup/Popup.stories.tsx`:
- Around line 16-23: Update the story description in Popup.stories.tsx by
changing docs.description.component from the outdated "앱 다운로드 팝업" text to a
concise description matching the current festival/동소한 popup behavior (e.g.,
mention it's the 모바일 축제/동소한 팝업 shown on mobile); locate the
docs.description.component entry in the story config (near
viewport/INITIAL_VIEWPORTS and defaultViewport: 'iphone6') and replace the
string so the story docs accurately describe the popup's current purpose.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/pages/MainPage/components/Popup/Popup.stories.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣연관된 이슈
📝작업 내용
동소한 팝업 추가
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
새로운 기능
문서/개발환경