[PR] Refactor: 네이버지도로 마이그레이션 #41
Conversation
Walkthrough이번 PR은 Naver Maps API와 관련된 여러 변경 사항을 포함합니다.
Changes
Sequence Diagram(s)sequenceDiagram
participant MP as MainPage/Meeting Page
participant DOM as mapRef/Div 컨테이너
participant NM as Naver Map API
MP->>DOM: 지도 컨테이너 참조 획득
MP->>NM: 지도 인스턴스 초기화 (중심 좌표, 줌 설정)
NM-->>MP: 초기화된 지도 반환
MP->>Marker: Naver Map 인스턴스 전달 (마커 렌더링)
sequenceDiagram
participant EM as EventMarker Component
participant NM as Naver Marker
participant RN as ReactDOMServer
participant IW as InfoWindow
EM->>NM: Marker 생성 및 지도에 추가
NM->>RN: OverCard 컴포넌트 렌더링 (renderToString)
RN-->>IW: InfoWindow 콘텐츠 반환
NM->>EM: 이벤트 리스너(클릭 등) 연결
EM->>IW: 클릭 시 InfoWindow 표시
Note over EM: cleanup 시 Marker 제거 및 InfoWindow 닫힘
Possibly related issues
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
src/components/main/OverCard.tsx (1)
72-77: 닫기 버튼의 기능이 구현되지 않았습니다.닫기 버튼이 있지만 실제로 동작하지 않습니다. 네이버 지도의 InfoWindow와 연동이 필요합니다.
package.json (1)
21-21: 의존성 정리가 필요합니다.네이버 지도로 마이그레이션이 진행 중이지만, Kakao Maps SDK가 여전히 의존성으로 남아있습니다.
- "react-kakao-maps-sdk": "^1.1.21",Also applies to: 54-54
🧹 Nitpick comments (7)
src/pages/MainPage.tsx (2)
8-9: Naver 지도 객체를 State로 관리하는 방안 재검토 권장
naverMap을State로 두는 것은 뷰 로직과 결합되어 재렌더링이 필요할 때 유용하지만, 지도 객체 자체는 화면에 직접 렌더링되지 않으므로useRef만으로도 충분할 수 있습니다. 필요한 경우에만 State로 관리하여 불필요한 렌더링을 피할 수 있게끔 설계를 고려해 보세요.
19-37: 초기화와 정리(cleanup) 흐름 적절
Naver 지도 생성 시 DOM 레퍼런스를 확인하고,map?.destroy()를 통해 리소스를 정리하는 로직이 잘 작성되었습니다. 에러나 예외가 발생할 수 있는 부분을 대비하여,window.naver접근 시 try/catch 등으로 안정성을 높이는 방안도 검토해 보세요.src/components/main/EventMarker.tsx (2)
13-99: 마커 및 InfoWindow 관리 로직 개선 제안
현재document.querySelector를 통해.bg-white.h-fit.cursor-pointer등을 선택하는 방식은 스타일 클래스 변경 시 쉽게 깨질 수 있다는 점에서 유지보수가 어려울 수 있습니다. 별도의 식별자를 두거나 InfoWindow 내부 엘리먼트와 직접적인 이벤트 바인딩을 고려해 보시면 좋겠습니다.
104-104: React.memo 사용 용도 점검
마커 로직 자체가 복잡하지 않고, 자식 컴포넌트 구조도 없어 리렌더링 빈도가 낮다면React.memo의 이점이 적을 수도 있습니다. 다만, 확장성을 위해 유지하는 것도 무방합니다.src/pages/meeting/CreateMeetingBasicPage.tsx (2)
18-22: 전역 타입 선언 별도 파일로 분리 권장
declare global { interface Window { naver: any; } }구문은 프로젝트 규모가 커질수록 헷갈리기 쉽습니다. 별도의.d.ts파일로 분리해 관리하면 전역 타입이 명확해집니다.
88-95: 사용자 위치 변경에 따른 마커 위치 동기화
사용자 위치가 변경될 때 마커와 지도 중심을 재설정하는 로직은 매끄럽습니다. 반복 호출될 여지가 있다면,debounce나throttle사용을 검토해 볼 수도 있습니다.src/components/main/OverCard.tsx (1)
33-36: 장소 표시 로직이 개선되었습니다.정규식을 사용하여 괄호 안의 내용을 추출하는 방식이 적절합니다. 하지만 매칭이 실패할 경우의 폴백(fallback) 처리가 더 명시적이면 좋겠습니다.
- const displayContent = isPlace ? (content.match(/\(([^)]+)\)/) ?? [null, content])[1] : content; + const displayContent = isPlace + ? content.match(/\(([^)]+)\)/)?.[1] || content + : content;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.json(1 hunks)public/index.html(1 hunks)src/components/main/EventMarker.tsx(1 hunks)src/components/main/OverCard.tsx(2 hunks)src/pages/MainPage.tsx(2 hunks)src/pages/meeting/CreateMeetingBasicPage.tsx(4 hunks)
🔇 Additional comments (8)
src/pages/MainPage.tsx (2)
14-14: useMemo 활용 적절
data?.data.content가 자주 변하지 않는다면,useMemo로 불필요한 재계산을 막는 구현은 적절해 보입니다.
53-65: 지도 영역과 Marker 렌더링 구조 적절
mapRef에 지도를 표시하고,naverMap객체가 존재할 때만 마커를 생성하는 흐름이 명확하게 분리되어 있습니다. 마커들에key값을 지정해 주어 React 렌더링 시 안정성도 확보된 점이 인상적입니다.src/components/main/EventMarker.tsx (2)
1-11: 전반적인 인터페이스 설계 적절
map: naver.maps.Map;을 명시적으로 타입 선언한 것은 가독성과 유지보수성에 도움이 됩니다.ReactDOMServer.renderToString을 통한 InfoWindow 콘텐츠 렌더링 또한 Naver 지도의 한계를 우회하는 데 적합한 방법입니다.
101-102: JSX 렌더링 생략 의도 명확
마커만 생성하고 JSX를 렌더링하지 않으므로return null;처리가 타당합니다.src/pages/meeting/CreateMeetingBasicPage.tsx (3)
28-31: mapRef, naverMap, marker 선언 구조 양호
지도를 그릴 DOM 레퍼런스와 지도/마커 상태값을 분리 선언하여 가독성이 좋습니다. 단, 마커 재사용 범위를 검토해 볼 여지는 있습니다.
60-86: 지도 초기화 로직 적절
컴포넌트 마운트 시점에 Naver 지도를 생성하고, 언마운트 시점에 마커와 지도를 해제(destroy)하는 로직이 잘 작성되었습니다. 위치 정보가 없을 때 조건문으로 빠르게 반환하는 방식도 안정적입니다.
271-275: 지도를 위한 DOM 컨테이너 구성
<div ref={mapRef} />를 통해 직접 지도를 렌더링하는 구조가 간단명료합니다. CSS 스타일 적용 및 크기 관리를 이 레퍼런스 기반으로 하여 직관적인 지도가 가능합니다.src/components/main/OverCard.tsx (1)
13-15: Props 인터페이스가 단순화되었습니다.
onClose핸들러가 제거되어 컴포넌트의 책임이 더 명확해졌습니다.
| <script src="//t1.daumcdn.net/mapjsapi/bundle/postcode/prod/postcode.v2.js"></script> | ||
| <!--Kakao Map API--> | ||
| <script type="text/javascript" src="//dapi.kakao.com/v2/maps/sdk.js?appkey=%REACT_APP_KAKAOMAP_API_KEY%&libraries=services"></script> | ||
| <!-- Naver Map API --> | ||
| <script type="text/javascript" src="https://openapi.map.naver.com/openapi/v3/maps.js?ncpClientId=%REACT_APP_NAVER_MAP_CLIENT_ID%&submodules=geocoder"></script> |
There was a problem hiding this comment.
스크립트 의존성 정리가 필요합니다.
Daum 주소 검색 기능은 유지하되, Kakao Maps API 스크립트는 제거해야 합니다. PR 설명에 따르면 Daum의 주소 검색 기능은 계속 사용할 예정이기 때문입니다.
<script src="//t1.daumcdn.net/mapjsapi/bundle/postcode/prod/postcode.v2.js"></script>
-<!--Kakao Map API-->
-<script type="text/javascript" src="//dapi.kakao.com/v2/maps/sdk.js?appkey=%REACT_APP_KAKAOMAP_API_KEY%&libraries=services"></script>📝 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.
| <script src="//t1.daumcdn.net/mapjsapi/bundle/postcode/prod/postcode.v2.js"></script> | |
| <!--Kakao Map API--> | |
| <script type="text/javascript" src="//dapi.kakao.com/v2/maps/sdk.js?appkey=%REACT_APP_KAKAOMAP_API_KEY%&libraries=services"></script> | |
| <!-- Naver Map API --> | |
| <script type="text/javascript" src="https://openapi.map.naver.com/openapi/v3/maps.js?ncpClientId=%REACT_APP_NAVER_MAP_CLIENT_ID%&submodules=geocoder"></script> | |
| <script src="//t1.daumcdn.net/mapjsapi/bundle/postcode/prod/postcode.v2.js"></script> | |
| <!-- Naver Map API --> | |
| <script type="text/javascript" src="https://openapi.map.naver.com/openapi/v3/maps.js?ncpClientId=%REACT_APP_NAVER_MAP_CLIENT_ID%&submodules=geocoder"></script> |
src/components/main/OverCard.tsx
Outdated
| <ShotInform title="관심" content={meeting.likeDto.likeCount.toString()} /> | ||
| <ShotInform title="인원" content={meeting.maxParticipants.toString()} unit="명" /> | ||
| <ShotInform title="기간" content={toKoreanDuration(meeting.duration)} /> | ||
| <ShotInform title="마감" content={meeting.endDate} /> | ||
| <div className="col-span-2"> | ||
| <ShotInform title="스택" content={techStacks.join(", ")} /> | ||
| <ShotInform title="스택" content={meeting.techStacks.join(", ")} /> | ||
| </div> | ||
| <div className="col-span-2"> | ||
| <ShotInform title="장소" content={location} /> | ||
| <ShotInform title="장소" content={meeting.location} /> |
There was a problem hiding this comment.
물론 취향차이지만 계속 meeting.으로 접근하고 있는데 구조분해할당을 미리 하고 사용해도 좋았을 것 같아요!!
There was a problem hiding this comment.
좋아용!!! 빼먹었는데 알려줘서 고마우어용 😄
src/components/main/EventMarker.tsx
Outdated
| // 컨텐츠 영역을 누른 경우, 상세 페이지로 이동 | ||
| const contentDiv = document.querySelector('.bg-white.h-fit.cursor-pointer'); | ||
| if (contentDiv) { |
There was a problem hiding this comment.
tailwind를 처음 써보는데 그러면 이렇게 쿼리셀렉터를 길게 주게 되는건가용???
가독성이 너무 떨어져 보여유......
There was a problem hiding this comment.
키키 궁금해서 찾아봤어용
방법이 2가지가 있다네용
- Portal 사용하기
ReactDOMServer.renderToString()을 사용해서 OverCard를 문자열로 변환 후 infoWindow에 넣고 있지만, React Portal을 사용하면 직접 React 컴포넌트로 관리할 수 있다네용
대충 요론 느낌?
{isOpen &&
createPortal(
<div
className="absolute bg-white shadow-lg rounded-md p-4 cursor-pointer"
style={{
position: 'absolute',
top: `${position.latitude}px`,
left: `${position.longitude}px`,
zIndex: 1000
}}
onClick={() => (window.location.href = `/meeting/detail/${meeting.id}`)}
>
<OverCard meeting={meeting} />
<button
className="absolute top-2 right-2 text-gray-600"
onClick={(e) => {
e.stopPropagation();
setIsOpen(false);
}}
>
x
</button>
</div>,
document.body
)}이 방식은 더 React스러운데 수정하기에는 좀 공사같기는하네용
- data-* 사용하기
<div data-overcard className="bg-white h-fit cursor-pointer">const contentDiv = document.querySelector('[data-overcard]');이 방식이 더 수정하기 깔끔할 것 같아용
There was a problem hiding this comment.
어머 덕분에 좋은 거 새로 배우네용..!
Portal 방식이 더 좋아보이지만.. 나중에 리팩토링 기간 있으니 조금만 나중에 하겠습니당 ㅋㅋㅋ
지금은 알려준 data-* 방식으로 코드 수정할게용
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/main/OverCard.tsx (1)
62-65: 접근성 개선이 필요합니다.InfoWindow의 내용을 나타내는 div에 적절한 ARIA 레이블이 누락되어 있습니다.
다음과 같이 ARIA 레이블을 추가하는 것을 추천드립니다:
<div data-overcard + role="dialog" + aria-label={`${title} 모임 정보`} className="bg-white h-fit cursor-pointer box-content" style={{ width: 'max-content' }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/main/EventMarker.tsx(1 hunks)src/components/main/OverCard.tsx(4 hunks)
🔇 Additional comments (5)
src/components/main/EventMarker.tsx (3)
16-20: 마커 초기화 로직이 잘 구현되었습니다.마커 생성 및 설정이 깔끔하게 구현되어 있습니다.
92-97: 클린업 로직이 잘 구현되었습니다.컴포넌트 언마운트 시 마커와 InfoWindow를 적절히 정리하고 있습니다.
102-102: React.memo 사용이 적절합니다.불필요한 리렌더링을 방지하기 위한 React.memo 적용이 적절합니다.
src/components/main/OverCard.tsx (2)
48-59: 구조 분해 할당이 잘 적용되었습니다.이전 리뷰 코멘트를 반영하여 meeting 객체의 구조 분해 할당이 잘 구현되었습니다.
84-90: 닫기 버튼의 접근성이 개선되었습니다.닫기 버튼에 aria-label이 적절히 추가되어 있습니다.
| setTimeout(() => { | ||
| // 컨텐츠 영역을 누른 경우, 상세 페이지로 이동 | ||
| const contentDiv = document.querySelector('[data-overcard]'); | ||
| if (contentDiv) { | ||
| contentDiv.addEventListener('click', (e) => { | ||
| const target = e.target as HTMLElement; | ||
| if (!target.closest('[aria-label="Close"]')) { | ||
| window.location.href = `/meeting/detail/${meeting.id}`; | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| const handleMarkerClick = () => { | ||
| setMarkerState(prev => ({ | ||
| ...prev, | ||
| isClicked: !prev.isClicked | ||
| })); | ||
| }; | ||
| // 닫기 버튼을 누른 경우, InfoWindow 닫음 | ||
| const closeButton = document.querySelector('[data-button="close"]'); | ||
| if (closeButton) { | ||
| closeButton.addEventListener('click', (e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| closeInfoWindow(); | ||
| }); | ||
| } | ||
| }, 0); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
setTimeout 사용에 대한 개선이 필요합니다.
setTimeout을 사용하여 DOM 요소를 찾는 현재 구현은 다음과 같은 잠재적인 문제가 있습니다:
- 레이스 컨디션 발생 가능성
- 불필요한 지연 발생
- DOM 조작의 신뢰성 저하
MutationObserver를 사용하여 더 안정적으로 구현하는 것을 추천드립니다:
- setTimeout(() => {
+ const observer = new MutationObserver((mutations, obs) => {
const contentDiv = document.querySelector('[data-overcard]');
const closeButton = document.querySelector('[data-button="close"]');
+ if (contentDiv && closeButton) {
contentDiv.addEventListener('click', (e) => {
const target = e.target as HTMLElement;
if (!target.closest('[aria-label="Close"]')) {
window.location.href = `/meeting/detail/${meeting.id}`;
}
});
- }
- if (closeButton) {
closeButton.addEventListener('click', (e) => {
e.preventDefault();
e.stopPropagation();
closeInfoWindow();
});
+ obs.disconnect();
}
- }, 0);
+ });
+
+ observer.observe(document.body, {
+ childList: true,
+ subtree: true
+ });📝 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.
| setTimeout(() => { | |
| // 컨텐츠 영역을 누른 경우, 상세 페이지로 이동 | |
| const contentDiv = document.querySelector('[data-overcard]'); | |
| if (contentDiv) { | |
| contentDiv.addEventListener('click', (e) => { | |
| const target = e.target as HTMLElement; | |
| if (!target.closest('[aria-label="Close"]')) { | |
| window.location.href = `/meeting/detail/${meeting.id}`; | |
| } | |
| }); | |
| } | |
| const handleMarkerClick = () => { | |
| setMarkerState(prev => ({ | |
| ...prev, | |
| isClicked: !prev.isClicked | |
| })); | |
| }; | |
| // 닫기 버튼을 누른 경우, InfoWindow 닫음 | |
| const closeButton = document.querySelector('[data-button="close"]'); | |
| if (closeButton) { | |
| closeButton.addEventListener('click', (e) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| closeInfoWindow(); | |
| }); | |
| } | |
| }, 0); | |
| }; | |
| const observer = new MutationObserver((mutations, obs) => { | |
| // 컨텐츠 영역을 누른 경우, 상세 페이지로 이동 | |
| const contentDiv = document.querySelector('[data-overcard]'); | |
| // 닫기 버튼을 누른 경우, InfoWindow 닫음 | |
| const closeButton = document.querySelector('[data-button="close"]'); | |
| if (contentDiv && closeButton) { | |
| contentDiv.addEventListener('click', (e) => { | |
| const target = e.target as HTMLElement; | |
| if (!target.closest('[aria-label="Close"]')) { | |
| window.location.href = `/meeting/detail/${meeting.id}`; | |
| } | |
| }); | |
| closeButton.addEventListener('click', (e) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| closeInfoWindow(); | |
| }); | |
| obs.disconnect(); | |
| } | |
| }); | |
| observer.observe(document.body, { | |
| childList: true, | |
| subtree: true | |
| }); |
개요
이슈 넘버
#39
PR 유형
어떤 변경 사항이 있나요?
의논할 부분
-> 주소 검색은 다음 지도 유지하도록 코드 변경 X
Summary by CodeRabbit
@types/navermaps가 추가되었습니다.