[Feature/#70] ControlBox 구현 및 페이지 적용 - #71
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdsListPage 하단에 재사용 가능한 UI 블록인 ControlBox 컴포넌트를 새로 추가하고 Storybook 스토리를 등록했으며, AdsListPage 레이아웃을 재구성해 CampaignTable 아래에 ControlBox를 배치했습니다. Changes
Sequence Diagram(s)(생성 조건 미충족 — 변경은 주로 레이아웃과 신규 UI 컴포넌트 추가로, 복잡한 다중 컴포넌트 제어 흐름 시각화가 필요하지 않습니다.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 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
🧹 Nitpick comments (2)
src/pages/ads/list/AdsListPage.tsx (1)
33-33: 불필요한 빈 DOM 노드는 제거하는 편이 좋습니다.Line 33의
<div />는 현재 역할이 없어 레이아웃 복잡도만 늘립니다. 제거해도 동작 영향이 없다면 정리해주세요.♻️ 제안 diff
- - <div /> </section>As per coding guidelines
src/**:2. 구조와 책임 분리: 페이지에 비즈니스 로직이 과도하지 않은지 확인. 커스텀 훅으로의 분리 여부 검토.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ads/list/AdsListPage.tsx` at line 33, AdsListPage의 불필요한 빈 DOM 노드인 self-closing <div /> (현재 AdsListPage 컴포넌트 내에 위치한 `<div />`)를 제거하세요; 레이아웃이나 스타일에 영향이 없는지 확인하기 위해 해당 컴포넌트의 렌더 트리와 CSS 클래스(또는 부모 컨테이너)만 빠르게 점검하고, 제거 후 브라우저에서 화면이 동일하게 보이는지 및 관련 유닛/통합 테스트가 통과하는지 확인하십시오.src/components/common/controlbox/ControlBox.tsx (1)
35-38: 버튼에서stopPropagation강제는 재사용성을 낮출 수 있습니다.Line 35-38에서 이벤트 버블링을 항상 막으면, 상위 컨테이너의 클릭 추적/라우팅 핸들러가 필요한 화면에서 의도치 않게 동작이 끊길 수 있어요. 특별한 이유가 없다면 제거하거나 옵션화하는 편이 안전합니다.
♻️ 제안 diff
<button type="button" - onClick={(e) => { - e.stopPropagation(); - onButtonClick(); - }} + onClick={onButtonClick} className="px-8 py-4 bg-chart-3 text-white font-body1 rounded-component-md hover:bg-chart-3/90 overflow-hidden transition-all shrink-0 active:scale-95" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/controlbox/ControlBox.tsx` around lines 35 - 38, ControlBox currently forcefully calls e.stopPropagation() inside the onClick handler which reduces reusability; change this to be opt-in by adding a prop (e.g., stopPropagation?: boolean) to the ControlBox component props and use that prop in the onClick handler (only call e.stopPropagation() when stopPropagation is true), or remove the call entirely if you prefer default bubbling—update the ControlBox component signature and the onClick closure that calls onButtonClick() to respect the new prop and adjust any consumers to pass stopPropagation where needed.
🤖 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/AdsListPage.tsx`:
- Line 26: The ControlBox action currently only logs to console in AdsListPage
(onButtonClick={() => console.log("페이지 이동")}); replace this no-op with a real
user feedback or routing action: call the router navigation (e.g.,
useNavigate()/history.push) to go to the target ad creation or detail route, or
alternatively trigger a visible UI feedback (e.g., showToast/showSnackbar or
open a "준비 중" modal via state) so users see an effect; update the onButtonClick
handler in AdsListPage to perform navigation or set the feedback state and
remove the console.log.
---
Nitpick comments:
In `@src/components/common/controlbox/ControlBox.tsx`:
- Around line 35-38: ControlBox currently forcefully calls e.stopPropagation()
inside the onClick handler which reduces reusability; change this to be opt-in
by adding a prop (e.g., stopPropagation?: boolean) to the ControlBox component
props and use that prop in the onClick handler (only call e.stopPropagation()
when stopPropagation is true), or remove the call entirely if you prefer default
bubbling—update the ControlBox component signature and the onClick closure that
calls onButtonClick() to respect the new prop and adjust any consumers to pass
stopPropagation where needed.
In `@src/pages/ads/list/AdsListPage.tsx`:
- Line 33: AdsListPage의 불필요한 빈 DOM 노드인 self-closing <div /> (현재 AdsListPage 컴포넌트
내에 위치한 `<div />`)를 제거하세요; 레이아웃이나 스타일에 영향이 없는지 확인하기 위해 해당 컴포넌트의 렌더 트리와 CSS 클래스(또는
부모 컨테이너)만 빠르게 점검하고, 제거 후 브라우저에서 화면이 동일하게 보이는지 및 관련 유닛/통합 테스트가 통과하는지 확인하십시오.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/common/controlbox/ControlBox.stories.tsxsrc/components/common/controlbox/ControlBox.tsxsrc/pages/ads/list/AdsListPage.tsx
jjjsun
reviewed
Mar 1, 2026
Seojegyeong
approved these changes
Mar 1, 2026
Collaborator
|
P4: 고생하셨습니다! PR 제목 양식 지켜주세요 :) @YermIm |
Collaborator
|
P2: 현재 WorkspaceSetting의 워크스페이스 삭제 섹션도 인라인으로 구현되어 있는데, 이 ControlBox 컴포넌트를 확장해서 사용하는 방향으로 리팩토링하면 좋을 것 같습니다!! @jjjsun |
YermIm
changed the title
Collaborator
네 좋은것같아요! API연동 PR완료한후에 리팩토링 PR추가로 따로 올릴게요! |
📚 Storybook 배포 완료
|
📚 Storybook 배포 완료
|
This was referenced Mar 3, 2026
9 tasks
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 #70
✨ 변경사항
✏️ 작업 내용
ControlBox컴포넌트 제작😅 미완성 작업
📢 논의 사항 및 참고 사항
N/A
Summary by CodeRabbit