[feature] 동아리 상세페이지에 동아리방 위치를 지도에 표시한다#1367
Open
suhyun113 wants to merge 15 commits intodevelop-fefrom
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Map 컴포넌트·스타일 frontend/src/components/map/NaverMap.tsx, frontend/src/components/map/NaverMap.styles.ts |
NaverMap React 컴포넌트와 MapContainer styled-component 추가; ref 기반으로 렌더 영역 제공. |
스크립트 로더·훅 frontend/src/components/map/loadNaverMapScript.ts, frontend/src/components/map/useNaverMap.ts |
Naver Maps 스크립트 동적 로드(중복 검사) 함수 추가 및 비동기 지도/마커 초기화와 인터랙티브 옵션을 처리하는 훅 추가. |
클럽 위치 데이터 frontend/src/constants/clubLocation.ts |
ClubLocation 타입과 다수 클럽의 좌표·건물·상세위치 정보를 담은 읽기 전용 배열 clubLocations 추가. |
ClubDetailPage 및 스타일 frontend/src/pages/ClubDetailPage/ClubDetailPage.tsx, frontend/src/pages/ClubDetailPage/ClubDetailPage.styles.ts |
clubLocations 조회 후 일치 시 NaverMap 및 위치 정보 블록을 조건부 렌더; 레이아웃(LeftSection, MapInfo, MapCard 등) 추가. |
프로필 카드 변경 frontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.tsx, frontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.ts |
카드에 선택적 location·mapPath props 추가 및 태블릿 레이아웃·위치 관련 UI(링크 포함) 추가. |
클럽 맵 페이지 frontend/src/pages/ClubMapPage/ClubMapPage.tsx, frontend/src/pages/ClubMapPage/ClubMapPage.styles.ts |
전체 화면 지도 페이지 ClubMapPage 및 관련 스타일(백버튼, 하단 카드 등) 추가; 라우트로 연결됨. |
라우팅·인덱스·타입 frontend/src/App.tsx, frontend/src/index.tsx, frontend/src/types/window.d.ts |
신규 map 라우트(/clubDetail/:id/map, /webview/.../map) 등록, 개발용 window.navermap_authFailure 핸들러 등록(DEV 한정), 글로벌 Window에 naver 및 navermap_authFailure 선언 추가. |
Sequence Diagram
sequenceDiagram
participant User as User
participant ClubDetail as ClubDetailPage
participant NaverMap as NaverMap Component
participant Hook as useNaverMap Hook
participant Loader as loadNaverMapScript
participant API as Naver Maps API
User->>ClubDetail: 클럽 상세 페이지 방문
ClubDetail->>ClubDetail: `clubLocations`에서 위치 조회
alt 위치 발견
ClubDetail->>NaverMap: 위치 props로 렌더
NaverMap->>Hook: mapRef, lat, lng 전달
Hook->>Loader: 스크립트 로드 요청
alt 스크립트 이미 존재
Loader-->>Hook: 즉시 해결
else 스크립트 미존재
Loader->>API: 외부 스크립트 요청 (clientId 쿼리)
API-->>Loader: 스크립트 로드 완료
Loader-->>Hook: Promise 해결
end
Hook->>API: naver.maps 인스턴스 생성 및 마커 추가
API-->>NaverMap: 지도/마커 렌더링 완료
NaverMap->>User: 지도 표시
else 위치 미발견
ClubDetail->>User: 지도 및 위치 정보 미표시
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~35 minutes
Possibly related PRs
- [feature] 상세 페이지 모바일 탑바 웹뷰로 이관 및 리액트 네이티브 API 연동 구조 도입 #1090: ClubDetailPage 및 ClubProfileCard 관련 변경을 포함 — 위치/프로필 카드 변경과 코드 수준 연관성 높음.
- [release] FE v1.1.20 #1116: App 라우트 수정(클럽 상세/맵 경로 관련) — 신규 map 라우트 추가와 관련 있음.
- [release] FE v1.1.19 #1106: ClubMapPage/지도 관련 UI 및 웹뷰 연동 변경 — 지도 페이지와 하단 카드/내비게이션 처리 연관.
Suggested reviewers
- oesnuj
- seongwon030
- lepitaaar
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목은 Naver 지도 API 연동을 통해 동아리방 위치를 상세페이지에 표시하는 주요 변경사항을 명확하게 요약하고 있습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/#1357-club-detail-map-location-MOA-767
⚔️ Resolve merge conflicts
- Resolve merge conflict in branch
feature/#1357-club-detail-map-location-MOA-767
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 help to get the list of available commands and usage tips.
🎨 UI 변경사항을 확인해주세요
4개 스토리 변경 · 전체 56개 스토리 · 22개 컴포넌트 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.ts (1)
171-173:⚠️ Potential issue | 🔴 Critical구문 오류:
IntroSection의 닫는 중괄호가 누락되었습니다.
IntroSectionstyled-component의${media.tablet}블록에 닫는}가 없어 빌드 오류가 발생합니다.🐛 수정 제안
export const IntroSection = styled.section` padding: 16px; border-radius: 14px; background-color: ${colors.base.white}; ${media.tablet} { background-color: ${colors.gray[100]}; + } `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.ts` around lines 171 - 173, The IntroSection styled-component has a missing closing brace causing a syntax/build error; locate the IntroSection definition (the block containing ${media.tablet} and colors.gray[100]) and add the missing closing curly brace(s) to properly terminate the ${media.tablet} media query and the IntroSection styled-component so the CSS block parses correctly.
🧹 Nitpick comments (6)
frontend/src/constants/clubLocation.ts (1)
9-9: 상수 이름에 UPPER_SNAKE_CASE 사용을 고려해 주세요.코딩 가이드라인에 따르면 상수는
UPPER_SNAKE_CASE를 사용해야 합니다.clubLocations→CLUB_LOCATIONS로 변경하면 일관성이 향상됩니다.♻️ 상수 이름 변경 예시
-export const clubLocations = [ +export const CLUB_LOCATIONS = [ // ... ] as const;사용처도 함께 업데이트:
-import { clubLocations } from '@/constants/clubLocation'; +import { CLUB_LOCATIONS } from '@/constants/clubLocation'; -const location = clubLocations.find( +const location = CLUB_LOCATIONS.find(As per coding guidelines:
Use UPPER_SNAKE_CASE for constant names and centralize them in src/constants/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/constants/clubLocation.ts` at line 9, Rename the exported constant clubLocations to CLUB_LOCATIONS and update its export accordingly; then update all usages/imports of clubLocations across the codebase to reference CLUB_LOCATIONS (search for the symbol "clubLocations" and replace with "CLUB_LOCATIONS") and ensure it remains exported from src/constants/clubLocation.ts so other modules import the new name.frontend/src/types/window.d.ts (1)
4-5:naver에any대신 최소한의 타입 정의를 고려해 주세요.코딩 가이드라인에서
any타입 사용을 지양하도록 권장하고 있습니다. Naver Maps SDK에 공식 TypeScript 타입이 없으므로, 프로젝트에서 사용하는 API에 대해 최소한의 인터페이스를 정의하면 타입 안전성이 향상됩니다.♻️ 최소 타입 정의 예시
interface NaverMaps { Map: new (element: HTMLElement, options: object) => object; LatLng: new (lat: number, lng: number) => object; Marker: new (options: object) => object; // 필요한 추가 API... } interface Naver { maps: NaverMaps; } declare global { interface Window { Kakao: any; naver: Naver | undefined; navermap_authFailure: () => void; } }As per coding guidelines:
frontend/**/*.{ts,tsx}: Do not use 'any' type; use explicit type definitions instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/types/window.d.ts` around lines 4 - 5, Replace the broad any on window.naver with a minimal typed interface: define NaverMaps and Naver interfaces that cover the SDK pieces you use (e.g., Map, LatLng, Marker constructors and any methods/options you call) and change the Window declaration so naver is typed as Naver | undefined; keep navermap_authFailure as a function type. Update the types in frontend/src/types/window.d.ts by adding the NaverMaps and Naver interface names and using them in the global Window declaration instead of any to satisfy the lint rule against any.frontend/src/index.tsx (1)
8-12: SDK 초기화 로직을initSDK.ts로 이동하는 것을 고려해 주세요.학습된 패턴에 따르면 SDK 초기화는
src/utils/initSDK.ts에서 관리됩니다.navermap_authFailure핸들러도 해당 파일로 이동하면 일관성이 향상됩니다.♻️ initSDK.ts로 이동 예시
frontend/src/utils/initSDK.ts에 추가:export function initializeNaverMapAuthHandler() { if (import.meta.env.DEV) { window.navermap_authFailure = function () { console.error('Naver Map Error 인증 실패'); }; } }
frontend/src/index.tsx에서 호출:-if (import.meta.env.DEV) { - window.navermap_authFailure = function () { - console.error('Naver Map Error 인증 실패'); - }; -} +import { initializeNaverMapAuthHandler } from './utils/initSDK'; +initializeNaverMapAuthHandler();Based on learnings:
Manage all SDK initialization (Mixpanel, Sentry, Channel.io, Kakao) in src/utils/initSDK.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/index.tsx` around lines 8 - 12, Move the dev-only Naver Map auth failure handler out of frontend/src/index.tsx into the centralized SDK init module: create or use src/utils/initSDK.ts and add a function (e.g., initializeNaverMapAuthHandler) that registers window.navermap_authFailure in the same DEV-only conditional, then import and call that function from index.tsx as part of the SDK initialization sequence (alongside other init functions like Mixpanel/Sentry/Channel.io/Kakao) so the handler and SDK bootstrapping are managed in one place.frontend/src/components/map/NaverMap.tsx (1)
5-13: 사용되지 않는 props를 제거하거나 활용해 주세요.
NaverMapProps인터페이스에clubName,building,detailLocation이 정의되어 있지만 컴포넌트에서 사용되지 않습니다. 현재 필요하지 않다면 제거하고, 향후 마커 레이블이나 InfoWindow에 사용할 예정이라면 해당 기능을 구현하거나 TODO 주석을 추가해 주세요.♻️ 사용하지 않는 props 제거
interface NaverMapProps { lat: number; lng: number; - clubName: string; - building: string; - detailLocation: string; } -const NaverMap = ({ lat, lng }: NaverMapProps) => { +const NaverMap = ({ lat, lng }: NaverMapProps) => { const mapRef = useRef<HTMLDivElement | null>(null); useNaverMap(mapRef, lat, lng); return <Styled.MapContainer ref={mapRef} />; };또는
ClubDetailPage.tsx에서 호출 시에도 해당 props를 제거해야 합니다:<NaverMap - clubName={location.clubName} lat={location.lat} lng={location.lng} - building={location.building} - detailLocation={location.detailLocation} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/map/NaverMap.tsx` around lines 5 - 13, The NaverMapProps interface declares clubName, building, and detailLocation but the NaverMap component signature and body only use lat and lng; either remove those unused props from NaverMapProps and from the NaverMap call sites (e.g., where ClubDetailPage calls NaverMap) or update NaverMap to accept and use clubName/building/detailLocation (for example to render a marker label or InfoWindow) and add a TODO comment if you prefer to defer UI work; update the interface, the NaverMap function parameters, and all places that construct NaverMap props (notably ClubDetailPage) to keep them in sync.frontend/src/pages/ClubDetailPage/ClubDetailPage.styles.ts (1)
57-66: 하드코딩된 색상 대신 테마 색상 사용을 권장합니다.
background-color:#f2f2f2``가 하드코딩되어 있습니다. 일관성을 위해 테마 시스템의 색상을 사용하는 것이 좋습니다.border속성에서는 이미 `colors.gray[400]`을 사용하고 있습니다.♻️ 수정 제안
export const MapCard = styled.div` width: 100%; height: 189px; border-radius: 20px; border: 1px solid ${colors.gray[400]}; overflow: hidden; - background-color: `#f2f2f2`; + background-color: ${colors.gray[100]}; `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/ClubDetailPage/ClubDetailPage.styles.ts` around lines 57 - 66, Replace the hardcoded background-color in the MapCard styled component with a themed color; instead of `background-color: `#f2f2f2``, use the project's color token (e.g., `colors.gray[100]`) to match the existing `colors.gray[400]` usage so the component follows the theme system and remains consistent with other styles.frontend/src/components/map/useNaverMap.ts (1)
3-3: 내부 모듈 import는@/alias로 통일해 주세요.Line 3의 상대경로 import를 alias 경로로 맞추면 규칙 일관성과 리팩터링 안정성이 좋아집니다.
수정 제안
-import { loadNaverMapScript } from './loadNaverMapScript'; +import { loadNaverMapScript } from '@/components/map/loadNaverMapScript';As per coding guidelines,
Use path alias@/* to reference src/* for cleaner imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/map/useNaverMap.ts` at line 3, Replace the relative import of loadNaverMapScript in useNaverMap.ts with the project path-alias form (use "@/..." to reference the same module) so the import for loadNaverMapScript uses the `@/` alias consistently with the repo convention; update the import statement that currently references './loadNaverMapScript' to the equivalent '@/...' alias path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/map/loadNaverMapScript.ts`:
- Around line 2-3: Remove the debug console.log statements in
loadNaverMapScript.ts (the two lines logging import.meta.env and
VITE_NAVER_CLIENT_ID) to eliminate CI `Unexpected console statement` warnings
and noise; also correct the environment variable usage to the actual runtime key
(replace references to VITE_NAVER_CLIENT_ID with VITE_NAVER_MAP_CLIENT_ID where
the script reads the Naver map client id) so the loader uses the correct value.
- Around line 5-24: The loader currently resolves too early when an existing
script tag is present and never rejects on load failure; update
loadNaverMapScript so it never resolves simply because document.querySelector
found a script — instead attach load and error listeners to the existingScript
(listen for 'load' to resolve and 'error' to reject or fallback) and add both
onload and onerror handlers to the newly created script before appending it
(ensure you append the script to document.head/document.body); keep using
window.naver?.maps to short-circuit only if the SDK is already available, and
ensure the Promise always settles (resolve on load, reject or resolve with
failure handling on error) so callers won’t hang.
In `@frontend/src/constants/clubLocation.ts`:
- Around line 118-132: There are duplicate entries for clubName '모비딕'
(detailLocation '106호' and '214호') so only the first will be returned by any
Array.find lookup; either deduplicate the data by removing the unintended
duplicate or merge them into a single record (e.g., change detailLocation to an
array like detailLocations: ['106호','214호'] and adjust consumers), or if
multiple locations are intended keep both objects but update the lookup code to
use Array.filter (or a dedicated getLocationsByClubName function) instead of
Array.find; locate the objects with clubName '모비딕' in clubLocation.ts and apply
the chosen fix consistently across consumers that reference clubName.
---
Outside diff comments:
In
`@frontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.ts`:
- Around line 171-173: The IntroSection styled-component has a missing closing
brace causing a syntax/build error; locate the IntroSection definition (the
block containing ${media.tablet} and colors.gray[100]) and add the missing
closing curly brace(s) to properly terminate the ${media.tablet} media query and
the IntroSection styled-component so the CSS block parses correctly.
---
Nitpick comments:
In `@frontend/src/components/map/NaverMap.tsx`:
- Around line 5-13: The NaverMapProps interface declares clubName, building, and
detailLocation but the NaverMap component signature and body only use lat and
lng; either remove those unused props from NaverMapProps and from the NaverMap
call sites (e.g., where ClubDetailPage calls NaverMap) or update NaverMap to
accept and use clubName/building/detailLocation (for example to render a marker
label or InfoWindow) and add a TODO comment if you prefer to defer UI work;
update the interface, the NaverMap function parameters, and all places that
construct NaverMap props (notably ClubDetailPage) to keep them in sync.
In `@frontend/src/components/map/useNaverMap.ts`:
- Line 3: Replace the relative import of loadNaverMapScript in useNaverMap.ts
with the project path-alias form (use "@/..." to reference the same module) so
the import for loadNaverMapScript uses the `@/` alias consistently with the repo
convention; update the import statement that currently references
'./loadNaverMapScript' to the equivalent '@/...' alias path.
In `@frontend/src/constants/clubLocation.ts`:
- Line 9: Rename the exported constant clubLocations to CLUB_LOCATIONS and
update its export accordingly; then update all usages/imports of clubLocations
across the codebase to reference CLUB_LOCATIONS (search for the symbol
"clubLocations" and replace with "CLUB_LOCATIONS") and ensure it remains
exported from src/constants/clubLocation.ts so other modules import the new
name.
In `@frontend/src/index.tsx`:
- Around line 8-12: Move the dev-only Naver Map auth failure handler out of
frontend/src/index.tsx into the centralized SDK init module: create or use
src/utils/initSDK.ts and add a function (e.g., initializeNaverMapAuthHandler)
that registers window.navermap_authFailure in the same DEV-only conditional,
then import and call that function from index.tsx as part of the SDK
initialization sequence (alongside other init functions like
Mixpanel/Sentry/Channel.io/Kakao) so the handler and SDK bootstrapping are
managed in one place.
In `@frontend/src/pages/ClubDetailPage/ClubDetailPage.styles.ts`:
- Around line 57-66: Replace the hardcoded background-color in the MapCard
styled component with a themed color; instead of `background-color: `#f2f2f2``,
use the project's color token (e.g., `colors.gray[100]`) to match the existing
`colors.gray[400]` usage so the component follows the theme system and remains
consistent with other styles.
In `@frontend/src/types/window.d.ts`:
- Around line 4-5: Replace the broad any on window.naver with a minimal typed
interface: define NaverMaps and Naver interfaces that cover the SDK pieces you
use (e.g., Map, LatLng, Marker constructors and any methods/options you call)
and change the Window declaration so naver is typed as Naver | undefined; keep
navermap_authFailure as a function type. Update the types in
frontend/src/types/window.d.ts by adding the NaverMaps and Naver interface names
and using them in the global Window declaration instead of any to satisfy the
lint rule against any.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09b15eb6-e842-4223-bbea-bae4fee24d6e
⛔ Files ignored due to path filters (2)
frontend/src/assets/images/icons/location_icon.svgis excluded by!**/*.svgfrontend/src/assets/images/icons/marker.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
frontend/src/components/map/NaverMap.styles.tsfrontend/src/components/map/NaverMap.tsxfrontend/src/components/map/loadNaverMapScript.tsfrontend/src/components/map/useNaverMap.tsfrontend/src/constants/clubLocation.tsfrontend/src/index.tsxfrontend/src/pages/ClubDetailPage/ClubDetailPage.styles.tsfrontend/src/pages/ClubDetailPage/ClubDetailPage.tsxfrontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.tsfrontend/src/types/window.d.ts
Comment on lines
+2
to
+3
| console.log(import.meta.env); | ||
| console.log('NAVER CLIENT ID: ', import.meta.env.VITE_NAVER_CLIENT_ID); |
Contributor
There was a problem hiding this comment.
디버그 console.log는 머지 전에 제거해 주세요.
Line 2-3은 CI 경고(Unexpected console statement)와 동일하며, 운영 로그 노이즈를 유발합니다. 또한 VITE_NAVER_CLIENT_ID와 실제 사용 키(VITE_NAVER_MAP_CLIENT_ID)가 달라 디버깅 혼선을 줍니다.
🧰 Tools
🪛 GitHub Check: ci
[warning] 3-3:
Unexpected console statement
[warning] 2-2:
Unexpected console statement
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/map/loadNaverMapScript.ts` around lines 2 - 3, Remove
the debug console.log statements in loadNaverMapScript.ts (the two lines logging
import.meta.env and VITE_NAVER_CLIENT_ID) to eliminate CI `Unexpected console
statement` warnings and noise; also correct the environment variable usage to
the actual runtime key (replace references to VITE_NAVER_CLIENT_ID with
VITE_NAVER_MAP_CLIENT_ID where the script reads the Naver map client id) so the
loader uses the correct value.
Comment on lines
+5
to
+24
| return new Promise<void>((resolve) => { | ||
| if (window.naver?.maps) { | ||
| resolve(); | ||
| return; | ||
| } | ||
|
|
||
| const existingScript = document.querySelector( | ||
| 'script[src*="oapi.map.naver.com"]', | ||
| ); | ||
| if (existingScript) { | ||
| resolve(); | ||
| return; | ||
| } | ||
|
|
||
| const script = document.createElement('script'); | ||
| script.src = `https://oapi.map.naver.com/openapi/v3/maps.js?ncpKeyId=${import.meta.env.VITE_NAVER_MAP_CLIENT_ID}`; | ||
| script.async = true; | ||
|
|
||
| script.onload = () => resolve(); | ||
|
|
Contributor
There was a problem hiding this comment.
SDK 로더가 조기 resolve/무한 대기 상태를 만들 수 있습니다.
Line 14-16에서 script 태그가 “존재”하기만 해도 resolve되어 SDK 준비 전에 다음 로직이 실행될 수 있습니다. 또한 Line 23은 onload만 있어 로드 실패 시 Promise가 정착되지 않습니다. 로더 Promise를 단일화하고, 기존 script 분기에서도 load/error를 기다리도록 바꿔주세요.
수정 제안
+let naverMapScriptPromise: Promise<void> | null = null;
+
export const loadNaverMapScript = () => {
- console.log(import.meta.env);
- console.log('NAVER CLIENT ID: ', import.meta.env.VITE_NAVER_CLIENT_ID);
-
- return new Promise<void>((resolve) => {
- if (window.naver?.maps) {
- resolve();
- return;
- }
+ if (window.naver?.maps) return Promise.resolve();
+ if (naverMapScriptPromise) return naverMapScriptPromise;
+
+ naverMapScriptPromise = new Promise<void>((resolve, reject) => {
+ const finish = () => resolve();
+ const fail = () => reject(new Error('Failed to load Naver Map SDK'));
const existingScript = document.querySelector(
'script[src*="oapi.map.naver.com"]',
- );
+ ) as HTMLScriptElement | null;
+
if (existingScript) {
- resolve();
+ existingScript.addEventListener('load', finish, { once: true });
+ existingScript.addEventListener('error', fail, { once: true });
return;
}
const script = document.createElement('script');
script.src = `https://oapi.map.naver.com/openapi/v3/maps.js?ncpKeyId=${import.meta.env.VITE_NAVER_MAP_CLIENT_ID}`;
script.async = true;
-
- script.onload = () => resolve();
+ script.onload = finish;
+ script.onerror = fail;
document.head.appendChild(script);
});
+
+ return naverMapScriptPromise;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/map/loadNaverMapScript.ts` around lines 5 - 24, The
loader currently resolves too early when an existing script tag is present and
never rejects on load failure; update loadNaverMapScript so it never resolves
simply because document.querySelector found a script — instead attach load and
error listeners to the existingScript (listen for 'load' to resolve and 'error'
to reject or fallback) and add both onload and onerror handlers to the newly
created script before appending it (ensure you append the script to
document.head/document.body); keep using window.naver?.maps to short-circuit
only if the SDK is already available, and ensure the Promise always settles
(resolve on load, reject or resolve with failure handling on error) so callers
won’t hang.
lepitaaar
reviewed
Mar 29, 2026
| ); | ||
| if (existingScript) { | ||
| resolve(); | ||
| return; |
Contributor
There was a problem hiding this comment.
현재 로더는 existingScript 존재만 확인하고 즉시 resolve해서, 실제 로드 완료 전에 훅이 실행되면 window.naver가 없어 초기화가 누락될 수 있습니다. 기존 스크립트가 있더라도 load/error 리스너를 연결해 로드 완료 시점에 resolve/reject 하도록 보강해 주세요.
lepitaaar
reviewed
Mar 29, 2026
|
|
||
| const location = clubLocations.find( | ||
| (location) => location.clubName === clubDetail?.name, | ||
| ); |
Contributor
There was a problem hiding this comment.
clubName 기준 조회(find)와 데이터 중복이 결합되어 두 번째 동일 이름 항목이 사실상 도달 불가합니다. 위치 매핑은 clubId 같은 고유 키로 전환하거나, 최소한 중복 이름 데이터를 정리해 매핑 오차를 막아주세요.
Collaborator
Author
There was a problem hiding this comment.
현재는 동아리명이 고유하게 관리되고 있어 name 기반 매핑으로 구현했습니다. 또한 위치 데이터는 프론트에서 직접 관리하고 있어, 작성 편의성 측면에서 clubId보다 clubName 기반이 더 간단한 장점이 있었습니다.
다만 확장 시 안정성을 위해 clubId 기반 매핑으로 전환하는 방향도 고려하겠습니다.
Member
There was a problem hiding this comment.
백엔드에서 @cluaName 으로 라우팅을 하고 있어요. 그래서 clubName이 고유하지 않으면 라우팅도 제대로 되지 않아요.
lepitaaar
reviewed
Mar 29, 2026
| @@ -0,0 +1,27 @@ | |||
| export const loadNaverMapScript = () => { | |||
| console.log(import.meta.env); | |||
| console.log('NAVER CLIENT ID: ', import.meta.env.VITE_NAVER_CLIENT_ID); | |||
Contributor
There was a problem hiding this comment.
운영 콘솔 노이즈와 불필요한 환경 정보 노출을 줄이기 위해 디버그 로그(import.meta.env, client id)는 제거하거나 DEV 조건으로 제한하는 것을 권장합니다.
lepitaaar
approved these changes
Mar 29, 2026
Contributor
lepitaaar
left a comment
lepitaaar
left a comment
There was a problem hiding this comment.
Always Approve 정책에 따라 승인합니다.
중복 제거 및 근거 보강된 핵심 사항은 인라인 코멘트로 남겼습니다.
거절 사유(비차단 아님)
-
Severity: Medium
-
항목: Naver script loader의 existingScript 즉시 resolve로 인한 race condition
-
영향: 스크립트 태그가 존재하지만 아직 로드 전인 타이밍에서 지도 초기화가 누락되어 빈 지도 상태가 발생할 수 있음
-
수정방안: existingScript 분기에서도 load/error 리스너로 로드 완료를 보장한 뒤 resolve/reject 처리
-
Severity: Medium
-
항목: clubName 기반 위치 매핑 + 중복 데이터 결합
-
영향: 동명이칭 또는 중복 이름 항목에서 잘못된 좌표가 선택되거나 일부 데이터가 사실상 도달 불가
-
수정방안: clubId 등 고유 키 기반 매핑으로 전환하고 중복 이름 데이터 정리
-
Severity: Low
-
항목: map loader 내 운영 콘솔 디버그 로그 노출
-
영향: 콘솔 노이즈 증가 및 불필요한 환경 정보 노출
-
수정방안: 로그 제거 또는 DEV 조건으로 제한
suhyun113
changed the title
seongwon030
approved these changes
Apr 4, 2026
Member
seongwon030
left a comment
seongwon030
left a comment
There was a problem hiding this comment.
특별히 리뷰남길게 없어서 승인합니다 수고하셨습니다!
|
|
||
| const location = clubLocations.find( | ||
| (location) => location.clubName === clubDetail?.name, | ||
| ); |
Member
There was a problem hiding this comment.
백엔드에서 @cluaName 으로 라우팅을 하고 있어요. 그래서 clubName이 고유하지 않으면 라우팅도 제대로 되지 않아요.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/pages/ClubMapPage/ClubMapPage.styles.ts (1)
5-8: 타이포그래피 헬퍼는 공용 유틸로 추출해두면 유지보수가 쉬워집니다.동일한
setTypography패턴이frontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.ts에도 있어 중복 제거 여지가 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/ClubMapPage/ClubMapPage.styles.ts` around lines 5 - 8, Extract the duplicate setTypography helper into a single shared utility (exported function name setTypography with the same signature typo: { size: string; weight: number }) and replace the local definitions in both ClubMapPage.styles.ts and ClubProfileCard.styles.ts with imports from that new util; ensure the util exports the typed function and both style files import and use the shared setTypography so the visual output/typing remains identical and duplication is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.ts`:
- Around line 218-236: LocationInfo의 현재 CSS는 긴 위치 텍스트가 지도 링크를 밀어내면서
text-overflow가 작동하지 않을 수 있으니, 컨테이너와 텍스트에 축소/최소 너비 속성을 추가해 안정적으로 말줄임이 되도록 수정하세요;
구체적으로 styled component LocationInfo에 min-width: 0을 추가하고 내부 span에 flex: 1(또는
flex-grow:0; flex-shrink:1;), 및 min-width: 0을 설정해 text-overflow: ellipsis가 정상
동작하도록 하며 img 규칙은 그대로 둡니다 (참조: LocationInfo, span, img, setTypography).
In `@frontend/src/pages/ClubMapPage/ClubMapPage.tsx`:
- Around line 21-44: The component currently returns null when clubDetail or
clubLocation is missing which can leave the user on a blank page; instead, in
the ClubMapPage component replace the bare "if (!clubDetail || !clubLocation)
return null" with a user-facing fallback (e.g., a loading/error message or
skeleton) and a deterministic redirect fallback: when useGetClubDetail finishes
but clubLocation is still not found, call navigate('/', { replace: true }) or
navigate to a safe route (or show an error with a retry) so the UI doesn't
remain blank; locate this logic around the useGetClubDetail,
clubLocations/clubLocation lookup, mapRef, and handleBackClick/navigate code
paths and ensure the fallback distinguishes loading vs resolved-but-not-found
states.
- Line 57: The ClubMapPage currently renders <Styled.ClubLogo
src={clubDetail.logo} alt={`${clubDetail.name} 로고`} /> which shows a broken
image when clubDetail.logo is missing; update the ClubMapPage to provide a safe
fallback by using clubDetail.logo || DEFAULT_CLUB_LOGO (or attaching an onError
handler to <Styled.ClubLogo> that sets a default image) so the component always
displays the default logo when clubDetail.logo is falsy or the image fails to
load; reference the Styled.ClubLogo element and clubDetail.logo/clubDetail.name
to implement the fallback.
---
Nitpick comments:
In `@frontend/src/pages/ClubMapPage/ClubMapPage.styles.ts`:
- Around line 5-8: Extract the duplicate setTypography helper into a single
shared utility (exported function name setTypography with the same signature
typo: { size: string; weight: number }) and replace the local definitions in
both ClubMapPage.styles.ts and ClubProfileCard.styles.ts with imports from that
new util; ensure the util exports the typed function and both style files import
and use the shared setTypography so the visual output/typing remains identical
and duplication is removed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2dfac662-351d-4433-b917-7fbe3e73d063
⛔ Files ignored due to path filters (1)
frontend/src/assets/images/icons/location_icon.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
frontend/src/App.tsxfrontend/src/components/map/NaverMap.tsxfrontend/src/components/map/useNaverMap.tsfrontend/src/pages/ClubDetailPage/ClubDetailPage.styles.tsfrontend/src/pages/ClubDetailPage/ClubDetailPage.tsxfrontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.tsfrontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.tsxfrontend/src/pages/ClubMapPage/ClubMapPage.styles.tsfrontend/src/pages/ClubMapPage/ClubMapPage.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/pages/ClubDetailPage/ClubDetailPage.tsx
- frontend/src/components/map/NaverMap.tsx
- frontend/src/pages/ClubDetailPage/ClubDetailPage.styles.ts
- frontend/src/components/map/useNaverMap.ts
Comment on lines
+218
to
+236
| export const LocationInfo = styled.div` | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 5px; | ||
|
|
||
| img { | ||
| width: 12px; | ||
| height: 15px; | ||
| flex-shrink: 0; | ||
| } | ||
|
|
||
| span { | ||
| ${setTypography(typography.paragraph.p6)}; | ||
| color: ${colors.gray[600]}; | ||
| white-space: nowrap; | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| } | ||
| `; |
Contributor
There was a problem hiding this comment.
위치 텍스트 말줄임이 좁은 폭에서 안정적으로 동작하지 않을 수 있습니다.
LocationInfo에 축소 허용 속성이 없어 긴 텍스트가 지도 링크를 밀어낼 수 있습니다.
🔧 제안 수정안
export const LocationRow = styled.div`
display: flex;
align-items: center;
gap: 6px;
flex-wrap: nowrap;
+ min-width: 0;
`;
export const LocationInfo = styled.div`
display: flex;
align-items: center;
gap: 5px;
+ flex: 1;
+ min-width: 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/pages/ClubDetailPage/components/ClubProfileCard/ClubProfileCard.styles.ts`
around lines 218 - 236, LocationInfo의 현재 CSS는 긴 위치 텍스트가 지도 링크를 밀어내면서
text-overflow가 작동하지 않을 수 있으니, 컨테이너와 텍스트에 축소/최소 너비 속성을 추가해 안정적으로 말줄임이 되도록 수정하세요;
구체적으로 styled component LocationInfo에 min-width: 0을 추가하고 내부 span에 flex: 1(또는
flex-grow:0; flex-shrink:1;), 및 min-width: 0을 설정해 text-overflow: ellipsis가 정상
동작하도록 하며 img 규칙은 그대로 둡니다 (참조: LocationInfo, span, img, setTypography).
Comment on lines
+21
to
+44
| const { data: clubDetail } = useGetClubDetail((clubName ?? clubId) || ''); | ||
|
|
||
| const clubLocation = clubLocations.find( | ||
| (loc) => loc.clubName === clubDetail?.name, | ||
| ); | ||
|
|
||
| const mapRef = useRef<HTMLDivElement | null>(null); | ||
|
|
||
| useNaverMap(mapRef, clubLocation?.lat ?? 0, clubLocation?.lng ?? 0, { | ||
| bubbleText: '동아리방', | ||
| }); | ||
|
|
||
| const handleBackClick = () => { | ||
| const handled = requestNavigateBack(); | ||
| if (!handled) { | ||
| if (window.history.state && window.history.state.idx > 0) { | ||
| navigate(-1); | ||
| } else { | ||
| navigate('/', { replace: true }); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| if (!clubDetail || !clubLocation) return null; |
Contributor
There was a problem hiding this comment.
Line 44에서 데이터 미충족 시 null 반환으로 빈 화면이 고정될 수 있습니다.
로딩 이후에도 clubLocation 매칭 실패/조회 실패 시 화면이 영구적으로 비어 보일 수 있어요. 최소한 리다이렉트 fallback이 필요합니다.
🔧 제안 수정안
-import { useNavigate, useParams } from 'react-router-dom';
+import { Navigate, useNavigate, useParams } from 'react-router-dom';
...
- const { data: clubDetail } = useGetClubDetail((clubName ?? clubId) || '');
+ const { data: clubDetail, isPending } = useGetClubDetail(
+ (clubName ?? clubId) || '',
+ );
...
- if (!clubDetail || !clubLocation) return null;
+ if (isPending) return null;
+ if (!clubDetail || !clubLocation) return <Navigate to='/' replace />;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/ClubMapPage/ClubMapPage.tsx` around lines 21 - 44, The
component currently returns null when clubDetail or clubLocation is missing
which can leave the user on a blank page; instead, in the ClubMapPage component
replace the bare "if (!clubDetail || !clubLocation) return null" with a
user-facing fallback (e.g., a loading/error message or skeleton) and a
deterministic redirect fallback: when useGetClubDetail finishes but clubLocation
is still not found, call navigate('/', { replace: true }) or navigate to a safe
route (or show an error with a retry) so the UI doesn't remain blank; locate
this logic around the useGetClubDetail, clubLocations/clubLocation lookup,
mapRef, and handleBackClick/navigate code paths and ensure the fallback
distinguishes loading vs resolved-but-not-found states.
| </Styled.BackButton> | ||
|
|
||
| <Styled.BottomCard> | ||
| <Styled.ClubLogo src={clubDetail.logo} alt={`${clubDetail.name} 로고`} /> |
Contributor
There was a problem hiding this comment.
Line 57은 로고가 없을 때 깨진 이미지가 노출될 수 있습니다.
상세 페이지 카드와 동일하게 기본 로고 fallback을 넣는 편이 안전합니다.
🔧 제안 수정안
+import DefaultLogo from '@/assets/images/logos/default_profile_image.svg';
...
- <Styled.ClubLogo src={clubDetail.logo} alt={`${clubDetail.name} 로고`} />
+ <Styled.ClubLogo
+ src={clubDetail.logo || DefaultLogo}
+ alt={`${clubDetail.name} 로고`}
+ />📝 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.
Suggested change
| <Styled.ClubLogo src={clubDetail.logo} alt={`${clubDetail.name} 로고`} /> | |
| import DefaultLogo from '@/assets/images/logos/default_profile_image.svg'; | |
| // ... other imports and code ... | |
| <Styled.ClubLogo | |
| src={clubDetail.logo || DefaultLogo} | |
| alt={`${clubDetail.name} 로고`} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/ClubMapPage/ClubMapPage.tsx` at line 57, The ClubMapPage
currently renders <Styled.ClubLogo src={clubDetail.logo}
alt={`${clubDetail.name} 로고`} /> which shows a broken image when clubDetail.logo
is missing; update the ClubMapPage to provide a safe fallback by using
clubDetail.logo || DEFAULT_CLUB_LOGO (or attaching an onError handler to
<Styled.ClubLogo> that sets a default image) so the component always displays
the default logo when clubDetail.logo is falsy or the image fails to load;
reference the Styled.ClubLogo element and clubDetail.logo/clubDetail.name to
implement the fallback.
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.
#️⃣연관된 이슈
📝작업 내용
웹
모바일
❓작업 개요
상세페이지에 동아리방 위치를 확인할 수 있도록 네이버 지도 API를 연동하고, 커스텀 마커 및 UI를 적용했습니다.
신입 부원 기준으로 동아리실 위치를 쉽게 찾을 수 있도록 UX 개선을 목표로 구현했습니다.
🤔지도 API 선정 이유
1. 네이버 지도
2. 카카오 지도
3. 구글 지도
=> 위 이유로 네이버 지도를 채택했습니다.
🤗도입 목적
🛠️구현 내용
🔑환경 변수 설정
네이버 지도는 API 키가 없으면 동작하지 않기 때문에
.env파일에 추가해야합니다. FE 노션 자료실에 NaverMap API 키 글에 작성해두었으니 참고해주세요.네이버 지도 API 특성상 preview URL이 매번 변경되기 때문에
모든 preview 도메인을 사전에 등록하기 어려운 한계가 있습니다.
현재 등록된 preview 주소에서는 정상 동작하지만,
그 외 새로 생성되는 preview 환경에서는 도메인 미등록으로 인해
지도 API 요청이 정상적으로 동작하지 않을 수 있습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit