DEV -> PROD #13
DEV -> PROD #13
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a site-wide mobile detection hook and makes UI mobile-aware: sidebar auto-collapses on small screens with a fixed open button, modals gain ESC/backdrop/mobile-layout options, new PlaceActionModal fetches coordinates, and PlacePage/cards open the action modal on mobile taps. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Mobile User
participant Card as PlaceCard (Main/Other)
participant Page as PlacePage
participant Modal as PlaceActionModal
participant API as /places/geographies
User->>Card: tap card (mobile)
Card->>Page: onMobileClick(place)
Page->>Modal: open(place, isOpen=true)
alt coordinates missing
Modal->>API: GET /places/geographies?placeId=...
API-->>Modal: coordinates (or error)
end
Modal->>User: render image + action buttons
User->>Modal: click action (e.g., "좌표 수정")
Modal->>Page: invoke callback (onCoordinateEdit/onEdit/onDelete/onImageManage)
Modal->>Page: call onClose (close modal)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/layout/UseIsMobile.jsx (1)
3-15: SSR safety concern: Directwindowaccess during initial render.The hook accesses
window.matchMediasynchronously during state initialization (line 4), which will throw aReferenceErrorif this code ever runs in a server-side rendering context or during tests without a DOM. Consider initializing to a safe default and syncing in auseEffect:export function useIsMobile() { - const [isMobile, setIsMobile] = useState(window.matchMedia("(max-width: 768px)").matches); + const [isMobile, setIsMobile] = useState(false); useEffect(() => { const mediaQuery = window.matchMedia("(max-width: 768px)"); + setIsMobile(mediaQuery.matches); const handler = (e) => setIsMobile(e.matches); mediaQuery.addEventListener("change", handler); return () => mediaQuery.removeEventListener("change", handler); }, []); return isMobile; }Also, the file naming convention for hooks is typically lowercase (e.g.,
useIsMobile.js). Consider renaming for consistency with React conventions.src/App.jsx (2)
53-58: Potential UI flash on mobile due to initial state mismatch.The
sidebarOpenstate initializes totrue(line 53), then theuseEffectsets it tofalseon mobile (line 57). This causes a brief flash where the sidebar appears open before closing on mobile devices.Consider initializing
sidebarOpenbased on viewport width directly, or use a loading state to prevent the flash:- const [sidebarOpen, setSidebarOpen] = useState(true); + const [sidebarOpen, setSidebarOpen] = useState(() => + typeof window !== 'undefined' ? !window.matchMedia("(max-width: 768px)").matches : true + ); const isMobile = useIsMobile(); - useEffect(() => { - setSidebarOpen(!isMobile); // 모바일에서는 기본 닫힘 - }, [isMobile]);Alternatively, keep the effect but add a brief loading state to prevent the flash.
121-129: Redundantmd:hiddenclass.The button rendering is already gated by
isMobile && !sidebarOpen(line 121), so themd:hiddenclass on line 123 is redundant. IfisMobileis false, the button won't render regardless of the CSS class.<button - className="fixed bottom-6 right-6 z-40 md:hidden bg-white border border-gray-200 shadow-sm px-3 py-2 rounded-full text-gray-700 flex items-center gap-2" + className="fixed bottom-6 right-6 z-40 bg-white border border-gray-200 shadow-sm px-3 py-2 rounded-full text-gray-700 flex items-center gap-2" onClick={() => setSidebarOpen(true)}Otherwise, this mobile toggle implementation looks good with proper accessibility (
aria-label).src/components/layout/Sidebar.jsx (1)
110-112: Missing backdrop overlay for mobile sidebar.The mobile sidebar uses
fixed inset-0 z-50positioning but only occupiesw-72of the screen, leaving the rest of the viewport unhandled. Users have no way to dismiss the sidebar by tapping outside it — a standard mobile UX pattern.Consider adding a backdrop overlay:
+ const backdropClasses = isMobile && open + ? 'fixed inset-0 z-40 bg-black/30' + : 'hidden'; + return ( + <> + {isMobile && open && ( + <div + className={backdropClasses} + onClick={() => setOpen(false)} + aria-hidden="true" + /> + )} <aside className={sidebarClasses} style={{ minHeight: '100vh' }} >This provides visual context and allows tap-to-dismiss.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/App.jsx(4 hunks)src/components/layout/Sidebar.jsx(5 hunks)src/components/layout/UseIsMobile.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/layout/UseIsMobile.jsx (2)
src/App.jsx (1)
isMobile(54-54)src/components/layout/Sidebar.jsx (1)
isMobile(8-8)
src/components/layout/Sidebar.jsx (4)
src/hooks/usePage.js (2)
usePage(4-4)usePage(4-4)src/pages/PlaceGeographyPage.jsx (1)
usePage(19-19)src/App.jsx (3)
isMobile(54-54)page(50-50)sidebarOpen(53-53)src/components/layout/UseIsMobile.jsx (2)
isMobile(4-4)useIsMobile(3-16)
src/App.jsx (2)
src/components/layout/Sidebar.jsx (1)
isMobile(8-8)src/components/layout/UseIsMobile.jsx (2)
isMobile(4-4)useIsMobile(3-16)
🔇 Additional comments (3)
src/components/layout/Sidebar.jsx (3)
8-12: Good mobile UX: Auto-close sidebar on navigation.The
handleNavfunction properly closes the sidebar after navigation on mobile devices, which is the expected behavior for mobile drawer patterns.
26-45: NavLink rendering logic looks correct.The
isExpanded = open || isMobilelogic correctly shows labels when either the sidebar is open (desktop) or on mobile (where the sidebar is a full drawer). ThehandleNavintegration properly handles mobile dismissal.
50-50: SubMenu expansion logic is consistent.The
isExpanded = sidebarOpen || isMobilemirrors the NavLink logic, ensuring consistent behavior across navigation elements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/components/modals/PlaceActionModal.jsx (3)
9-21: Duplicate ESC key handler — already provided by theModalcomponent.The
Modalcomponent (lines 18-32 inModal.jsx) already implements ESC key handling. This effect is redundant and can be removed.🔎 Proposed fix
- // ESC 키로 닫기 - useEffect(() => { - const handleKeyPress = (e) => { - if (e.key === 'Escape') { - onClose(); - } - }; - - document.addEventListener('keydown', handleKeyPress); - return () => { - document.removeEventListener('keydown', handleKeyPress); - }; - }, [onClose]); -
6-7: Unused state:coordinatesandloadingCoordinatesare never used.These values are fetched but never displayed or passed anywhere. If this is preparation for future use, consider removing until needed. If it's meant to display current coordinates, the UI is missing.
Also applies to: 23-66
107-136: Consider adding defensive null checks for consistency.
onImageManagehas a null check (line 95), butonCoordinateEdit,onEdit, andonDeletedo not. While these are always provided inPlacePage.jsx, adding guards would make the component more defensive and reusable.🔎 Proposed fix for onCoordinateEdit
- <button + {onCoordinateEdit && ( + <button onClick={() => { onCoordinateEdit(place); onClose(); }} className="w-full bg-indigo-50 hover:bg-indigo-100 text-indigo-700 font-semibold py-4 px-4 rounded-lg transition-all duration-200 flex items-center justify-center gap-2" > <i className="fas fa-map-marker-alt"></i> 좌표 수정 </button> + )}src/pages/PlacePage.jsx (3)
17-17: MultipleuseIsMobile()calls create redundant media query listeners.
useIsMobile()is called in bothMainPlaceCardandOtherPlaceCard, plusPlacePage. Each call creates a separatematchMedialistener. Consider passingisMobileas a prop fromPlacePageto avoid redundant listeners.🔎 Proposed approach
-const MainPlaceCard = ({ place, onEdit, onDelete, onImageManage, onCoordinateEdit, showToast, onMobileClick }) => { +const MainPlaceCard = ({ place, onEdit, onDelete, onImageManage, onCoordinateEdit, showToast, onMobileClick, isMobile }) => { const [isHovered, setIsHovered] = useState(false); const [imageLoading, setImageLoading] = useState(true); - const isMobile = useIsMobile();Then pass
isMobile={isMobile}fromPlacePagewhere the hook is already called (line 299).Also applies to: 186-186
32-40: DuplicatehandleCardClickimplementations in both card components.The logic in
MainPlaceCard(lines 32-40) andOtherPlaceCard(lines 205-213) is identical. Consider extracting to a shared helper or using a higher-order component.🔎 Proposed helper
// At the top of the file or in a utils file const createMobileCardClickHandler = (isMobile, onMobileClick, place) => (e) => { if (isMobile && onMobileClick && !e.target.closest('button')) { onMobileClick(place); } };Also applies to: 205-213
883-895: LGTM! Mobile modal integration is correct.The modal is properly controlled by state and cleanup resets both
mobileActionModalOpenandselectedPlaceForMobile.Minor: The
onImageManageconditional (line 888) duplicates theisMainPlacelogic from line 815. Consider extracting this category check to a helper function for consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/common/Modal.jsx(3 hunks)src/components/modals/PlaceActionModal.jsx(1 hunks)src/pages/PlacePage.jsx(13 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/modals/PlaceActionModal.jsx (2)
src/utils/api.js (1)
api(5-10)src/components/common/Modal.jsx (1)
Modal(3-77)
src/pages/PlacePage.jsx (1)
src/components/layout/UseIsMobile.jsx (1)
useIsMobile(3-16)
🔇 Additional comments (3)
src/components/common/Modal.jsx (2)
18-32: LGTM! ESC key handler is well-implemented.The effect properly guards against
!isOpen, includes cleanup, and has correct dependencies.
41-46: LGTM! Backdrop click handler is correctly implemented.The
containscheck ensures clicks on modal content don't trigger close, and thecloseOnBackdropClickguard provides opt-in behavior.src/pages/PlacePage.jsx (1)
297-299: LGTM!State management for the mobile modal flow is clean and straightforward.
Also applies to: 506-510
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/pages/PlaceGeographyPage.jsx (4)
327-335: Consider clearing the timeout on cleanup.If the component unmounts within 100ms, the timeout callback could attempt to access a stale ref. While unlikely to cause visible issues, it's good practice to clear pending timeouts.
🔎 Proposed fix
useEffect(() => { + let timeoutId; if (mapInstanceRef.current && window.naver) { - setTimeout(() => { + timeoutId = setTimeout(() => { window.naver.maps.Event.trigger(mapInstanceRef.current, 'resize'); }, 100); } + return () => { + if (timeoutId) clearTimeout(timeoutId); + }; }, [isMobile]);
337-357: Direct DOM manipulation of parent element creates tight coupling.Querying and modifying the
<main>element from a child page component introduces implicit dependencies on the layout structure. If the layout changes or the selector fails, this code silently does nothing.Consider alternatives:
- Pass a callback/context from the layout to allow pages to request scroll-lock
- Use a CSS class on
bodyor the layout root that child components can toggle- Use a dedicated modal/overlay library that handles body scroll-lock
The current implementation works but is fragile and harder to maintain.
404-412: Redundant ternary check on line 411.In the else branch of
style={isMobile ? {...} : {...}},isMobileis guaranteed to befalse, soheight: isMobile ? '100%' : 'calc(100% - 64px)'will always resolve to'calc(100% - 64px)'.🔎 Proposed fix
- style={isMobile ? { - flex: 1, - overflow: 'hidden', - paddingTop: '8px', - paddingBottom: '8px' - } : { height: isMobile ? '100%' : 'calc(100% - 64px)' }} + style={isMobile ? { + flex: 1, + overflow: 'hidden', + paddingTop: '8px', + paddingBottom: '8px' + } : { height: 'calc(100% - 64px)' }}
388-402: Consider extracting mobile styles for maintainability.The inline style objects with conditional mobile/desktop logic are becoming complex. Consider extracting these into named style objects or CSS classes to improve readability and maintainability.
// Example: extract at component top const mobileContainerStyle = { height: '100vh', boxSizing: 'border-box', overflow: 'hidden', position: 'fixed', top: 0, left: 0, right: 0, bottom: 0, width: '100%' }; const desktopContainerStyle = { height: '100%', boxSizing: 'border-box' };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/PlaceGeographyPage.jsx(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/PlaceGeographyPage.jsx (1)
src/components/layout/UseIsMobile.jsx (1)
useIsMobile(3-16)
🔇 Additional comments (4)
src/pages/PlaceGeographyPage.jsx (4)
9-9: LGTM!The
useIsMobilehook is imported correctly and used at the component's top level, following React's rules of hooks.Also applies to: 22-22
299-311: LGTM!Clean separation of desktop resize logic with proper early return for mobile. The dependency array correctly includes
isMobile.
414-465: LGTM!The mobile layout with 40%/55% height split for map/list containers is reasonable, leaving room for gaps and the title. The conditional overflow handling is appropriate for mobile scroll containment.
553-565: Button click propagates to parent, triggering map pan/zoom.Clicking the "좌표 수정/설정" button will also invoke the parent div's
onClickhandler via event propagation, causinghandlePlaceClick(booth)to run. This centers the map on the marker and zooms in, in addition to opening the modal.If this is intentional, consider adding a brief comment. If not, add
e.stopPropagation():🔎 Proposed fix if propagation should be stopped
<button className={`font-bold py-2 px-4 rounded-lg text-sm ${ hasCoordinates ? 'bg-blue-200 hover:bg-blue-300 text-blue-800' : 'bg-red-200 hover:bg-red-300 text-red-800' }`} - onClick={() => { + onClick={(e) => { + e.stopPropagation(); setSelectedPlace(booth); setModalOpen(true); }}
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 (3)
src/components/modals/PlaceEditModal.jsx (1)
142-155: Remove duplicate ESC key handler.The Modal component now handles ESC key presses internally (as shown in src/components/common/Modal.jsx lines 29-43). This local ESC handler duplicates that functionality and should be removed to avoid redundant event listeners.
🔎 Proposed fix
- // ESC 키 이벤트 리스너 - useEffect(() => { - const handleEscKey = (event) => { - if (event.key === 'Escape') { - onClose(); - } - }; - - document.addEventListener('keydown', handleEscKey); - - return () => { - document.removeEventListener('keydown', handleEscKey); - }; - }, [onClose]); -src/components/modals/PlaceImagesModal.jsx (1)
17-35: Remove duplicate ESC key handler.The Modal component now provides built-in ESC key handling (src/components/common/Modal.jsx lines 29-43). This local handler creates redundant event listeners and should be removed.
🔎 Proposed fix
- // ESC 키 이벤트 리스너 - useEffect(() => { - const handleEscKey = (event) => { - if (event.key === 'Escape') { - if (selectedImage) { - handleCloseDetail(); - } else if (isReorderMode || isDeleteMode) { - handleCancelMode(); - } else { - onClose(); - } - } - }; - - document.addEventListener('keydown', handleEscKey); - return () => { - document.removeEventListener('keydown', handleEscKey); - }; - }, [selectedImage, isReorderMode, isDeleteMode, onClose]); -Note: If you need custom ESC behavior (e.g., closing detail view or canceling modes first), consider moving this logic into a ref-based onClose handler passed to Modal instead of adding a separate listener.
src/components/modals/OtherPlaceEditModal.jsx (1)
43-55: Remove duplicate ESC key handler.The Modal component now handles ESC key presses internally (src/components/common/Modal.jsx lines 29-43). Remove this duplicate handler to avoid redundant event listeners.
🔎 Proposed fix
- // ESC 키 이벤트 리스너 - useEffect(() => { - const handleEscKey = (event) => { - if (event.key === 'Escape') { - onClose(); - } - }; - - document.addEventListener('keydown', handleEscKey); - return () => { - document.removeEventListener('keydown', handleEscKey); - }; - }, [onClose]); -
🧹 Nitpick comments (1)
src/pages/PlaceGeographyPage.jsx (1)
337-357: Consider risks of direct DOM manipulation on main element.This effect directly modifies the main element's
overflowandheightstyles on mobile. While the cleanup function restores original values, this approach could conflict if:
- Other code also modifies these styles
- The component unmounts unexpectedly
- Multiple instances run simultaneously (unlikely here)
Consider alternative approaches like:
- Using a portal for the mobile layout
- Applying mobile-specific classes to main via a context/prop
- Documenting this behavior for maintainability
The current implementation works correctly with proper cleanup, but the above approaches would be more maintainable long-term.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/common/Modal.jsx(2 hunks)src/components/modals/OtherPlaceEditModal.jsx(5 hunks)src/components/modals/PlaceActionModal.jsx(1 hunks)src/components/modals/PlaceEditModal.jsx(3 hunks)src/components/modals/PlaceImagesModal.jsx(4 hunks)src/pages/PlaceGeographyPage.jsx(8 hunks)src/pages/PlacePage.jsx(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/components/modals/OtherPlaceEditModal.jsx (3)
src/components/common/Modal.jsx (1)
isMobile(18-18)src/components/modals/PlaceImagesModal.jsx (1)
isMobile(15-15)src/components/layout/UseIsMobile.jsx (1)
useIsMobile(3-16)
src/components/modals/PlaceActionModal.jsx (3)
src/utils/api.js (1)
api(5-10)src/pages/PlacePage.jsx (1)
mainImage(29-29)src/components/common/Modal.jsx (1)
Modal(4-95)
src/components/common/Modal.jsx (1)
src/components/layout/UseIsMobile.jsx (1)
useIsMobile(3-16)
src/pages/PlacePage.jsx (3)
src/components/common/Modal.jsx (2)
isMobile(18-18)Modal(4-95)src/components/modals/PlaceImagesModal.jsx (1)
isMobile(15-15)src/components/layout/UseIsMobile.jsx (1)
useIsMobile(3-16)
src/components/modals/PlaceImagesModal.jsx (2)
src/components/common/Modal.jsx (2)
isMobile(18-18)Modal(4-95)src/components/layout/UseIsMobile.jsx (1)
useIsMobile(3-16)
src/components/modals/PlaceEditModal.jsx (2)
src/components/common/Modal.jsx (2)
isMobile(18-18)Modal(4-95)src/components/layout/UseIsMobile.jsx (1)
useIsMobile(3-16)
src/pages/PlaceGeographyPage.jsx (2)
src/components/common/Modal.jsx (2)
isMobile(18-18)Modal(4-95)src/components/layout/UseIsMobile.jsx (1)
useIsMobile(3-16)
🔇 Additional comments (9)
src/components/modals/PlaceEditModal.jsx (1)
4-4: LGTM! Mobile-aware modal integration looks good.The useIsMobile hook integration and enableMobileLayout usage correctly adapt the modal for mobile devices. The conditional wrapper with adjusted padding and typography provides appropriate mobile UX.
Also applies to: 21-21, 274-281
src/components/modals/PlaceImagesModal.jsx (1)
5-5: LGTM! Mobile layout adaptations are well-implemented.The mobile-aware changes appropriately adjust modal sizing, button layouts (horizontal → vertical stacking), and padding for mobile devices. The responsive button group provides better UX on smaller screens.
Also applies to: 15-15, 343-352, 493-506
src/components/modals/OtherPlaceEditModal.jsx (1)
4-4: LGTM! Mobile layout integration is clean.The useIsMobile hook integration and conditional layout adjustments provide appropriate mobile responsiveness. Button stacking on mobile improves usability.
Also applies to: 15-15, 119-121, 163-183
src/components/common/Modal.jsx (1)
2-2: LGTM! Modal enhancements improve mobile UX and accessibility.The additions are well-implemented:
- ESC key support improves keyboard accessibility
enableMobileLayoutandcloseOnBackdropClickprops provide flexible mobile behavior- Mobile-specific sizing and padding classes adapt appropriately
- Backdrop click handling respects the conditional prop
These changes centralize modal behavior and eliminate the need for individual components to implement ESC handlers.
Also applies to: 4-13, 18-18, 29-43, 52-66, 73-73, 77-77
src/components/modals/PlaceActionModal.jsx (2)
23-66: LGTM! Coordinate fetching with proper fallbacks and error handling.The logic correctly:
- Checks place.markerCoordinate first
- Falls back to place.coordinate
- Finally fetches from API if needed
- Handles errors gracefully with console logging
- Ensures setLoadingCoordinates(false) is always called in the finally block
70-146: LGTM! Modal integration and action buttons are well-structured.The component properly:
- Uses enableMobileLayout and closeOnBackdropClick for mobile UX
- Displays the main image with a fallback placeholder
- Provides clear action buttons (image management, coordinate edit, edit, delete)
- Closes the modal after each action
src/pages/PlaceGeographyPage.jsx (1)
9-9: LGTM! Mobile layout adaptations are comprehensive and well-executed.The mobile-specific changes properly:
- Adjust map and list container heights (40%/55% split)
- Lock main element scrolling on mobile
- Trigger map resize when layout changes
- Apply appropriate mobile styles and overflow handling
- Enable mobile layout for coordinate modal
The implementation maintains desktop functionality while providing a mobile-optimized experience.
Also applies to: 22-22, 299-335, 389-466, 592-597
src/pages/PlacePage.jsx (2)
9-10: LGTM! Mobile interaction pattern is well-designed and implemented.The changes effectively:
- Introduce mobile-specific card click behavior via onMobileClick
- Gate desktop hover actions with !isMobile checks to avoid conflicts
- Open PlaceActionModal on mobile card taps (excluding button clicks)
- Pass consistent props to MainPlaceCard and OtherPlaceCard
- Manage mobile modal state cleanly with proper cleanup
The desktop and mobile interaction patterns are cleanly separated, providing optimal UX for each context.
Also applies to: 14-48, 76-126, 297-299, 507-510, 826-827, 836-837, 888-900
865-887: LGTM! Coordinate modal properly uses mobile layout.The modal correctly applies
enableMobileLayout={true}for responsive behavior on mobile devices.
| // ESC 키로 닫기 | ||
| useEffect(() => { | ||
| const handleKeyPress = (e) => { | ||
| if (e.key === 'Escape') { | ||
| onClose(); | ||
| } | ||
| }; | ||
|
|
||
| document.addEventListener('keydown', handleKeyPress); | ||
| return () => { | ||
| document.removeEventListener('keydown', handleKeyPress); | ||
| }; | ||
| }, [onClose]); |
There was a problem hiding this comment.
Remove duplicate ESC key handler.
The Modal component now handles ESC key presses internally (src/components/common/Modal.jsx lines 29-43). This handler is redundant since the Modal already calls onClose on ESC.
🔎 Proposed fix
- // ESC 키로 닫기
- useEffect(() => {
- const handleKeyPress = (e) => {
- if (e.key === 'Escape') {
- onClose();
- }
- };
-
- document.addEventListener('keydown', handleKeyPress);
- return () => {
- document.removeEventListener('keydown', handleKeyPress);
- };
- }, [onClose]);
-📝 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.
| // ESC 키로 닫기 | |
| useEffect(() => { | |
| const handleKeyPress = (e) => { | |
| if (e.key === 'Escape') { | |
| onClose(); | |
| } | |
| }; | |
| document.addEventListener('keydown', handleKeyPress); | |
| return () => { | |
| document.removeEventListener('keydown', handleKeyPress); | |
| }; | |
| }, [onClose]); |
🤖 Prompt for AI Agents
In src/components/modals/PlaceActionModal.jsx around lines 9 to 21, there is a
redundant useEffect that listens for the Escape key and calls onClose; since the
Modal component now handles ESC internally, remove this entire useEffect block
(the handler, addEventListener, and cleanup removeEventListener) so the
component no longer registers its own keydown listener and relies on the Modal's
onClose behavior.
DEV -> PROD 반영
Summary by CodeRabbit
New Features
Behavior
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.