[Refactor/#144] 캠페인 상세 및 광고 리스트 UX/UI 리팩토링 - #148
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough캠페인 상세 페이지 UI와 광고 상세의 트래킹 제어 흐름을 리팩토링했습니다. 트래킹 링크 발급 흐름으로 제어 로직을 단순화하고, 캠페인 헤더와 광고 행 레이아웃을 재구성했으며 모달 버튼 스타일을 소폭 조정했습니다. Changes
Sequence DiagramsequenceDiagram
participant 사용자 as User
participant AdDetail as AdDetailContent
participant Modal as ModalContent
participant 클립보드 as Clipboard
User->>AdDetail: "링크 발급하기" 클릭
activate AdDetail
AdDetail->>Modal: 발급 모달 오픈
deactivate AdDetail
activate Modal
User->>Modal: "발급하기" 클릭
Modal->>AdDetail: landingUrl 검증 요청
alt landingUrl 없음
AdDetail->>User: 에러 토스트("랜딩 URL 필요") 표시
else landingUrl 존재
AdDetail->>클립보드: 트래킹 링크 복사 시도
클립보드->>User: 복사 성공/실패 응답
Modal->>Modal: 모달 닫기
end
deactivate Modal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
📚 Storybook 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ads/AdDetailContent.tsx (1)
106-123:⚠️ Potential issue | 🟡 Minor
landingUrl가 없을 때의 상태 처리가 아직 일관되지 않습니다.
landingUrl가 optional인데 지금 구현은 값이 없어도 랜딩 URL 복사 아이콘이 활성 상태로 보이고,링크 발급하기도 모달까지 들어간 다음에야 실패를 알게 됩니다. 게다가 confirm 분기에서toast.error후throw까지 해서useControlModal의 generic 실패 토스트가 한 번 더 붙습니다. placeholder/disabled 상태를 upfront로 주고, 검증은handleConfirm호출 전에 끝내는 쪽이 UX가 더 안정적입니다. 아이콘-only 버튼에는aria-label도 같이 넣어 주세요.🔧 제안 코드
<div className="flex items-center justify-between w-full h-9 px-4 py-2 bg-white border border-bg-disabled rounded-component-sm group-hover:border-primary-light transition-all"> <span className="font-body2 text-text-auth-sub truncate pr-10 select-all"> - {ad.landingUrl} + {ad.landingUrl ?? "등록된 랜딩 URL이 없습니다."} </span> <button + type="button" + disabled={!ad.landingUrl} + aria-label={ad.landingUrl ? "랜딩 URL 복사" : "등록된 랜딩 URL 없음"} onClick={(e) => { e.stopPropagation(); - ad.landingUrl && handleCopy(ad.landingUrl); + if (!ad.landingUrl) return; + handleCopy(ad.landingUrl); }} className="shrink-0 text-text-placeholder hover:text-primary-main transition-colors p-1" title="링크 복사" > <LinkIcon className="w-5 h-5" /> </button> </div> @@ - buttonDisabled={false} + buttonDisabled={!isTrackingActive && !ad.landingUrl} @@ - onConfirm={() => - trackControl.handleConfirm(async () => { - if (!ad.landingUrl) { - toast.error( - "광고에 등록된 랜딩 URL이 없어 발급이 불가능합니다.", - ); - throw new Error("랜딩 URL이 없습니다."); - } - await createTrackingUrl(Number(orgId), ad.id, ad.landingUrl); - }) - } + onConfirm={() => { + if (!ad.landingUrl) { + toast.error("광고에 등록된 랜딩 URL이 없어 발급이 불가능합니다."); + return; + } + + trackControl.handleConfirm(async () => { + await createTrackingUrl(Number(orgId), ad.id, ad.landingUrl); + }); + }}As per coding guidelines,
src/**: 다음 핵심 영역에 집중하여 리뷰한다. 6. 에러 처리: API 실패 대응 및 사용자 피드백 적절성 검토. 에러 바운더리 사용 확인. 7. 접근성: 시맨틱 HTML, ARIA 속성 사용 확인.Also applies to: 130-145, 207-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ads/AdDetailContent.tsx` around lines 106 - 123, The landingUrl handling is inconsistent: ensure the UI shows a placeholder and disables the copy/action affordances when ad.landingUrl is falsy, add an aria-label to the icon-only button (e.g., aria-label="링크 복사"), and make handleCopy only callable when ad.landingUrl exists (guard in the onClick and disable the button visually and via disabled attribute); likewise validate ad.landingUrl before invoking handleConfirm/useControlModal so the modal isn't opened for missing URLs, move toast.error into that pre-check (do not throw after showing the toast to avoid duplicate generic failure toasts from useControlModal), and update references in this component (ad.landingUrl, handleCopy, handleConfirm, useControlModal, LinkIcon) accordingly so the UX and accessibility are fixed.
🧹 Nitpick comments (1)
src/pages/ads/list/CampaignDetail.tsx (1)
23-27: 플랫폼 아이콘 매핑은 공용으로 빼두는 편이 좋겠습니다.여기서 새로 만든
LogoMap이src/components/ads/AdRow.tsx의 매핑과 사실상 동일하고, 사용 시점에는provider.toLowerCase() as TPlatform으로 API 값을 UI 타입에 강제 캐스팅하고 있습니다. 플랫폼이 추가되거나 네이밍이 바뀌면 한쪽만 수정돼도 헤더 아이콘이 조용히 빠질 수 있어서, 공용PlatformIcon컴포넌트나 상수로 묶어 두는 쪽이 유지보수에 더 안전합니다.As per coding guidelines,
src/**: 다음 핵심 영역에 집중하여 리뷰한다. 2. 구조와 책임 분리: 페이지에 비즈니스 로직이 과도하지 않은지 확인. 커스텀 훅으로의 분리 여부 검토.Also applies to: 117-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/CampaignDetail.tsx` around lines 23 - 27, The LogoMap object defined in CampaignDetail (LogoMap) duplicates the same platform-to-icon mapping in src/components/ads/AdRow.tsx and risks drift when platform names change; extract this mapping into a shared constant or a new PlatformIcon component and replace both usages with that shared symbol. Update CampaignDetail to stop using provider.toLowerCase() as TPlatform casting and instead pass the raw provider value into the shared PlatformIcon API (or normalize centrally inside the shared utility), and update AdRow to import and use the same PlatformIcon/PLATFORM_ICON_MAP so both components reference a single source of truth (symbols to change: LogoMap, TPlatform usages, provider.toLowerCase() as TPlatform, and the mapping in AdRow).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/ads/list/CampaignDetail.tsx`:
- Around line 85-91: Replace the back navigation that uses navigate(-1) with an
explicit route using the orgId from useParams so the button always returns to
the ads list; in CampaignDetail change the onClick handler on the back button
(the element rendering LeftChevronIcon) to call navigate(`/ads/${orgId}`)
instead of navigate(-1), ensuring orgId is read from useParams and available in
the component before using it.
---
Outside diff comments:
In `@src/components/ads/AdDetailContent.tsx`:
- Around line 106-123: The landingUrl handling is inconsistent: ensure the UI
shows a placeholder and disables the copy/action affordances when ad.landingUrl
is falsy, add an aria-label to the icon-only button (e.g., aria-label="링크 복사"),
and make handleCopy only callable when ad.landingUrl exists (guard in the
onClick and disable the button visually and via disabled attribute); likewise
validate ad.landingUrl before invoking handleConfirm/useControlModal so the
modal isn't opened for missing URLs, move toast.error into that pre-check (do
not throw after showing the toast to avoid duplicate generic failure toasts from
useControlModal), and update references in this component (ad.landingUrl,
handleCopy, handleConfirm, useControlModal, LinkIcon) accordingly so the UX and
accessibility are fixed.
---
Nitpick comments:
In `@src/pages/ads/list/CampaignDetail.tsx`:
- Around line 23-27: The LogoMap object defined in CampaignDetail (LogoMap)
duplicates the same platform-to-icon mapping in src/components/ads/AdRow.tsx and
risks drift when platform names change; extract this mapping into a shared
constant or a new PlatformIcon component and replace both usages with that
shared symbol. Update CampaignDetail to stop using provider.toLowerCase() as
TPlatform casting and instead pass the raw provider value into the shared
PlatformIcon API (or normalize centrally inside the shared utility), and update
AdRow to import and use the same PlatformIcon/PLATFORM_ICON_MAP so both
components reference a single source of truth (symbols to change: LogoMap,
TPlatform usages, provider.toLowerCase() as TPlatform, and the mapping in
AdRow).
🪄 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: 5e0a2e33-af12-49dc-8df6-53794090ad19
⛔ Files ignored due to path filters (1)
src/assets/icon/chevron/chervon-left.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (4)
src/components/ads/AdDetailContent.tsxsrc/components/ads/AdRow.tsxsrc/components/common/modal/ModalContent.tsxsrc/pages/ads/list/CampaignDetail.tsx
Seojegyeong
approved these changes
Mar 28, 2026
Seojegyeong
left a comment
Seojegyeong
left a comment
Collaborator
There was a problem hiding this comment.
P4: 확인했습니다! 수고하셨습니다:)
9 tasks
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
3 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.
🚨 관련 이슈
close #144
✨ 변경사항
✏️ 작업 내용
Tracking.URL.X.mov
캠페인 상세 헤더 및 반응형 레이아웃 개선
광고 리스트
AdRowUI 최적화광고 상세 정보
AdDetailContent및 트래킹 로직 리팩토링트래킹 중단버튼 제거링크 복사하기, 없을 시링크 발급하기버튼으로 변경😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
현재 워크스페이스 1의 캠페인 1에 대한 데이터가 없어서, 작업 시
MainLayout.tsx에서 캠페인 ID를 2로 고정하여 진행했습니다.Summary by CodeRabbit
릴리스 노트
새로운 기능
UI/스타일 개선
버그 수정