[UNI-119] 길찾기 결과 바텀 시트 아래로 내릴 시 스크롤 끝까지 안되는 현상 #51
Conversation
| <AnimatedSheetContainer | ||
| isVisible={isDetailView && !isLoading} | ||
| className="absolute bottom-0 w-full left-0 bg-white rounded-t-2xl shadow-xl overflow-auto" | ||
| positionDelta={MAX_SHEET_HEIGHT} | ||
| transition={{ type: "spring", damping: 20, duration: 0.3 }} | ||
| height={sheetHeight} | ||
| className="bg-white rounded-t-2xl shadow-xl" | ||
| transition={{ type: "tween", duration: 0.3 }} | ||
| motionProps={{ | ||
| drag: "y", | ||
| dragControls, | ||
| dragListener: false, | ||
| dragConstraints: { | ||
| top: 0, | ||
| bottom: MIN_SHEET_HEIGHT, | ||
| bottom: 0, | ||
| }, | ||
| onDrag: handleDrag, | ||
| onDragEnd: handleDrag, | ||
| }} | ||
| > | ||
| <BottomSheetHandle dragControls={dragControls} /> | ||
|
|
||
| <div | ||
| className="w-full overflow-y-auto" | ||
| style={{ | ||
| height: MAX_SHEET_HEIGHT - BOTTOM_SHEET_HANDLE_HEIGHT, | ||
| height: sheetHeight - BOTTOM_SHEET_HANDLE_HEIGHT, | ||
| }} | ||
| > | ||
| <NavigationDescription isDetailView={isDetailView && !isLoading} /> | ||
| <NavigationDescription isDetailView={true} /> | ||
| <RouteList | ||
| routes={route.route} | ||
| originBuilding={route.originBuilding} | ||
| destinationBuilding={route.destinationBuilding} | ||
| /> | ||
| </div> | ||
| </AnimatedContainer> | ||
| </AnimatedSheetContainer> |
There was a problem hiding this comment.
LGTM!
애니메이션의 달인이 되었네요!
바텀시트 완성이 된다면 지도 페이지에도 적용해보겠습니다!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (33)
uniro_admin_frontend/src/components/log/logList.tsx (1)
8-10:
⚠️ Potential issueAdd key prop to mapped elements.
React requires a unique key prop for elements in an iteration to maintain component state and optimize rendering.
- {logCardData.map((log) => { - return <LogCard date={log.date} change={log.change} />; + {logCardData.map((log, index) => { + return <LogCard key={`log-${index}`} date={log.date} change={log.change} />; })}📝 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.{logCardData.map((log, index) => { return <LogCard key={`log-${index}`} date={log.date} change={log.change} />; })}🧰 Tools
🪛 Biome (1.9.4)
[error] 9-9: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
uniro_admin_frontend/Dockerfile (2)
1-1:
⚠️ Potential issueUse a more stable Node.js version.
node:22-slimis a very new and potentially unstable version. Consider using an LTS version likenode:20-slim.-FROM node:22-slim AS build +FROM node:20-slim AS build📝 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.FROM node:20-slim AS build
6-6:
⚠️ Potential issueAvoid using --legacy-peer-deps flag.
The
--legacy-peer-depsflag can lead to dependency conflicts. Consider resolving peer dependencies properly or usingnpm install --strict-peer-depsinstead.-RUN npm install --legacy-peer-deps --no-audit && npm run build +RUN npm install --strict-peer-deps --no-audit && npm run build📝 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.RUN npm install --strict-peer-deps --no-audit && npm run build.github/workflows/fe-admin-deploy.yml (2)
62-62:
⚠️ Potential issueUse safer Docker cleanup command.
The current Docker cleanup command might remove containers from other services. Use a more targeted approach.
- sudo docker rm $(sudo docker ps -a -q) || true + sudo docker ps -a --format '{{.ID}} {{.Names}}' \ + | egrep -v 'nginx-container|uniro-fe' \ + | awk '{print $1}' \ + | xargs -r sudo docker rm || 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.sudo docker ps -a --format '{{.ID}} {{.Names}}' \ | egrep -v 'nginx-container|uniro-fe' \ | awk '{print $1}' \ | xargs -r sudo docker rm || true
22-24:
⚠️ Potential issuePin action version to SHA for security.
Using version tags like
v2can be insecure. Pin to specific SHA for better security.- uses: "google-github-actions/auth@v2" + uses: "google-github-actions/auth@ef5d53e30bbcd8d0836f4288f5e50ff3e086997d"📝 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.uses: "google-github-actions/auth@ef5d53e30bbcd8d0836f4288f5e50ff3e086997d" with: credentials_json: ${{ secrets.GCP_SA_KEY }}uniro_frontend/src/pages/error.tsx (1)
1-1:
⚠️ Potential issueRename imported component to avoid shadowing global Error.
The current import shadows the global
Errorproperty. Consider renaming the import to be more specific.-import Error from "../components/error/Error"; +import ErrorComponent from "../components/error/Error";📝 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.import ErrorComponent from "../components/error/Error";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
uniro_frontend/src/hooks/useRedirectUndefined.tsx (1)
11-11:
⚠️ Potential issueFix dependency array spread in useEffect.
The spread operator in the dependency array is unnecessary and could cause performance issues. React's exhaustive-deps rule expects direct references.
- }, [...deps]); + }, [deps, navigate, url]);📝 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.}, [deps, navigate, url]);uniro_frontend/src/components/error/Error.tsx (1)
3-15:
⚠️ Potential issueRename component to avoid shadowing global Error property.
The component implementation looks good, but it shadows the global
Errorproperty. Consider renaming it to something more specific likeErrorMessageorErrorDisplay.Apply this diff to fix the shadowing issue:
-export default function Error() { +export default function ErrorMessage() { return ( <div className="w-[285px] h-[198px] flex flex-col items-center py-2 space-y-[14px]"> <ErrorIcon /> <div className="space-y-[6px]"> <h3 className="text-kor-heading2 font-semibold text-gray-900"> 일시적인 오류로 <br /> 데이터를 불러올 수 없습니다. </h3> <p className="text-kor-body2 font-medium text-gray-700">잠시 후 다시 시도해 주세요.</p> </div> </div> ); }📝 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.export default function ErrorMessage() { return ( <div className="w-[285px] h-[198px] flex flex-col items-center py-2 space-y-[14px]"> <ErrorIcon /> <div className="space-y-[6px]"> <h3 className="text-kor-heading2 font-semibold text-gray-900"> 일시적인 오류로 <br /> 데이터를 불러올 수 없습니다. </h3> <p className="text-kor-body2 font-medium text-gray-700">잠시 후 다시 시도해 주세요.</p> </div> </div> ); }🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
uniro_admin_frontend/src/map/loader/googleMapLoader.ts (1)
9-14: 🛠️ Refactor suggestion
Avoid shadowing the global Map object.
The destructured
Mapvariable shadows the globalMapobject, which could lead to confusion. Consider renaming it to be more specific.const loadGoogleMapsLibraries = async () => { - const { Map, OverlayView, Polyline } = await GoogleMapsLoader.importLibrary("maps"); + const { Map: GoogleMap, OverlayView, Polyline } = await GoogleMapsLoader.importLibrary("maps"); const { AdvancedMarkerElement } = await GoogleMapsLoader.importLibrary("marker"); - return { Map, OverlayView, AdvancedMarkerElement, Polyline }; + return { GoogleMap, OverlayView, AdvancedMarkerElement, Polyline }; };📝 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 loadGoogleMapsLibraries = async () => { const { Map: GoogleMap, OverlayView, Polyline } = await GoogleMapsLoader.importLibrary("maps"); const { AdvancedMarkerElement } = await GoogleMapsLoader.importLibrary("marker"); return { GoogleMap, OverlayView, AdvancedMarkerElement, Polyline }; };🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
uniro_admin_frontend/src/container/mapContainer.tsx (1)
10-10: 🛠️ Refactor suggestion
Replace hardcoded date/time with dynamic value.
The current implementation uses a static date/time string. Consider using a dynamic value that updates in real-time.
- <div className="text-kor-heading2">2025년 2월 3일 15:34</div> + <div className="text-kor-heading2"> + {new Date().toLocaleString('ko-KR', { + year: 'numeric', + month: 'long', + day: 'numeric', + hour: '2-digit', + minute: '2-digit', + hour12: false + })} + </div>📝 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.<div className="text-kor-heading2"> {new Date().toLocaleString('ko-KR', { year: 'numeric', month: 'long', day: 'numeric', hour: '2-digit', minute: '2-digit', hour12: false })} </div>uniro_frontend/src/hooks/useNetworkStatus.tsx (1)
18-26:
⚠️ Potential issueAdd dependency array to useEffect to prevent unnecessary re-renders
The useEffect hook is missing its dependency array, which could cause unnecessary re-renders and event listener re-attachments.
- }); + }, [navigate]);📝 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.useEffect(() => { window.addEventListener("offline", handleOffline); window.addEventListener("online", handleOnline); return () => { window.removeEventListener("offline", handleOffline); window.removeEventListener("online", handleOnline); }; }, [navigate]);uniro_frontend/src/utils/fetch/mockFetch.ts (1)
11-12: 🛠️ Refactor suggestion
Remove console logs from production code
Console logs should be removed or wrapped in a development-only condition.
+ const isDev = process.env.NODE_ENV === 'development'; + return new Promise((resolve, reject) => { - console.log(`${delay / 1000}초 동안 로딩 중...`); + isDev && console.log(`${delay / 1000}초 동안 로딩 중...`); setTimeout(() => { if (shouldFail) { - console.error("네트워크 오류 발생!"); + isDev && console.error("네트워크 오류 발생!"); reject(new Error("네트워크 오류 발생")); } else { - console.log("데이터 로드 완료:", data); + isDev && console.log("데이터 로드 완료:", data); resolve(data); } }, delay);Also applies to: 18-19
uniro_admin_frontend/src/component/Map.tsx (1)
8-8: 🛠️ Refactor suggestion
Rename component to avoid shadowing global Map object.
The component name shadows the global
Mapobject. Consider using a more specific name.-const Map = ({ style }: MapProps) => { +const MapComponent = ({ style }: MapProps) => {📝 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 MapComponent = ({ style }: MapProps) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
uniro_frontend/src/components/map/reportModal.tsx (3)
14-20:
⚠️ Potential issuePrevent modal close when clicking links.
The modal currently closes when clicking anywhere including the links due to event bubbling.
<Link to={"/report/route"} className="w-full h-[58px] flex items-center justify-between px-5 bg-gray-100 rounded-400 text-kor-body2 font-semibold text-primary-500" + onClick={(e) => e.stopPropagation()} >Also applies to: 22-28
1-1:
⚠️ Potential issueFix the import statement for React Router.
The import statement for React Router appears incorrect. It should be
react-router-dominstead ofreact-router.-import { Link } from "react-router"; +import { Link } from "react-router-dom";📝 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.import { Link } from "react-router-dom";
10-13: 🛠️ Refactor suggestion
Add accessibility attributes to the modal overlay.
The modal overlay should have proper ARIA attributes and keyboard handling for accessibility.
<div className="fixed inset-0 flex flex-col items-center justify-end bg-[rgba(0,0,0,0.2)] z-20 py-6 px-4 space-y-[10px]" onClick={close} + role="dialog" + aria-modal="true" + onKeyDown={(e) => e.key === 'Escape' && close()} + tabIndex={-1} >📝 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.<div className="fixed inset-0 flex flex-col items-center justify-end bg-[rgba(0,0,0,0.2)] z-20 py-6 px-4 space-y-[10px]" onClick={close} role="dialog" aria-modal="true" onKeyDown={(e) => e.key === 'Escape' && close()} tabIndex={-1} >uniro_frontend/src/utils/polylines/createSubnodes.ts (1)
6-32: 🛠️ Refactor suggestion
Add edge case handling and improve mathematical accuracy.
The current implementation has several potential improvements:
- No validation for empty paths or invalid input
- Linear interpolation might not be accurate for long distances on Earth's surface
- No handling for the case when
lengthis 0export default function createSubNodes(polyLine: google.maps.Polyline): google.maps.LatLngLiteral[] { const paths = polyLine.getPath(); + if (paths.getLength() < 2) { + throw new Error('Polyline must have at least 2 points'); + } const [startNode, endNode] = paths.getArray().map((el) => LatLngToLiteral(el)); const length = distance(startNode, endNode); + if (length === 0) { + return [startNode]; + } const subEdgesCount = Math.ceil(length / EDGE_LENGTH); const interval = { lat: (endNode.lat - startNode.lat) / subEdgesCount, lng: (endNode.lng - startNode.lng) / subEdgesCount, }; const subNodes = []; for (let i = 0; i < subEdgesCount; i++) { const subNode: google.maps.LatLngLiteral = { lat: startNode.lat + interval.lat * i, lng: startNode.lng + interval.lng * i, }; subNodes.push(subNode); } subNodes.push(endNode); return subNodes; }Consider using spherical interpolation for better accuracy over long distances:
function sphericalInterpolate(start: google.maps.LatLngLiteral, end: google.maps.LatLngLiteral, fraction: number): google.maps.LatLngLiteral { const d = distance(start, end); const a = Math.sin((1 - fraction) * d) / Math.sin(d); const b = Math.sin(fraction * d) / Math.sin(d); return { lat: a * start.lat + b * end.lat, lng: a * start.lng + b * end.lng }; }uniro_frontend/src/utils/polylines/findNearestEdge.ts (1)
4-32: 🛠️ Refactor suggestion
Add input validation and optimize performance.
The current implementation has several areas for improvement:
- No validation for empty edges array
- Unnecessary sorting of all edges when only the nearest is needed
- Type assertions could be replaced with proper typing
export default function findNearestSubEdge( edges: [google.maps.LatLngLiteral, google.maps.LatLngLiteral][], point: google.maps.LatLngLiteral, ): { edge: [google.maps.LatLngLiteral, google.maps.LatLngLiteral]; point: google.maps.LatLngLiteral; } { + if (edges.length === 0) { + throw new Error('Edges array cannot be empty'); + } + + let nearestEdge: [google.maps.LatLngLiteral, google.maps.LatLngLiteral] | null = null; + let minDistance = Infinity; + + for (const [startNode, endNode] of edges) { + const center = centerCoordinate(startNode, endNode); + const dist = distance(point, center); + if (dist < minDistance) { + minDistance = dist; + nearestEdge = [startNode, endNode]; + } + } + + if (!nearestEdge) { + throw new Error('Failed to find nearest edge'); + } - const edgesWithDistance = edges - .map(([startNode, endNode]) => { - return { - edge: [startNode, endNode] as [google.maps.LatLngLiteral, google.maps.LatLngLiteral], - distance: distance(point, centerCoordinate(startNode, endNode)), - }; - }) - .sort((a, b) => { - return a.distance - b.distance; - }); - - const nearestEdge = edgesWithDistance[0].edge; - const distance0 = distance(nearestEdge[0], point); const distance1 = distance(nearestEdge[1], point); const nearestPoint = distance0 > distance1 ? nearestEdge[1] : nearestEdge[0]; return { edge: nearestEdge, point: nearestPoint, }; }📝 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.export default function findNearestSubEdge( edges: [google.maps.LatLngLiteral, google.maps.LatLngLiteral][], point: google.maps.LatLngLiteral, ): { edge: [google.maps.LatLngLiteral, google.maps.LatLngLiteral]; point: google.maps.LatLngLiteral; } { if (edges.length === 0) { throw new Error('Edges array cannot be empty'); } let nearestEdge: [google.maps.LatLngLiteral, google.maps.LatLngLiteral] | null = null; let minDistance = Infinity; for (const [startNode, endNode] of edges) { const center = centerCoordinate(startNode, endNode); const dist = distance(point, center); if (dist < minDistance) { minDistance = dist; nearestEdge = [startNode, endNode]; } } if (!nearestEdge) { throw new Error('Failed to find nearest edge'); } const distance0 = distance(nearestEdge[0], point); const distance1 = distance(nearestEdge[1], point); const nearestPoint = distance0 > distance1 ? nearestEdge[1] : nearestEdge[0]; return { edge: nearestEdge, point: nearestPoint, }; }uniro_frontend/src/map/initializer/googleMapInitializer.ts (1)
16-16:
⚠️ Potential issueAvoid shadowing the global Map object.
The destructured variable name shadows the global
Mapobject, which could lead to confusion.- const { Map, OverlayView, AdvancedMarkerElement, Polyline } = await loadGoogleMapsLibraries(); + const { Map: GoogleMap, OverlayView, AdvancedMarkerElement, Polyline } = await loadGoogleMapsLibraries(); - const map = new Map(mapElement, { + const map = new GoogleMap(mapElement, {📝 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 { Map: GoogleMap, OverlayView, AdvancedMarkerElement, Polyline } = await loadGoogleMapsLibraries(); // ... other code ... const map = new GoogleMap(mapElement, { // map options });🧰 Tools
🪛 Biome (1.9.4)
[error] 16-16: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
uniro_admin_frontend/src/hooks/useMap.tsx (2)
40-40:
⚠️ Potential issueAdd mapOptions to useEffect dependencies.
The useEffect hook should include mapOptions in its dependencies array to properly handle updates to map options.
-}, []); +}, [mapOptions]);📝 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.}, [mapOptions]);
28-30: 🛠️ Refactor suggestion
Improve error handling.
Using alert for error handling is not ideal for production. Consider implementing a more robust error handling mechanism.
-alert("Error while initializing map: " + e); +console.error("Map initialization failed:", e); +// Consider using a proper error boundary or error state +setMapLoaded(false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.} catch (e) { console.error("Map initialization failed:", e); // Consider using a proper error boundary or error state setMapLoaded(false); }uniro_frontend/src/utils/fetch/fetch.ts (2)
20-31: 🛠️ Refactor suggestion
Add Content-Type header for POST/PUT requests.
The POST and PUT methods are sending JSON data but missing the Content-Type header. This could cause issues with the server interpreting the request body correctly.
Apply this diff to both POST and PUT methods:
const post = async <T, K>(url: string, body?: Record<string, K>): Promise<T> => { const response = await fetch(`${baseURL}${url}`, { method: "POST", body: JSON.stringify(body), + headers: { + 'Content-Type': 'application/json', + }, });Also applies to: 33-44
17-17: 🛠️ Refactor suggestion
Add error handling for JSON parsing.
The response.json() calls could fail if the response is not valid JSON. Consider adding try-catch blocks to handle parsing errors gracefully.
Apply this diff to all methods:
- return response.json(); + try { + return await response.json(); + } catch (error) { + throw new Error(`Failed to parse JSON response: ${error.message}`); + }Also applies to: 30-30, 43-43
uniro_frontend/src/pages/universitySearch.tsx (1)
6-6:
⚠️ Potential issueFix incorrect import path.
The import path for
react-routershould bereact-router-domfor web applications.Apply this diff:
-import { Link } from "react-router"; +import { Link } from "react-router-dom";📝 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.import { Link } from "react-router-dom";uniro_frontend/src/components/report/secondaryForm.tsx (1)
32-42: 🛠️ Refactor suggestion
Replace array index with unique identifier as key prop.
Using array indices as keys in map functions can lead to rendering issues when the array items are reordered or modified.
The
buttonvalue itself can be used as the key since it's an enum value and therefore unique:-key={index} +key={button}Also applies to: 44-54
uniro_frontend/src/pages/reportForm.tsx (2)
110-112: 🛠️ Refactor suggestion
Add loading state to prevent multiple submissions.
The submit button doesn't handle loading state, which could lead to multiple submissions.
Consider adding loading state:
+const [isSubmitting, setIsSubmitting] = useState(false); +const handleSubmit = async () => { + if (isSubmitting) return; + setIsSubmitting(true); + try { + await onReportSubmitSuccess(); + } catch (error) { + onReportSubmitFail(); + } finally { + setIsSubmitting(false); + } +}; -<Button onClick={onReportSubmitSuccess} variant={disabled ? "disabled" : "primary"}> +<Button + onClick={handleSubmit} + variant={disabled || isSubmitting ? "disabled" : "primary"} +> - 제보하기 + {isSubmitting ? "제보 중..." : "제보하기"} </Button>📝 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.import React, { useState } from "react"; // ... other imports const ReportForm = (props) => { // ... other state declarations const [isSubmitting, setIsSubmitting] = useState(false); const handleSubmit = async () => { if (isSubmitting) return; setIsSubmitting(true); try { await onReportSubmitSuccess(); } catch (error) { onReportSubmitFail(); } finally { setIsSubmitting(false); } }; // ... other functions and JSX return ( // ... other JSX elements <Button onClick={handleSubmit} variant={disabled || isSubmitting ? "disabled" : "primary"} > {isSubmitting ? "제보 중..." : "제보하기"} </Button> // ... other JSX elements ); }; export default ReportForm;
39-45: 🛠️ Refactor suggestion
Consider removing console.log statement and adding proper error handling.
The
useEffecthook contains a console.log statement and uses setTimeout without cleanup.Apply this diff to improve the implementation:
useEffect(() => { - console.log(reportType, startNode, endNode); + let timeoutId: NodeJS.Timeout; - setTimeout(() => { + timeoutId = setTimeout(() => { setReportMode("update"); }, 2000); + + return () => clearTimeout(timeoutId); }, []);📝 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.useEffect(() => { let timeoutId: NodeJS.Timeout; timeoutId = setTimeout(() => { setReportMode("update"); }, 2000); return () => clearTimeout(timeoutId); }, []);uniro_frontend/src/pages/demo.tsx (3)
73-74: 🛠️ Refactor suggestion
Add proper click handlers for toggle buttons.
The toggle buttons have empty click handlers.
Consider implementing proper handlers:
-<DangerToggleButton onClick={() => {}} isActive={false} /> -<DangerToggleButton onClick={() => {}} isActive={true} /> +<DangerToggleButton onClick={() => setDangerActive(prev => !prev)} isActive={dangerActive} /> +<DangerToggleButton onClick={() => setDangerActive(prev => !prev)} isActive={dangerActive} /> -<CautionToggleButton onClick={() => {}} isActive={false} /> -<CautionToggleButton onClick={() => {}} isActive={true} /> +<CautionToggleButton onClick={() => setCautionActive(prev => !prev)} isActive={cautionActive} /> +<CautionToggleButton onClick={() => setCautionActive(prev => !prev)} isActive={cautionActive} />Also applies to: 78-79
20-25: 🛠️ Refactor suggestion
Add error handling to API calls.
The
getTestfunction lacks proper error handling and type safety.Consider adding error handling and proper typing:
+interface GetTestResponse { + postId: string; +} -const getTest = () => { +const getTest = async (): Promise<GetTestResponse> => { + try { /** https://jsonplaceholder.typicode.com/comments?postId=1 */ - return getFetch<{ postId: string }>("/comments", { + return await getFetch<GetTestResponse>("/comments", { postId: 1, }); + } catch (error) { + console.error('Failed to fetch test data:', error); + throw error; + } };📝 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.interface GetTestResponse { postId: string; } const getTest = async (): Promise<GetTestResponse> => { try { /** https://jsonplaceholder.typicode.com/comments?postId=1 */ return await getFetch<GetTestResponse>("/comments", { postId: 1, }); } catch (error) { console.error('Failed to fetch test data:', error); throw error; } };
17-18:
⚠️ Potential issueRename imported Error component to avoid global name conflict.
The imported Error component shadows the global Error object.
Apply this diff to fix the naming conflict:
-import Error from "../components/error/Error"; +import ErrorComponent from "../components/error/Error"; // Update usage -<Error /> +<ErrorComponent />📝 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.import Offline from "../components/error/Offline"; import ErrorComponent from "../components/error/Error";🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
uniro_frontend/src/pages/reportHazard.tsx (1)
164-186:
⚠️ Potential issueAdd cleanup for map event listeners.
Event listeners are added but never cleaned up, which could lead to memory leaks.
Consider adding cleanup:
useEffect(() => { + const listeners: google.maps.MapsEventListener[] = []; if (map) { - map.addListener("click", () => { + listeners.push(map.addListener("click", () => { setReportMarker((prevMarker) => { if (prevMarker) { setMessage(ReportHazardMessage.DEFAULT); resetMarker(prevMarker); } else setMessage(ReportHazardMessage.ERROR); return undefined; }); - }); + })); } + return () => { + listeners.forEach(listener => listener.remove()); + }; }, [map, AdvancedMarker, Polyline]);📝 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.useEffect(() => { drawRoute(mockNavigationRoute.route); addHazardMarker(); + const listeners: google.maps.MapsEventListener[] = []; if (map) { - map.addListener("click", () => { + listeners.push(map.addListener("click", () => { setReportMarker((prevMarker) => { if (prevMarker) { setMessage(ReportHazardMessage.DEFAULT); resetMarker(prevMarker); } else setMessage(ReportHazardMessage.ERROR); return undefined; }); - }); + })); } + return () => { + listeners.forEach(listener => listener.remove()); + }; }, [map, AdvancedMarker, Polyline]);uniro_frontend/src/pages/reportRoute.tsx (1)
221-256:
⚠️ Potential issueAdd cleanup for map resources and event listeners.
Map resources and event listeners should be properly cleaned up.
Consider adding cleanup:
useEffect(() => { + const markers: google.maps.Marker[] = []; + const polylines: google.maps.Polyline[] = []; + const listeners: google.maps.MapsEventListener[] = []; drawRoute(mockNavigationRoute.route); addHazardMarker(); if (Polyline) { newPolyLine.current = new Polyline({ map: map, path: [], strokeColor: "#0367FB" }); + polylines.push(newPolyLine.current); } if (map && AdvancedMarker) { - map.addListener("click", (e: ClickEvent) => { + listeners.push(map.addListener("click", (e: ClickEvent) => { // ... existing click handler code - }); + })); } + return () => { + markers.forEach(marker => marker.setMap(null)); + polylines.forEach(polyline => polyline.setMap(null)); + listeners.forEach(listener => listener.remove()); + }; }, [map]);📝 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.useEffect(() => { const markers: google.maps.Marker[] = []; const polylines: google.maps.Polyline[] = []; const listeners: google.maps.MapsEventListener[] = []; drawRoute(mockNavigationRoute.route); addHazardMarker(); if (Polyline) { newPolyLine.current = new Polyline({ map: map, path: [], strokeColor: "#0367FB" }); polylines.push(newPolyLine.current); } if (map && AdvancedMarker) { listeners.push(map.addListener("click", (e: ClickEvent) => { if (originPoint.current) { const point = LatLngToLiteral(e.latLng); setNewPoints((prevPoints) => { if (prevPoints.element) { prevPoints.element.position = point; return { ...prevPoints, coords: [...prevPoints.coords, point], }; } else { setIsActive(true); return { element: new AdvancedMarker({ map: map, position: point, content: createMarkerElement({ type: Markers.DESTINATION, }), }), coords: [...prevPoints.coords, point], }; } }); } })); } return () => { markers.forEach(marker => marker.setMap(null)); polylines.forEach(polyline => polyline.setMap(null)); listeners.forEach(listener => listener.remove()); }; }, [map]);uniro_frontend/src/constant/enum/routeEnum.ts (1)
1-4:
⚠️ Potential issueRemove
constmodifier from enum declaration.The
constenum can cause issues with bundlers and is incompatible with the 'isolatedModules' mode. This could lead to importing non-existent values.Apply this diff to fix the issue:
-export const enum RoutePoint { +export enum RoutePoint { ORIGIN = "origin", DESTINATION = "destination", }📝 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.export enum RoutePoint { ORIGIN = "origin", DESTINATION = "destination", }🧰 Tools
🪛 Biome (1.9.4)
[error] 1-4: The enum declaration should not be const
Const enums are not supported by bundlers and are incompatible with the 'isolatedModules' mode. Their use can lead to import inexistent values.
See TypeScript Docs for more details.
Safe fix: Turn the const enum into a regular enum.(lint/suspicious/noConstEnum)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 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 (
|
#️⃣ 작업 내용
해결 방식
AS IS
2025-02-05.16.33.02.mov
TO BE
2025-02-05.16.31.35.mov
Summary by CodeRabbit
New Features
Improvements