[Feat] 토스트 메세지 구현 및 툴팁 CSS 수정 #57
Conversation
- 툴팁 꼬리 형태를 삼각형에서 사각형으로 수정하여 꼬리와 몸통 사이의 미세한 경계를 제거하였습니다.
WalkthroughAdds a toast notification system (context, provider, hook) and wires it app-wide. Introduces auth handling (hook, ProtectedRoute), input/auth guards in fetchUnits, central API retry policy, and error page routing. Updates pages to use centralized error handling and retry. Adjusts tooltip UI. Minor path refactors and a new dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router
participant ProtectedRoute
participant useAuthHandler
participant Page
User->>Router: Navigate to protected path
Router->>ProtectedRoute: Render
ProtectedRoute->>useAuthHandler: checkAuthToken()
alt token missing
ProtectedRoute->>useAuthHandler: redirectToLogin()
useAuthHandler->>Router: navigate("/")
ProtectedRoute-->>Router: render null
else token present
ProtectedRoute-->>Router: render children
Router->>Page: Render page
end
sequenceDiagram
autonumber
participant Component
participant useToast
participant ToastContextProvider
participant UI
Component->>useToast: get showToast()
Component->>ToastContextProvider: showToast({message, duration})
ToastContextProvider->>ToastContextProvider: set toast state, reset timer
ToastContextProvider->>UI: AnimatePresence render toast
Note over UI: Toast enters, auto-hides after duration
sequenceDiagram
autonumber
participant Page
participant useQuery
participant API
participant shouldRetryApiRequest
participant useAuthHandler
Page->>useQuery: fetch(...)
useQuery->>API: request
API-->>useQuery: error (ApiError/Network)
useQuery->>shouldRetryApiRequest: decide retry
alt retry allowed
useQuery->>API: retry (<=2 for network)
else no retry
useQuery-->>Page: isError=true, error
Page->>useAuthHandler: handleApiError(error)
alt Unauthorized/Forbidden
useAuthHandler->>Page: handled=true
useAuthHandler->>Router: redirect to login with toast
else Other
useAuthHandler-->>Page: handled=false
end
end
sequenceDiagram
autonumber
participant Router
participant ErrorBoundary
participant ErrorPage
Router->>ErrorBoundary: Route error occurs
ErrorBoundary->>ErrorPage: render with route error
alt 404/401/500
ErrorPage-->>ErrorBoundary: Render specific component
else ApiError/Other
ErrorPage-->>ErrorBoundary: Render generic or API error UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 9
♻️ Duplicate comments (1)
src/pages/ChapterDetailPage.tsx (1)
45-49: Centralize auth-only redirects; avoid catching all ApiError here.Once handleApiError is fixed to check error.error, this effect will behave correctly.
🧹 Nitpick comments (28)
package.json (1)
18-18: Consider LazyMotion to keep bundle size in check.If toasts use small animations, prefer LazyMotion/mjs build and code-splitting to avoid pulling the full library on first paint.
src/types/@common/toast.ts (1)
1-4: Clarify units and extensibility (severity, a11y).Add docs for duration units (ms) and consider optional fields like id, variant ('success' | 'error' | ...), and ariaLive/role for accessibility.
Apply:
-export type ToastType = { - message: string; - duration?: number; -}; +/** + * Toast payload. + * duration: milliseconds (default handled by provider) + */ +export type ToastType = { + message: string; + duration?: number; // ms + // variant?: 'success' | 'info' | 'warning' | 'error'; + // ariaLive?: 'polite' | 'assertive' | 'off'; + // role?: 'status' | 'alert'; + // id?: string; +};src/types/transformers/chapters.ts (1)
5-11: Avoid unsafe cast; validate chapterId and sanitize counts.Direct cast to PlanetId can leak invalid IDs. Use the existing isPlanetId guard and clamp counts to prevent negative/computed anomalies.
Apply:
-import type { PlanetId } from '../../constants/planet-image'; +import type { PlanetId } from '../../constants/planet-image'; +import { isPlanetId } from '../../constants/planet-image'; @@ -export const transformChapter = (response: ChapterResponse): Chapter => ({ - id: response.chapterId as PlanetId, - name: response.name, - description: response.description, - totalUnits: response.totalUnits, - completedUnits: response.completedUnits, -}); +export const transformChapter = (response: ChapterResponse): Chapter => { + const id = isPlanetId(response.chapterId) ? (response.chapterId as PlanetId) : (1 as PlanetId); // TODO: centralize default + const totalUnits = Math.max(0, response.totalUnits ?? 0); + const completedUnits = Math.min(totalUnits, Math.max(0, response.completedUnits ?? 0)); + return { + id, + name: response.name, + description: response.description, + totalUnits, + completedUnits, + }; +};.github/pull_request_template.md (2)
24-29: Tighten checklist to reduce regressions.Make tests/screenshots explicit when UI or logic changes occur; add a breaking-changes section.
Apply:
## ✅ 체크리스트 - -- [ ] 코드가 정상적으로 동작합니다 -- [ ] 변경 사항이 기존 기능에 영향을 주지 않습니다 -- [ ] 새로운 테스트를 추가했습니다 (해당하는 경우) -- [ ] 문서를 업데이트했습니다 (해당하는 경우) + - [ ] 코드가 정상적으로 동작합니다 + - [ ] 변경 사항이 기존 기능에 영향을 주지 않습니다 + - [ ] (UI/로직 변경 시 필수) 새로운 테스트를 추가했습니다 / 사유: [ ] + - [ ] (UI 변경 시 필수) 스크린샷/동영상 첨부했습니다 / 사유: [ ] + - [ ] 문서를 업데이트했습니다 (해당하는 경우) + +## ⚠️ 브레이킹 체인지 +<!-- 공개 API, 라우팅, 스키마 등이 바뀌었다면 상세히 작성해주세요 --> +없음 / 상세:
10-12: Rename “style” to avoid CSS-vs-formatting ambiguity.Minor wording tweak: “style (코드 스타일/포맷팅)” to distinguish from CSS style.
Apply:
-- [ ] 🤝🏻 style (코드 포맷팅, 세미콜론 누락 등) +- [ ] 🎨 style (코드 스타일/포맷팅, 세미콜론 누락 등).github/ISSUE_TEMPLATE/feature.yml (1)
14-18: Show checkbox examples in the placeholder for clarity.Helps reporters provide actionable tasks.
Apply:
description: 할 일을 체크박스 형태로 작성해주세요. - placeholder: 최대한 세분화 해서 적어주세요! + placeholder: | + - [ ] API 스펙 정의 + - [ ] UI 와이어프레임 + - [ ] 구현 및 테스트.github/ISSUE_TEMPLATE/bug_report.yml (1)
31-39: Optional: capture more diagnostics in 환경 정보.
Consider adding App Version/Build, URL, Locale, and Console/Network logs to speed up triage.placeholder: | - - OS: [예: iOS] - - Browser: [예: Chrome, Safari] - - Version: [예: 22] + - OS: [예: iOS 17.6] + - Browser: [예: Chrome 128, Safari 17] + - App Version/Build: [예: 1.3.2 (123)] + - URL: [예: /lesson/42] + - Locale/Timezone: [예: ko-KR, GMT+9] + - Console/Network 로그: [첨부/요약]src/components/chapter-page/Tooltip.tsx (2)
9-9: Avoid double-centering inside an already centered wrapper.
The inner body uses left-1/2 and -translate-x-1/2 again; not needed and can complicate stacking contexts.- <div className="bg-[#FFB608] flex flex-col p-4 w-[371px] h-[130px] rounded-2xl justify-between z-50 transform -translate-x-1/2 left-1/2 relative"> + <div className="bg-[#FFB608] flex flex-col p-4 w-[371px] h-[130px] rounded-2xl justify-between z-50">
7-7: Remove unused border utilities on the diamond tail.
With a solid rotated square, border-l/t transparent have no visual effect.- <div className="absolute top-0 left-1/2 transform -translate-x-1/2 -translate-y-1/2 h-7 w-7 bg-[#FFB608] border-l border-t border-transparent rotate-45 z-40" /> + <div className="absolute top-0 left-1/2 -translate-x-1/2 -translate-y-1/2 h-7 w-7 bg-[#FFB608] rotate-45 z-40" />src/pages/Error.tsx (1)
62-76: Optional: use client-side navigation instead of full reload/redirect.
Avoids losing app state from a hard navigation.- <button - onClick={() => (window.location.href = '/login')} + {/* consider useNavigate() */} + <button + onClick={() => (window.location.href = '/login')} className="px-4 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all" > 로그인하기 </button>src/api/fetchChapters.ts (1)
3-44: Tighten headers, add timeout, and drop debug log for production hygiene
- Avoid sending
Content-Type: application/jsonon GET (triggers unnecessary CORS preflight).- Don’t send
Authorization: Bearer nullwhen no token is present.- Add an AbortController-based timeout to fail fast on hung requests.
- Remove the console log or gate it behind dev checks.
import type { Chapter } from '../types/@common/chapter'; -import { transformChapters } from '../types/transformers/chapters'; +import { transformChapters } from '../types/transformers/chapters'; export default async function fetchChapters(): Promise<Chapter[]> { const accessToken = localStorage.getItem('accessToken'); - try { - const response = await fetch(`https://grav-it.inuappcenter.kr/api/v1/learning/chapters`, { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + try { + const response = await fetch(`https://grav-it.inuappcenter.kr/api/v1/learning/chapters`, { method: 'GET', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${accessToken}`, - }, + headers: { + ...(accessToken ? { Authorization: `Bearer ${accessToken}` } : {}), + }, + signal: controller.signal, }); if (!response.ok) { let errorData: ApiError; try { const errorResponse = await response.json(); errorData = new ApiError( errorResponse.message || `HTTP ${response.status}: ${response.statusText}`, errorResponse.error || 'HTTP_ERROR' ); } catch { // JSON 파싱 실패 시 기본 에러 메시지 errorData = new ApiError(`HTTP ${response.status}: ${response.statusText}`, 'HTTP_ERROR'); } throw errorData; } // 성공 응답 파싱 const data = await response.json(); - console.log('서버 응답:', data); // 디버깅용 - return transformChapters(data); } catch (error) { // 네트워크 에러나 기타 예외 처리 if (error instanceof ApiError) { throw error; } throw new ApiError('네트워크 연결을 확인해주세요.', 'NETWORK_ERROR'); } + finally { + clearTimeout(timeoutId); + } }src/routes/router.tsx (1)
71-79: Reduce repetition by grouping protected routes under a single gateYou wrap several leaves with
ProtectedRoute. Consider a parent route withelement: <ProtectedRoute><Outlet/></ProtectedRoute>and move the protected children under it to DRY the config.Example sketch (apply structure, not exact lines):
{ element: <WithHeaderLayout headerOverlay={true} />, children: [ + { + element: ( + <LazyPage> + <ProtectedRoute> + <Outlet /> + </ProtectedRoute> + </LazyPage> + ), + children: [ + { path: 'main', element: <MainPage /> }, + { path: 'study', element: <ChapterListPage /> }, + { path: 'study/:chapterId', element: <ChapterDetailPage /> }, + ], + }, - ...existing per-route ProtectedRoute wrappers... ], }Also applies to: 86-94, 96-104, 127-135
src/hooks/useToast.tsx (1)
5-8: ReturnshowToastdirectly to avoid recreating a wrapper each renderThis keeps a stable function identity (assuming the context value is stable) and trims a tiny bit of overhead.
export default function useToast() { - const { showToast } = useContext(ToastContext); - return (toast: ToastType) => showToast(toast); + const { showToast } = useContext(ToastContext); + return showToast; }src/utils/api-retry.ts (1)
3-13: Type the error asunknownand consider transient HTTP retries
- TanStack Query passes
unknown; type it asunknownand narrow toApiError.- Optionally retry transient server errors (e.g., 5xx, 429) if your
ApiErrorcan carry status codes. If not, consider extending it.-export default function shouldRetryApiRequest(failureCount: number, error: Error) { - if (error instanceof ApiError) { +export default function shouldRetryApiRequest(failureCount: number, error: unknown) { + if (error instanceof ApiError) { if (['UNAUTHORIZED', 'FORBIDDEN'].includes(error.error)) { return false; } if (error.error === 'NETWORK_ERROR') { return failureCount < 2; } + // Optional: if ApiError includes `status`, treat 5xx/429 as transient + // if (error.error === 'HTTP_ERROR' && typeof (error as any).status === 'number') { + // const status = (error as any).status; + // if (status >= 500 || status === 429) return failureCount < 2; + // } } return false; }src/api/fetchUnits.ts (2)
6-8: Tighten ID validation (integer and positive).Prevent floats/negatives from passing; the API likely expects a positive integer.
- if (!Number.isFinite(id)) { + if (!Number.isInteger(id) || id <= 0) { throw new ApiError('유효하지 않은 학습 ID입니다.', 'INVALID_ARGUMENT'); }
12-15: Avoid noisy console logs on auth guard.Either remove or downgrade to warn; rely on UI/toast for messaging.
- if (!accessToken) { - console.log('로그인이 필요합니다.', 'UNAUTHORIZED'); - throw new ApiError('로그인이 필요합니다.', 'UNAUTHORIZED'); - } + if (!accessToken) { + // console.warn('로그인이 필요합니다.', 'UNAUTHORIZED'); + throw new ApiError('로그인이 필요합니다.', 'UNAUTHORIZED'); + }src/context/ToastContext.tsx (1)
4-10: Export the context type and set a displayName for DevTools.-type ToastContextType = { +export type ToastContextType = { showToast: (toast: ToastType) => void; }; const ToastContext = createContext<ToastContextType>({ showToast: () => {}, }); +ToastContext.displayName = 'ToastContext';src/components/@common/auth/ProtectedRoute.tsx (1)
11-19: Avoid double token checks per render.Compute once, use in effect and render.
- useEffect(() => { - if (!checkAuthToken()) { - redirectToLogin(); - } - }, [checkAuthToken, redirectToLogin]); - - if (!checkAuthToken()) { - return null; - } + const isAuthed = checkAuthToken(); + + useEffect(() => { + if (!isAuthed) { + redirectToLogin(); + } + }, [isAuthed, redirectToLogin]); + + if (!isAuthed) { + return null; + }src/pages/ChapterListPage.tsx (1)
12-16: Type the query for data and error.This removes unknown-typed error and helps call sites.
- const { data, isLoading, isError, error } = useQuery({ + const { data, isLoading, isError, error } = useQuery<Chapter[], ApiError>({ queryKey: ['chapter-list'], queryFn: fetchChapters, retry: shouldRetryApiRequest, });Additional imports required:
+import type { Chapter } from '../types/@common/chapter'; +import { ApiError } from '../types/@common/api';src/pages/ChapterDetailPage.tsx (5)
26-39: Validate chapterId once and use enabled to avoid firing invalid requests.Prevents NaN/invalid IDs from calling the API and wrongly redirecting.
- const { chapterId } = useParams(); - const { handleApiError } = useAuthHandler(); + const { chapterId } = useParams(); + const { handleApiError } = useAuthHandler(); + const id = Number(chapterId); + const isValidId = Number.isInteger(id) && id > 0; @@ - } = useQuery({ - queryKey: ['unit-list', { id: chapterId }], - queryFn: () => fetchUnits(Number(chapterId)), - retry: shouldRetryApiRequest, - }); + } = useQuery({ + queryKey: ['unit-list', { id }], + queryFn: () => fetchUnits(id), + retry: shouldRetryApiRequest, + enabled: isValidId, + });
51-56: Handle invalid ID UX explicitly before loading.+ if (!isValidId) { + return <div>잘못된 학습 ID입니다.</div>; + } if (isLoading) { return <div>로딩중</div>; } if (!units) { return <div>유닛 정보가 없습니다.</div>; }
59-61: Remove leftover debug log.- if (units) { - console.log(units); - } + // (debug log removed)
75-83: Use strict equality for index check.- if (index == 0) { + if (index === 0) {Also consider guarding when units.length > positions.length to avoid undefined access.
67-69: Reuse parsed id for planet image.- backgroundImage: `url(${backgroundImg}), url(${getPlanetImage(Number(chapterId))})`, + backgroundImage: `url(${backgroundImg}), url(${getPlanetImage(id)})`,src/pages/MainPage.tsx (2)
33-35: Show a dedicated error state instead of falling back to “데이터가 없습니다.”When
isErroris true and the error isn’t auth-related, render a small retryable error UI; otherwise the user only sees “데이터가 없습니다.” with no guidance.- if (!data) { - return <div>데이터가 없습니다.</div>; - } + if (isError) { + return <div className="p-6">데이터를 불러오지 못했습니다. 잠시 후 다시 시도해주세요.</div>; + } + if (!data) { + return <div className="p-6">데이터가 없습니다.</div>; + }
37-39: Defensive access for recentData to avoid crashes on partial responses.If
recentLearningSummaryResponseis missing, these fields will throw. Add a guard or sensible fallback.- const { nickname, level, xp, league, recentLearningSummaryResponse: recentData } = data; + const { nickname, level, xp, league, recentLearningSummaryResponse: recentData } = data; + if (!recentData) { + return <div className="p-6">최근 학습 데이터가 없습니다.</div>; + }Also applies to: 57-99
src/context/ToastContextProvider.tsx (1)
39-46: Add basic a11y attributes to toast.Announce messages to assistive tech and mark atomically.
- <motion.div + <motion.div + role="status" + aria-live="polite" + aria-atomic="true"src/hooks/useAuthHandler.tsx (1)
9-12: Optionally clear invalid tokens before redirect to avoid loops.Clearing stale tokens prevents repeated unauthorized calls after navigation.
- const redirectToLogin = useCallback(() => { + const redirectToLogin = useCallback(() => { + localStorage.removeItem('accessToken'); showToast({ message: '로그인 후 이용할 수 있는 서비스입니다.' }); navigate('/', { replace: true }); }, [navigate, showToast]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (23)
.github/ISSUE_TEMPLATE/bug_report.yml(1 hunks).github/ISSUE_TEMPLATE/chore.yml(1 hunks).github/ISSUE_TEMPLATE/enhancement.yml(1 hunks).github/ISSUE_TEMPLATE/feature.yml(1 hunks).github/pull_request_template.md(1 hunks)package.json(1 hunks)src/App.tsx(1 hunks)src/api/fetchChapters.ts(1 hunks)src/api/fetchUnits.ts(1 hunks)src/components/@common/auth/ProtectedRoute.tsx(1 hunks)src/components/chapter-page/Tooltip.tsx(1 hunks)src/context/ToastContext.tsx(1 hunks)src/context/ToastContextProvider.tsx(1 hunks)src/hooks/useAuthHandler.tsx(1 hunks)src/hooks/useToast.tsx(1 hunks)src/pages/ChapterDetailPage.tsx(2 hunks)src/pages/ChapterListPage.tsx(1 hunks)src/pages/Error.tsx(1 hunks)src/pages/MainPage.tsx(1 hunks)src/routes/router.tsx(6 hunks)src/types/@common/toast.ts(1 hunks)src/types/transformers/chapters.ts(1 hunks)src/utils/api-retry.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (14)
src/types/transformers/chapters.ts (5)
src/utils/transformChapter.ts (1)
response(5-11)src/types/@common/chapter.ts (1)
Chapter(3-9)src/types/api/chapter.ts (1)
ChapterResponse(1-7)src/components/study-page/ChapterCard.tsx (1)
ChapterCard(7-31)src/constants/planet-image.tsx (1)
getPlanetImage(29-34)
src/api/fetchChapters.ts (3)
src/utils/transformChapter.ts (2)
response(5-11)responses(13-13)src/types/@common/chapter.ts (1)
Chapter(3-9)src/types/api/chapter.ts (1)
ChapterResponse(1-7)
src/hooks/useAuthHandler.tsx (4)
src/hooks/useToast.tsx (1)
useToast(5-8)src/api/PostOAuth.tsx (1)
PostOAuth(5-57)src/components/@common/header/Header.tsx (1)
Header(6-25)src/api/getMypage.tsx (1)
accessToken(5-30)
src/components/@common/auth/ProtectedRoute.tsx (2)
src/hooks/useAuthHandler.tsx (1)
useAuthHandler(5-30)src/api/PostOAuth.tsx (1)
PostOAuth(5-57)
src/utils/api-retry.ts (1)
src/types/@common/api.ts (1)
ApiError(1-11)
src/context/ToastContext.tsx (1)
src/types/@common/toast.ts (1)
ToastType(1-4)
src/routes/router.tsx (2)
src/pages/Error.tsx (1)
ErrorPage(157-206)src/components/@common/auth/ProtectedRoute.tsx (1)
ProtectedRoute(8-22)
src/pages/ChapterListPage.tsx (3)
src/hooks/useAuthHandler.tsx (1)
useAuthHandler(5-30)src/api/fetchChapters.ts (1)
fetchChapters(5-45)src/utils/api-retry.ts (1)
shouldRetryApiRequest(3-13)
src/api/fetchUnits.ts (4)
src/types/api/unit.ts (1)
UnitsResponse(7-15)src/types/transformers/units.ts (2)
response(11-21)responses(22-22)src/types/@common/unit.ts (1)
Unit(3-9)src/api/fetchMainInfo.ts (1)
fetchMainInfo(5-47)
src/hooks/useToast.tsx (1)
src/types/@common/toast.ts (1)
ToastType(1-4)
src/pages/ChapterDetailPage.tsx (3)
src/hooks/useAuthHandler.tsx (1)
useAuthHandler(5-30)src/api/fetchUnits.ts (1)
fetchUnits(5-52)src/utils/api-retry.ts (1)
shouldRetryApiRequest(3-13)
src/context/ToastContextProvider.tsx (1)
src/types/@common/toast.ts (1)
ToastType(1-4)
src/pages/Error.tsx (1)
src/types/@common/api.ts (1)
ApiError(1-11)
src/pages/MainPage.tsx (3)
src/hooks/useAuthHandler.tsx (1)
useAuthHandler(5-30)src/api/fetchMainInfo.ts (1)
fetchMainInfo(5-47)src/utils/api-retry.ts (1)
shouldRetryApiRequest(3-13)
🪛 LanguageTool
.github/pull_request_template.md
[grammar] ~1-~1: There might be a mistake here.
Context: ## 🔍 작업 유형 - [ ] 🛠 feat (새로운 기능) - [ ] 🔧 fix (버그 ...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ...는 항목에 x 표시 --> - [ ] 🛠 feat (새로운 기능) - [ ] 🔧 fix (버그 수정) - [ ] ✨ enhancement...
(QB_NEW_EN)
[grammar] ~6-~6: There might be a mistake here.
Context: ... 🛠 feat (새로운 기능) - [ ] 🔧 fix (버그 수정) - [ ] ✨ enhancement (기존 기능 개선) - [ ] ⚙️ ...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...(버그 수정) - [ ] ✨ enhancement (기존 기능 개선) - [ ] ⚙️ chore (빌드, 설정 변경) - [ ] 📝 docs...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ... (기존 기능 개선) - [ ] ⚙️ chore (빌드, 설정 변경) - [ ] 📝 docs (문서 수정) - [ ] 🤝🏻 style (...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...hore (빌드, 설정 변경) - [ ] 📝 docs (문서 수정) - [ ] 🤝🏻 style (코드 포맷팅, 세미콜론 누락 등) - [...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...) - [ ] 🤝🏻 style (코드 포맷팅, 세미콜론 누락 등) - [ ] 💻 refactor (코드 리팩토링) - [ ] ✅ test...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...세미콜론 누락 등) - [ ] 💻 refactor (코드 리팩토링) - [ ] ✅ test (테스트 추가/수정) ## 📄 작업 내용 <!-...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...tor (코드 리팩토링) - [ ] ✅ test (테스트 추가/수정) ## 📄 작업 내용 <!-- 이번 PR에서 변경된 내용을 설명해주세요 --...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ... - [ ] ✅ test (테스트 추가/수정) ## 📄 작업 내용 ## 🔗 관련 이슈 -...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ... 이번 PR에서 변경된 내용을 설명해주세요 --> ## 🔗 관련 이슈 - Closes #이슈번호 ## ✅ 체크리스트 - [ ] 코드가 정상...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...이트했습니다 (해당하는 경우) ## 🖼️ 🖥 구현 결과 (선택사항) ## ✔ 리뷰 요구사항 <!-- 리뷰어에게 요청하고 싶은 부분을 작성해주세요...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...뷰어에게 요청하고 싶은 부분을 작성해주세요 --> ## 📋 참고 문서
(QB_NEW_EN)
🪛 Biome (2.1.2)
src/pages/Error.tsx
[error] 181-183: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (14)
package.json (1)
18-18: No action needed: Framer Motion 12.23.12 supports React 19
Confirmed version 12.23.12 exists and its peerDependencies include react ^18.0.0 || ^19.0.0 and react-dom ^18.0.0 || ^19.0.0, so it’s compatible with React 19 under Vite 7.src/types/@common/toast.ts (1)
1-4: LGTM on the minimal toast payload type.src/types/transformers/chapters.ts (2)
13-13: Map helper is fine.No change needed here.
1-3: No stale imports remain. Verified that no references tosrc/utils/transformChapterexist..github/ISSUE_TEMPLATE/feature.yml (1)
1-3: Preset a title prefix and verify the GitHub labelAdd the
titlefield for consistent issue titles:name: '✨ feature' description: '새로운 기능 추가' labels: ['✨ feature'] +title: 'feat: 'Then confirm that the
✨ featurelabel exists in the repository (viagh label listor the GitHub UI) and create it if it’s missing..github/ISSUE_TEMPLATE/bug_report.yml (1)
1-39: LGTM — template reads well and is valid for GitHub Issue Forms.
No blocking concerns..github/ISSUE_TEMPLATE/chore.yml (1)
1-58: LGTM — clear structure for chore issues.
No blockers..github/ISSUE_TEMPLATE/enhancement.yml (1)
1-24: LGTM — concise enhancement form.
No blockers.src/pages/Error.tsx (1)
189-191: Retain the name-check fallback in isApiError after deduplication.
Once ApiError is imported, instanceof should work; keeping name-check preserves compatibility if errors originate from another bundle.src/App.tsx (1)
3-10: Provider wrap looks goodWrapping RouterProvider with ToastContextProvider is correct to expose toasts app-wide. No issues spotted.
src/context/ToastContext.tsx (1)
8-10: LGTM — sensible default no-op makes consumers safe before provider mounts.src/components/@common/auth/ProtectedRoute.tsx (1)
8-15: LGTM — simple and effective guard with redirect side-effect.src/pages/MainPage.tsx (1)
23-27: Avoid blanket login redirects; gate by auth-only errors.As written, any ApiError triggers handleApiError, which currently redirects to login for all ApiErrors. This will misroute on non-auth failures (e.g., 404/500/network). See my suggested fix in useAuthHandler to only redirect on 401/403 or UNAUTHORIZED/FORBIDDEN codes. After that change, this effect is fine.
src/context/ToastContextProvider.tsx (1)
43-43: Verify Tailwind classtop-1/6exists in your config.Not in Tailwind’s default inset scale in some setups. Confirm or replace with a spacing value (e.g.,
top-16) or arbitrary valuetop-[16vh].
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/pages/Error.tsx (2)
2-2: Resolved: Centralized ApiError import.Good fix. This addresses the earlier duplication risk and keeps instanceof checks reliable.
157-166: Resolved: Scoped const inside switch-case.Wrapping the 500-branch in a block fixes noSwitchDeclarations and avoids leakage across cases.
🧹 Nitpick comments (7)
src/pages/Error.tsx (7)
1-1: Use SPA navigation (Link) instead of window.location to avoid full reloads.Replace the “메인으로/로그인하기” buttons with react-router Link for better UX and state preservation.
- import { useRouteError, isRouteErrorResponse } from 'react-router-dom'; + import { useRouteError, isRouteErrorResponse, Link } from 'react-router-dom';@@ (404) - <button - onClick={() => (window.location.href = '/')} - className="px-4 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all" - > - 메인으로 - </button> + <Link + to="/" + className="px-4 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all inline-flex items-center justify-center" + > + 메인으로 + </Link>@@ (401) - <button - onClick={() => (window.location.href = '/login')} - className="px-4 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all" - > - 로그인하기 - </button> + <Link + to="/login" + className="px-4 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all inline-flex items-center justify-center" + > + 로그인하기 + </Link>@@ (ApiError) - <button - onClick={() => (window.location.href = '/')} - className="px-4 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all" - > - 메인으로 - </button> + <Link + to="/" + className="px-4 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all inline-flex items-center justify-center" + > + 메인으로 + </Link>@@ (DefaultError) - <button - onClick={() => (window.location.href = '/')} - className="px-6 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all" - > - 메인으로 - </button> + <Link + to="/" + className="px-6 py-2 bg-purple-600 text-white rounded-full hover:bg-purple-700 transition-all inline-flex items-center justify-center" + > + 메인으로 + </Link>Tip: If you also want SPA “이전으로”, switch to useNavigate and call navigate(-1).
Also applies to: 19-31, 50-63, 82-95, 122-135
141-143: Strengthen isApiError guard to reduce false positives.Also verify shape to avoid collisions on name-only errors.
-function isApiError(error: unknown): error is ApiError { - return error instanceof ApiError || (error instanceof Error && error.name === 'ApiError'); -} +function isApiError(error: unknown): error is ApiError { + return ( + error instanceof ApiError || + (error instanceof Error && error.name === 'ApiError') || + (typeof error === 'object' && + error !== null && + // minimal shape check + (error as any).name === 'ApiError' && + typeof (error as any).message === 'string' && + typeof (error as any).error === 'string') + ); +}
157-164: Prefer server-provided message when available.Surface error.data.message if present; fall back to statusText.
- case 500: { - const serverMessage = error.statusText ? error.statusText : 'Internal server error'; + case 500: { + const serverMessage = + (typeof error.data === 'object' && + error.data !== null && + 'message' in (error.data as any) && + typeof (error.data as any).message === 'string' + ? (error.data as any).message + : undefined) || + error.statusText || + 'Internal server error'; return ( <DefaultErrorComponent title="서버 오류" message="서버에서 오류가 발생했습니다." errorDetails={serverMessage} /> ); }
171-172: Unify locale: avoid English fallback in a Korean UI.Replace with a Korean default.
- message={error.statusText || 'Something went wrong'} + message={error.statusText || '문제가 발생했습니다.'}
5-35: Reduce duplication with an ErrorLayout wrapper.Shared structure (wrapper, icon, actions) is repeated; extract a small layout component to DRY.
Also applies to: 39-66, 69-98, 101-138
148-148: Log to monitoring in addition to console.error.Forward route errors to Sentry/LogRocket/etc. for visibility.
145-195: Add tests for routing branches and type guard.Cover: 404/401/500/default, ApiError path, generic Error path, and isApiError behavior. I can scaffold RTL tests if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/pages/Error.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/Error.tsx (1)
src/types/@common/api.ts (1)
ApiError(1-11)
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 (1)
src/pages/ChapterDetailPage.tsx (1)
76-87: Out-of-bounds read on positions; harden mapping.If units.length > positions.length, positions[index] is undefined → runtime crash when accessing .x/.y. Guard or generate coordinates dynamically.
- {units?.map((unit, index) => { - if (index == 0) { + {units?.map((unit, index) => { + const pos = positions[index]; + if (!pos) return null; // or compute fallback position + if (index === 0) { return ( <button key={unit.id} - className="absolute transform -translate-x-1/2 -translate-y-1/2 cursor-pointer" + className="absolute transform -translate-x-1/2 -translate-y-1/2 cursor-pointer" style={{ - left: `${positions[index].x}%`, - top: `${positions[index].y}px`, + left: `${pos.x}%`, + top: `${pos.y}px`, zIndex: tooltip === index ? 50 : 10, }} onClick={() => { setTooltip(tooltip === index ? null : index); }} >return ( <button key={unit.id} className="absolute transform -translate-x-1/2 -translate-y-1/2 cursor-pointer" style={{ - left: `${positions[index].x}%`, - top: `${positions[index].y}px`, + left: `${pos.x}%`, + top: `${pos.y}px`, zIndex: tooltip === index ? 50 : 10, }} onClick={() => { setTooltip(tooltip === index ? null : index); }} >Also applies to: 99-115
♻️ Duplicate comments (1)
src/pages/MainPage.tsx (1)
30-32: React Query v5 isPending usage LGTM.Matches prior guidance to replace isLoading with isPending.
🧹 Nitpick comments (13)
src/components/chapter-page/Tooltip.tsx (2)
7-7: Make the tail decorative-only and non-interactive; drop unnecessary borders.Prevents the tail from capturing hover/clicks, improves a11y, and removes unused borders. The
transformutility is redundant given translate/rotate utilities.- <div className="absolute top-0 left-1/2 transform -translate-x-1/2 -translate-y-1/2 h-7 w-7 bg-[#FFB608] border-l border-t border-transparent rotate-45 z-40" /> + <div aria-hidden="true" className="pointer-events-none absolute top-0 left-1/2 -translate-x-1/2 -translate-y-1/2 h-7 w-7 bg-[#FFB608] rotate-45 z-40" />
9-9: Remove redundant centering on the body and add semantic role.Outer wrapper already centers the tooltip; this avoids double transforms and adds
role="tooltip"for semantics.- <div className="bg-[#FFB608] flex flex-col p-4 w-[371px] h-[130px] rounded-2xl justify-between z-50 transform -translate-x-1/2 left-1/2 relative"> + <div role="tooltip" className="bg-[#FFB608] flex flex-col p-4 w-[371px] h-[130px] rounded-2xl justify-between z-50 relative">src/pages/ChapterDetailPage.tsx (6)
52-58: Avoid error flash; distinguish empty vs. error states.Currently, on errors users may briefly see “유닛 정보가 없습니다.” before redirect. Return null during isError and keep empty state for true-empty data.
- if (isPending) { + if (isPending) { return <div>로딩중</div>; } - if (!units) { - return <div>유닛 정보가 없습니다.</div>; - } + if (isError) { + return null; + } + if (!units?.length) { + return <div>유닛 정보가 없습니다.</div>; + }
60-62: Remove debug log.Console noise in production.
- if (units) { - console.log(units); - } + // no-op
77-77: Use strict equality.TS/JS best practice.
- if (index == 0) { + if (index === 0) {
79-96: Hardcoded chapterNumber=3 looks incorrect.Likely should reflect current item index or chapter context. Please confirm requirement.
- <button + <button + type="button" key={unit.id} className="absolute transform -translate-x-1/2 -translate-y-1/2 cursor-pointer" style={{ left: `${pos.x}%`, top: `${pos.y}px`, zIndex: tooltip === index ? 50 : 10, }} onClick={() => { setTooltip(tooltip === index ? null : index); }} > - <CircularSegmentIndicator chapterNumber={3}> + <CircularSegmentIndicator chapterNumber={index + 1}> <img src={getPlanetImage(Number(chapterId))} className="w-[110px]" alt={unit.name} /> </CircularSegmentIndicator>
100-115: Button type and a11y nit.Explicit type prevents accidental form submit behavior in nested forms.
- return ( - <button + return ( + <button + type="button" key={unit.id} className="absolute transform -translate-x-1/2 -translate-y-1/2 cursor-pointer"
65-74: Optional: avoid repeated Number(chapterId) and add image fallback.Cache chapterNum and fall back when NaN to prevent broken planet images.
Example (outside diff scope):
const chapterNum = Number(chapterId); const planetImg = Number.isFinite(chapterNum) ? getPlanetImage(chapterNum) : defaultPlanet; // use planetImg in backgroundImage and <img src={planetImg} .../>Also applies to: 91-93
src/pages/MainPage.tsx (2)
18-22: Prefer onError over effect for query errors.This avoids a post-render flash and keeps logic local to the query.
- const { data, isPending, isError, error } = useQuery({ + const { data, isPending, isError, error } = useQuery({ queryKey: ['main-info'], queryFn: fetchMainInfo, - retry: shouldRetryApiRequest, + staleTime: 60_000, + retry: shouldRetryApiRequest, + onError: (err) => { + if (err instanceof ApiError) { + handleApiError(err); + } + }, }); - - useEffect(() => { - if (isError && error instanceof ApiError) { - handleApiError(error); - } - }, [isError, error, handleApiError]);Also applies to: 24-29
34-36: Avoid showing “데이터가 없습니다.” on error.Prevent a brief empty-state flash while redirecting.
- if (!data) { - return <div>데이터가 없습니다.</div>; - } + if (isError) return null; + if (!data) return <div>데이터가 없습니다.</div>;src/pages/ChapterListPage.tsx (3)
12-17: Query key + retry policy LGTM; add cache window.Stable key and shared retry are good; consider a short staleTime.
- const { data, isPending, isError, error } = useQuery({ + const { data, isPending, isError, error } = useQuery({ queryKey: ['chapter-list'], queryFn: fetchChapters, - retry: shouldRetryApiRequest, + staleTime: 60_000, + retry: shouldRetryApiRequest, });
19-23: Inline onError can replace the effect.Keeps error handling colocated with the query and avoids a render pass.
- useEffect(() => { - if (isError && error instanceof ApiError) { - handleApiError(error); - } - }, [isError, error, handleApiError]); + // Alternative: + // const { data, ... } = useQuery({ ..., onError: (err) => err instanceof ApiError && handleApiError(err) })
25-27: Empty-state UX (optional).Consider a message when data is an empty list.
Example (outside diff scope):
if (!isPending && !isError && data?.length === 0) return <div>챕터가 없습니다.</div>;Also applies to: 35-41
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/components/chapter-page/Tooltip.tsx(1 hunks)src/context/ToastContextProvider.tsx(1 hunks)src/hooks/useAuthHandler.tsx(1 hunks)src/pages/ChapterDetailPage.tsx(2 hunks)src/pages/ChapterListPage.tsx(1 hunks)src/pages/MainPage.tsx(1 hunks)src/routes/router.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/routes/router.tsx
- src/hooks/useAuthHandler.tsx
- src/context/ToastContextProvider.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/pages/MainPage.tsx (4)
src/hooks/useAuthHandler.tsx (1)
useAuthHandler(6-34)src/api/fetchMainInfo.ts (1)
fetchMainInfo(5-47)src/utils/api-retry.ts (1)
shouldRetryApiRequest(3-13)src/types/@common/api.ts (1)
ApiError(1-11)
src/pages/ChapterListPage.tsx (4)
src/hooks/useAuthHandler.tsx (1)
useAuthHandler(6-34)src/api/fetchChapters.ts (1)
fetchChapters(5-45)src/utils/api-retry.ts (1)
shouldRetryApiRequest(3-13)src/types/@common/api.ts (1)
ApiError(1-11)
src/pages/ChapterDetailPage.tsx (4)
src/hooks/useAuthHandler.tsx (1)
useAuthHandler(6-34)src/api/fetchUnits.ts (2)
fetchUnits(5-52)fetchUnits(5-42)src/utils/api-retry.ts (1)
shouldRetryApiRequest(3-13)src/types/@common/api.ts (1)
ApiError(1-11)
🔇 Additional comments (2)
src/pages/ChapterDetailPage.tsx (1)
46-50: Auth error handler usage LGTM.Effect is correctly gated to ApiError; no redirect on non-auth errors.
src/pages/MainPage.tsx (1)
16-16: Good: centralized auth error handler wired.Consistent with shared hook.
| const { | ||
| data: units, | ||
| isPending, | ||
| isError, | ||
| error, | ||
| } = useQuery({ | ||
| queryKey: ['unit-list', { id: chapterId }], | ||
| queryFn: () => fetchUnits(Number(chapterId)), | ||
| retry: shouldRetryApiRequest, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard query with enabled; prevent NaN requests and add cache window.
When chapterId is undefined or non-numeric, fetchUnits throws INVALID_ARGUMENT. Gate the query and optionally cache results.
} = useQuery({
queryKey: ['unit-list', { id: chapterId }],
queryFn: () => fetchUnits(Number(chapterId)),
- retry: shouldRetryApiRequest,
+ enabled: Number.isFinite(Number(chapterId)),
+ staleTime: 60_000,
+ retry: shouldRetryApiRequest,
});📝 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.
| const { | |
| data: units, | |
| isPending, | |
| isError, | |
| error, | |
| } = useQuery({ | |
| queryKey: ['unit-list', { id: chapterId }], | |
| queryFn: () => fetchUnits(Number(chapterId)), | |
| retry: shouldRetryApiRequest, | |
| }); | |
| const { | |
| data: units, | |
| isPending, | |
| isError, | |
| error, | |
| } = useQuery({ | |
| queryKey: ['unit-list', { id: chapterId }], | |
| queryFn: () => fetchUnits(Number(chapterId)), | |
| enabled: Number.isFinite(Number(chapterId)), | |
| staleTime: 60_000, | |
| retry: shouldRetryApiRequest, | |
| }); |
🤖 Prompt for AI Agents
In src/pages/ChapterDetailPage.tsx around lines 31 to 40, the useQuery is firing
even when chapterId is undefined or non-numeric which causes fetchUnits to throw
INVALID_ARGUMENT; guard the query with an enabled flag (e.g. enabled:
Boolean(chapterId) && !Number.isNaN(Number(chapterId))) so it only runs for
valid IDs, convert chapterId to a number inside the queryFn (or in the queryKey)
to avoid NaN, and add a cache window (staleTime or cacheTime) to reduce repeated
calls (for example set staleTime/cacheTime to a few minutes) so results are
cached while the page remains active.
🔍 작업 유형
📄 작업 내용
🔗 관련 이슈
✅ 체크리스트
🖼️ 🖥 구현 결과 (선택사항)
✔ 리뷰 요구사항
📋 참고 문서
Summary by CodeRabbit