Repository navigation
feat: 내가 작성한 책 이야기 햄버거 버튼 UI 수정 - #263
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request adds edit and delete functionality to the story detail view by introducing new props and a conditional dropdown menu. The review feedback suggests implementing the currently empty click handlers and refactoring the dropdown menu to reduce code duplication, fix a bug where the menu remains open during login prompts, and improve layout flexibility by using auto-height.
| onEditClick={() => {}} | ||
| onDeleteClick={() => {}} |
There was a problem hiding this comment.
아직 수정 페이지(/edit)와 삭제 API 명세가 완전히 연동되지 않은 UI 작업 단계라 임시로 비워두었던 부분이었습니다.
리뷰 남겨주신 조언에 맞춰, 미래의 협업자와 제가 잊지 않도록 // TODO 주석으로 명확한 라우팅 및 API 연동 계획을 명시해 두었습니다!
추가적으로 유저가 클릭했을 때 아무 반응이 없으면 당황할 수 있으므로 임시로 toast("준비 중인 기능입니다") 알림 메시지를 띄우는 방향으로 보완해 두겠습니다.
| const renderDropdownMenu = () => { | ||
| if (!menuOpen) return null; | ||
|
|
||
| if (isMyStory) { | ||
| return ( | ||
| <div className="absolute right-0 top-full mt-1 w-34 h-22 rounded-lg bg-White z-10 px-2 shadow-md flex flex-col justify-center"> | ||
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| setMenuOpen(false); | ||
| onEditClick?.(); | ||
| }} | ||
| className="flex w-full items-center gap-2 px-4 py-3 body_1_2 text-Gray-4 hover:text-Gray-7 cursor-pointer" | ||
| > | ||
| <Image src="/Edit.svg" alt="수정" width={20} height={20} /> | ||
| 수정하기 | ||
| </button> | ||
| <div className="mx-2 border-b border-Subbrown-4" /> | ||
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| setMenuOpen(false); | ||
| onDeleteClick?.(); | ||
| }} | ||
| className="flex w-full items-center gap-2 px-4 py-3 body_1_2 text-Gray-4 hover:text-Gray-7 cursor-pointer" | ||
| > | ||
| <Image src="/delete.svg" alt="삭제" width={20} height={20} /> | ||
| 삭제하기 | ||
| </button> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <div className="absolute right-0 top-full mt-1 w-34 h-22 rounded-lg bg-White z-10 px-2 shadow-md flex flex-col justify-center"> | ||
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| if (!isLoggedIn) { | ||
| openLoginModal(); | ||
| return; | ||
| } | ||
| setIsReportModalOpen(true); | ||
| setMenuOpen(false); | ||
| }} | ||
| className="flex w-full items-center gap-2 px-4 py-3 body_1_2 text-Gray-4 hover:text-Gray-7 cursor-pointer" | ||
| > | ||
| <Image src="/report.svg" alt="신고" width={20} height={20} /> | ||
| 신고하기 | ||
| </button> | ||
| <div className="mx-2 border-b border-Subbrown-4" /> | ||
| <button | ||
| type="button" | ||
| onClick={handleShare} | ||
| className="flex w-full items-center gap-2 px-4 py-3 body_1_2 text-Gray-4 hover:text-Gray-7 cursor-pointer" | ||
| > | ||
| <Image | ||
| src="/gray_share.svg" | ||
| alt="공유" | ||
| width={20} | ||
| height={20} | ||
| /> | ||
| 공유하기 | ||
| </button> | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
renderDropdownMenu 함수 내에서 isMyStory 조건에 따라 반환되는 JSX 구조가 중복되고 있습니다. 메뉴 항목을 배열로 관리하고 공통 컨테이너를 사용하도록 리팩토링하면 유지보수성이 향상됩니다.
또한 다음과 같은 개선 사항이 있습니다:
- 일관성: 작성자 본인(
isMyStory)인 경우에도 드롭다운 메뉴에 '공유하기'를 포함하는 것이 좋습니다. - 버그 수정: 비로그인 상태에서 '신고하기' 클릭 시
openLoginModal만 호출되고 드롭다운 메뉴가 닫히지 않는 문제를 해결해야 합니다. - 유연성: 고정 높이(
h-22) 대신h-auto를 사용하여 메뉴 항목 수 변화에 대응할 수 있도록 합니다.
const renderDropdownMenu = () => {
if (!menuOpen) return null;
const menuItems = isMyStory
? [
{ label: "수정하기", icon: "/Edit.svg", onClick: onEditClick },
{ label: "삭제하기", icon: "/delete.svg", onClick: onDeleteClick },
{ label: "공유하기", icon: "/gray_share.svg", onClick: handleShare },
]
: [
{
label: "신고하기",
icon: "/report.svg",
onClick: () => (isLoggedIn ? setIsReportModalOpen(true) : openLoginModal()),
},
{ label: "공유하기", icon: "/gray_share.svg", onClick: handleShare },
];
return (
<div className="absolute right-0 top-full mt-1 w-34 h-auto py-1 rounded-lg bg-White z-10 px-2 shadow-md flex flex-col justify-center">
{menuItems.map((item, index) => (
<div key={item.label}>
<button
type="button"
onClick={() => {
setMenuOpen(false);
item.onClick?.();
}}
className="flex w-full items-center gap-2 px-4 py-3 body_1_2 text-Gray-4 hover:text-Gray-7 cursor-pointer"
>
<Image src={item.icon} alt={item.label} width={20} height={20} />
{item.label}
</button>
{index < menuItems.length - 1 && (
<div className="mx-2 border-b border-Subbrown-4" />
)}
</div>
))}
</div>
);
};
There was a problem hiding this comment.
-
내 책 이야기(isMyStory)의 드롭다운에도 '공유하기' 항목을 포함시켰습니다.
-
각 메뉴 버튼 바인딩 내부에 공통적으로 setMenuOpen(false)를 먼저 호출하도록 설계하여 비로그인 유저가 신고하기를 눌렀을 때 드롭다운 잔상이 남는 버그를 확실히 잡았습니다.
-
높이를 고정(h-22) 대신 h-auto와 py-1으로 유연하게 열어두어 향후 메뉴 개수가 변하더라도 UI가 깨지지 않도록 수정 반영하였습니다.
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 (1)
src/components/base-ui/BookStory/Detatil/bookstory_detail.tsx (1)
85-95:⚠️ Potential issue | 🟠 MajorUse separate refs for mobile and desktop dropdown roots.
The same
menuRefis assigned to both the mobile hamburger (line 241, hidden viat:hiddenCSS class) and the desktop hamburger (line 373). Both containers are mounted simultaneously, so React's ref will point only to the last one (desktop). This causes mobile dropdown interactions to be incorrectly treated as outside clicks becausehandleClickOutsidechecks only the desktop container.🔒 Proposed fix
- const menuRef = useRef<HTMLDivElement>(null); + const mobileMenuRef = useRef<HTMLDivElement>(null); + const desktopMenuRef = useRef<HTMLDivElement>(null);const handleClickOutside = (e: MouseEvent) => { - if (menuRef.current && !menuRef.current.contains(e.target as Node)) { + const target = e.target as Node; + const clickedInsideMobile = mobileMenuRef.current?.contains(target); + const clickedInsideDesktop = desktopMenuRef.current?.contains(target); + + if (!clickedInsideMobile && !clickedInsideDesktop) { setMenuOpen(false); } };- <div className="relative shrink-0 ml-2" ref={menuRef}> + <div className="relative shrink-0 ml-2" ref={mobileMenuRef}>- <div className="relative" ref={menuRef}> + <div className="relative" ref={desktopMenuRef}>Also applies to: 241-250, 373-383
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/base-ui/BookStory/Detatil/bookstory_detail.tsx` around lines 85 - 95, The click-outside logic uses a single menuRef for both mobile and desktop dropdowns causing incorrect outside-click detection; create two refs (e.g., mobileMenuRef and desktopMenuRef), attach mobileMenuRef to the mobile hamburger/dropdown element and desktopMenuRef to the desktop hamburger/dropdown element, then update handleClickOutside to treat a click as outside only if it is outside both refs (check both mobileMenuRef.current and desktopMenuRef.current via contains), and ensure refs are cleaned up correctly in useEffect cleanup and any dependent places that previously used menuRef (like setMenuOpen usage remains unchanged).
🤖 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/app/`(main)/stories/[id]/page.tsx:
- Around line 118-120: Owner actions are wired but empty: in the component where
isMyStory is passed (using story.writtenByMe), implement onEditClick to navigate
to the story edit route using story.id (e.g., router.push or next/navigation) so
“수정하기” opens the edit page, and implement onDeleteClick to open a confirmation
modal then call your delete API (or deleteStory(story.id)) and refresh/redirect
(invalidate SWR/React Query or router.replace) to remove the story;
alternatively hide the dropdown actions until these handlers exist. Reference
the props isMyStory, onEditClick, onDeleteClick, story.writtenByMe and story.id
when making the changes.
In `@src/components/base-ui/BookStory/Detatil/bookstory_detail.tsx`:
- Around line 157-168: The dropdown menu stays open when unauthenticated because
the onClick handler calls openLoginModal() and returns before closing the menu;
update the onClick in the component that renders the button (referenced by
isLoggedIn, openLoginModal and setMenuOpen in the handler) so you call
setMenuOpen(false) before calling openLoginModal(), then call openLoginModal()
and return to prevent opening the report modal; ensure the order is:
setMenuOpen(false) then openLoginModal() and keep the existing early return.
---
Outside diff comments:
In `@src/components/base-ui/BookStory/Detatil/bookstory_detail.tsx`:
- Around line 85-95: The click-outside logic uses a single menuRef for both
mobile and desktop dropdowns causing incorrect outside-click detection; create
two refs (e.g., mobileMenuRef and desktopMenuRef), attach mobileMenuRef to the
mobile hamburger/dropdown element and desktopMenuRef to the desktop
hamburger/dropdown element, then update handleClickOutside to treat a click as
outside only if it is outside both refs (check both mobileMenuRef.current and
desktopMenuRef.current via contains), and ensure refs are cleaned up correctly
in useEffect cleanup and any dependent places that previously used menuRef (like
setMenuOpen usage remains unchanged).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b539656f-8567-4eaf-9978-f8d99e038807
📒 Files selected for processing (2)
src/app/(main)/stories/[id]/page.tsxsrc/components/base-ui/BookStory/Detatil/bookstory_detail.tsx
| isMyStory={story.writtenByMe} | ||
| onEditClick={() => {}} | ||
| onDeleteClick={() => {}} |
There was a problem hiding this comment.
Wire real edit/delete handlers before showing owner actions.
With isMyStory={story.writtenByMe}, the dropdown exposes “수정하기” and “삭제하기”, but both handlers are empty, so owner actions visibly do nothing. Please connect the edit navigation/delete flow, or hide these actions until they are implemented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(main)/stories/[id]/page.tsx around lines 118 - 120, Owner actions
are wired but empty: in the component where isMyStory is passed (using
story.writtenByMe), implement onEditClick to navigate to the story edit route
using story.id (e.g., router.push or next/navigation) so “수정하기” opens the edit
page, and implement onDeleteClick to open a confirmation modal then call your
delete API (or deleteStory(story.id)) and refresh/redirect (invalidate SWR/React
Query or router.replace) to remove the story; alternatively hide the dropdown
actions until these handlers exist. Reference the props isMyStory, onEditClick,
onDeleteClick, story.writtenByMe and story.id when making the changes.
There was a problem hiding this comment.
@shinwokkang, 이해했습니다! 현재 파일 내용을 보니 이미 빈 no-op 대신 toast로 사용자에게 "준비 중" 안내를 표시하고 TODO 주석도 추가하셨네요 — 이 방식이 사용자 경험 측면에서 더 낫습니다.
추후 구현 시 놓치지 않도록 Follow-up 이슈를 생성해 드릴까요? 이슈에는 아래 내용을 포함할 수 있습니다:
onEditClick:router.push(/stories/${story.bookStoryId}/edit)연결onDeleteClick: 삭제 확인 모달 +useDeleteStoryMutationAPI 연동 및 리다이렉트 처리
원하시면 말씀해 주세요!
| return ( | ||
| <div className="absolute right-0 top-full mt-1 w-34 h-22 rounded-lg bg-White z-10 px-2 shadow-md flex flex-col justify-center"> | ||
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| if (!isLoggedIn) { | ||
| openLoginModal(); | ||
| return; | ||
| } | ||
| setIsReportModalOpen(true); | ||
| setMenuOpen(false); | ||
| }} |
There was a problem hiding this comment.
Close the dropdown before opening the login modal.
For unauthenticated users, openLoginModal() returns early while the hamburger menu stays open behind the modal.
🐛 Proposed fix
onClick={() => {
if (!isLoggedIn) {
+ setMenuOpen(false);
openLoginModal();
return;
}
setIsReportModalOpen(true);
setMenuOpen(false);📝 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.
| return ( | |
| <div className="absolute right-0 top-full mt-1 w-34 h-22 rounded-lg bg-White z-10 px-2 shadow-md flex flex-col justify-center"> | |
| <button | |
| type="button" | |
| onClick={() => { | |
| if (!isLoggedIn) { | |
| openLoginModal(); | |
| return; | |
| } | |
| setIsReportModalOpen(true); | |
| setMenuOpen(false); | |
| }} | |
| <button | |
| type="button" | |
| onClick={() => { | |
| if (!isLoggedIn) { | |
| setMenuOpen(false); | |
| openLoginModal(); | |
| return; | |
| } | |
| setIsReportModalOpen(true); | |
| setMenuOpen(false); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/base-ui/BookStory/Detatil/bookstory_detail.tsx` around lines
157 - 168, The dropdown menu stays open when unauthenticated because the onClick
handler calls openLoginModal() and returns before closing the menu; update the
onClick in the component that renders the button (referenced by isLoggedIn,
openLoginModal and setMenuOpen in the handler) so you call setMenuOpen(false)
before calling openLoginModal(), then call openLoginModal() and return to
prevent opening the report modal; ensure the order is: setMenuOpen(false) then
openLoginModal() and keep the existing early return.
📌 개요 (Summary)
🛠️ 변경 사항 (Changes)
📸 스크린샷 (Screenshots)
(UI 변경 사항이 있다면 첨부해주세요)
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)Summary by CodeRabbit