NetManagerMap Integration #2526
Conversation
…frontend into NetManagerMapFunc
…gestions as the user types
…frontend into NetManagerMapFunc
…frontend into NetManagerMapFunc
…frontend into NetManagerMapFunc
📝 WalkthroughWalkthroughThis update integrates an enhanced network mapping interface. A new React component named Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant NM as NetManagerMap Component
participant API as MapData API
participant Map as Mapbox Map Instance
U->>NM: Open Map View
NM->>API: Call GetAirQuoData(token)
API-->>NM: Return air quality data
NM->>NM: Convert data to GeoJSON (ConvertToGeojson)
NM->>Map: Add GeoJSON source to map
Map-->>NM: Render updated map
sequenceDiagram
participant U as User
participant NM as NetManagerMap Component
participant API as MapData API
participant Map as Mapbox Map
U->>NM: Enter search query
NM->>API: Call FetchSuggestions(query, token, session, lat, lon)
API-->>NM: Return suggestions
NM->>U: Display suggestion list
U->>NM: Select suggestion
NM->>API: Call UserClick(token, session, locationid)
API-->>NM: Return location data
NM->>Map: Update map view to selected location
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 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 (
|
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | 769b61a | netmanager-app/components/NetManagerMap/page.tsx | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (8)
src/netmanager-app/package.json (1)
39-39: Consider pinning critical dependencies to exact versions.While using caret (^) versioning allows for automatic minor version updates, consider pinning
mapbox-glto an exact version since it's a critical dependency for map functionality. This helps prevent unexpected breaking changes during builds.- "framer-motion": "^12.4.7", + "framer-motion": "12.4.7", - "mapbox-gl": "^3.10.0", + "mapbox-gl": "3.10.0",Also applies to: 44-44
src/netmanager-app/public/icons/map/layerIcon.js (1)
3-21: Add TypeScript types and improve accessibility.Consider the following improvements:
- Add TypeScript interface for props
- Add ARIA attributes for accessibility
- Define default props separately
+interface LayerIconProps { + width?: string; + height?: string; + fill?: string; +} -const layerIcon = ({ width, height, fill }) => { +const layerIcon = ({ width = '24', height = '24', fill = '#536A87' }: LayerIconProps) => { return ( <svg xmlns="http://www.w3.org/2000/svg" - width={width || '24'} - height={height || '24'} + width={width} + height={height} viewBox="0 0 24 24" fill="none" + role="img" + aria-label="Layer icon" > <path d="M7 11.9996L2 14.4996L11.6422 19.3207C11.7734 19.3863 11.839 19.4191 11.9078 19.432C11.9687 19.4434 12.0313 19.4434 12.0922 19.432C12.161 19.4191 12.2266 19.3863 12.3578 19.3207L22 14.4996L17 11.9996M2 9.49958L11.6422 4.67846C11.7734 4.61287 11.839 4.58008 11.9078 4.56717C11.9687 4.55574 12.0313 4.55574 12.0922 4.56717C12.161 4.58008 12.2266 4.61287 12.3578 4.67846L22 9.49958L12.3578 14.3207C12.2266 14.3863 12.161 14.4191 12.0922 14.432C12.0313 14.4434 11.9687 14.4434 11.9078 14.432C11.839 14.4191 11.7734 14.3863 11.6422 14.3207L2 9.49958Z" - stroke={fill || '#536A87'} + stroke={fill} strokeWidth="1.5" strokeLinecap="round" strokeLinejoin="round" /> </svg> ); };src/netmanager-app/public/icons/map/shareIcon.js (1)
1-23: Consider creating a base icon component.The icon components share identical structure and props. Consider creating a reusable base component to reduce code duplication.
+// BaseIcon.tsx +interface BaseIconProps { + width?: string; + height?: string; + fill?: string; + path: string; + label: string; +} + +const BaseIcon = ({ width = '24', height = '24', fill = '#536A87', path, label }: BaseIconProps) => ( + <svg + xmlns="http://www.w3.org/2000/svg" + width={width} + height={height} + viewBox="0 0 24 24" + fill="none" + role="img" + aria-label={label} + > + <path + d={path} + stroke={fill} + strokeWidth="1.5" + strokeLinecap="round" + strokeLinejoin="round" + /> + </svg> +); + +// shareIcon.tsx +const shareIcon = (props: Omit<BaseIconProps, 'path' | 'label'>) => ( + <BaseIcon + {...props} + path="M9.99825 13C10.4277 13.5741..." + label="Share icon" + /> +);src/netmanager-app/components/NetManagerMap/components/IconButton.tsx (1)
9-19: Consider enhancing button accessibility.The button implementation is clean and functional. Consider adding
aria-labelfor better screen reader support.<button onClick={onClick} title={title} + aria-label={title} className="inline-flex items-center justify-center p-2 md:p-3 mr-2 text-gray-600 rounded-full bg-white focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-blue-500 shadow-md" >src/netmanager-app/components/NetManagerMap/components/zoom.tsx (1)
75-86: Consider using URL state management library.The URL state management is implemented directly with URL API. Consider using a routing library like Next.js's router or react-router for more robust URL handling.
src/netmanager-app/lib/utils.ts (1)
81-115: Add error handling for map style refresh.The map style refresh could fail if the style URL is malformed. Consider adding validation.
if (map) { try { const originalStyle = map.getStyle().sprite.split('/').slice(0, -1).join('/') + '/style.json'; + if (!originalStyle.startsWith('mapbox://styles/')) { + throw new Error('Invalid style URL'); + } map.setStyle(originalStyle);src/netmanager-app/components/NetManagerMap/page.tsx (1)
72-74: Remove console.log statements before production.Remove debugging console.log statements for production deployment.
- console.log(Data) const geojsonData = ConvertToGeojson(Data); - console.log('GeoJson Data : ',geojsonData)src/netmanager-app/components/NetManagerMap/components/LayerModal.js (1)
166-166: Strengthen PropTypes validation for mapDetails.The
mapDetailsprop should have a more specific shape validation similar tomapStyles.Apply this diff to improve type safety:
- mapDetails: PropTypes.array, + mapDetails: PropTypes.arrayOf( + PropTypes.shape({ + name: PropTypes.string.isRequired, + image: PropTypes.oneOfType([PropTypes.string, PropTypes.object]).isRequired, + }) + ).isRequired,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
src/netmanager-app/package-lock.jsonis excluded by!**/package-lock.jsonsrc/netmanager-app/public/images/map/GoodAir.pngis excluded by!**/*.pngsrc/netmanager-app/public/images/map/Hazardous.pngis excluded by!**/*.pngsrc/netmanager-app/public/images/map/Moderate.pngis excluded by!**/*.pngsrc/netmanager-app/public/images/map/Unhealthy.pngis excluded by!**/*.pngsrc/netmanager-app/public/images/map/UnhealthySG.pngis excluded by!**/*.pngsrc/netmanager-app/public/images/map/UnknownAQ.pngis excluded by!**/*.pngsrc/netmanager-app/public/images/map/VeryUnhealthy.pngis excluded by!**/*.pngsrc/platform/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
src/netmanager-app/app/(authenticated)/network-map/page.tsx(1 hunks)src/netmanager-app/components.json(1 hunks)src/netmanager-app/components/NetManagerMap/components/IconButton.tsx(1 hunks)src/netmanager-app/components/NetManagerMap/components/LayerModal.js(1 hunks)src/netmanager-app/components/NetManagerMap/components/zoom.tsx(1 hunks)src/netmanager-app/components/NetManagerMap/data/constants.js(1 hunks)src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)src/netmanager-app/core/apis/MapData.ts(1 hunks)src/netmanager-app/lib/utils.ts(1 hunks)src/netmanager-app/package.json(1 hunks)src/netmanager-app/public/icons/map/layerIcon.js(1 hunks)src/netmanager-app/public/icons/map/refreshIcon.js(1 hunks)src/netmanager-app/public/icons/map/shareIcon.js(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/netmanager-app/app/(authenticated)/network-map/page.tsx
- src/netmanager-app/components.json
- src/netmanager-app/components/NetManagerMap/data/constants.js
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/core/apis/MapData.ts
[error] 21-21: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 10-13: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 92-92: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/netmanager-app/components/NetManagerMap/components/LayerModal.js
[error] 158-158: This property value named onMapDetailsSelect is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named onMapDetailsSelect
(lint/suspicious/noDuplicateObjectKeys)
[error] 159-159: This property value named onStyleSelect is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named onStyleSelect
(lint/suspicious/noDuplicateObjectKeys)
🔇 Additional comments (2)
src/netmanager-app/public/icons/map/refreshIcon.js (1)
1-23: Apply the same base icon component pattern.This component follows the same pattern as
layerIconandshareIcon. It would benefit from the same base component refactoring suggested earlier.src/netmanager-app/components/NetManagerMap/components/IconButton.tsx (1)
3-7: Props interface looks good!The TypeScript interface is well-defined with proper types for all props.
| createButton(title, component, onClick) { | ||
| const button = document.createElement('button'); | ||
| button.className = | ||
| 'mapboxgl-ctrl-icon rounded-full m-1 md:m-2 flex items-center justify-center'; | ||
| button.type = 'button'; | ||
| button.title = title; | ||
|
|
||
| const div = document.createElement('div'); | ||
| div.className = 'flex items-center justify-center h-full w-full'; | ||
| button.appendChild(div); | ||
|
|
||
| const root = createRoot(div); | ||
| if (component) { | ||
| root.render(React.cloneElement(component)); | ||
| } else { | ||
| console.error(`${title} icon component is missing.`); | ||
| } | ||
|
|
||
| button.addEventListener('click', onClick); | ||
| return button; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve React component integration.
The current implementation manually creates DOM elements and renders React components. Consider using React portals for a more idiomatic approach.
createButton(title, component, onClick) {
- const button = document.createElement('button');
- const div = document.createElement('div');
- div.className = 'flex items-center justify-center h-full w-full';
- button.appendChild(div);
- const root = createRoot(div);
+ const container = document.createElement('div');
+ const portal = createRoot(container);
+ portal.render(
+ <button
+ className="mapboxgl-ctrl-icon rounded-full m-1 md:m-2 flex items-center justify-center"
+ type="button"
+ title={title}
+ onClick={onClick}
+ >
+ <div className="flex items-center justify-center h-full w-full">
+ {component}
+ </div>
+ </button>
+ );
- if (component) {
- root.render(React.cloneElement(component));
- } else {
- console.error(`${title} icon component is missing.`);
- }
- button.addEventListener('click', onClick);
- return button;
+ return container;
}📝 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.
| createButton(title, component, onClick) { | |
| const button = document.createElement('button'); | |
| button.className = | |
| 'mapboxgl-ctrl-icon rounded-full m-1 md:m-2 flex items-center justify-center'; | |
| button.type = 'button'; | |
| button.title = title; | |
| const div = document.createElement('div'); | |
| div.className = 'flex items-center justify-center h-full w-full'; | |
| button.appendChild(div); | |
| const root = createRoot(div); | |
| if (component) { | |
| root.render(React.cloneElement(component)); | |
| } else { | |
| console.error(`${title} icon component is missing.`); | |
| } | |
| button.addEventListener('click', onClick); | |
| return button; | |
| } | |
| createButton(title, component, onClick) { | |
| const container = document.createElement('div'); | |
| const portal = createRoot(container); | |
| portal.render( | |
| <button | |
| className="mapboxgl-ctrl-icon rounded-full m-1 md:m-2 flex items-center justify-center" | |
| type="button" | |
| title={title} | |
| onClick={onClick} | |
| > | |
| <div className="flex items-center justify-center h-full w-full"> | |
| {component} | |
| </div> | |
| </button> | |
| ); | |
| return container; | |
| } |
| export const ConvertToGeojson=(data: any)=>{ | ||
| return { | ||
| type: "FeatureCollection" as const, | ||
| features: data.measurements.map((item: any) => ({ | ||
| type: "Feature", | ||
| geometry: { | ||
| type: "Point", | ||
| coordinates: [ | ||
| item.siteDetails.approximate_longitude, | ||
| item.siteDetails.approximate_latitude | ||
| ] | ||
| }, | ||
| properties: { | ||
| id: item._id, | ||
| site_id:item.site_id, | ||
| time: new Date(item.time).toLocaleDateString("en-US", { | ||
| weekday: "long", | ||
| year: "numeric", | ||
| month: "long", | ||
| day: "numeric" | ||
| }), | ||
| location_name:item.siteDetails.name, | ||
| aqi_category: item.aqi_category, | ||
| aqi_color:item.aqi_color.startsWith("#") ? item.aqi_color : `#${item.aqi_color}`, | ||
| value:item.pm2_5 ?.value??0, | ||
| } | ||
| })) | ||
| } | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety to GeoJSON conversion.
The function uses any type and lacks input validation. Consider adding proper types and validation.
+interface MeasurementData {
+ measurements: Array<{
+ _id: string;
+ site_id: string;
+ time: string;
+ siteDetails: {
+ name: string;
+ approximate_longitude: number;
+ approximate_latitude: number;
+ };
+ aqi_category: string;
+ aqi_color: string;
+ pm2_5?: { value: number };
+ }>;
+}
-export const ConvertToGeojson=(data: any)=>{
+export const ConvertToGeojson=(data: MeasurementData)=>{
+ if (!data?.measurements?.length) {
+ throw new Error('Invalid data format');
+ }📝 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 const ConvertToGeojson=(data: any)=>{ | |
| return { | |
| type: "FeatureCollection" as const, | |
| features: data.measurements.map((item: any) => ({ | |
| type: "Feature", | |
| geometry: { | |
| type: "Point", | |
| coordinates: [ | |
| item.siteDetails.approximate_longitude, | |
| item.siteDetails.approximate_latitude | |
| ] | |
| }, | |
| properties: { | |
| id: item._id, | |
| site_id:item.site_id, | |
| time: new Date(item.time).toLocaleDateString("en-US", { | |
| weekday: "long", | |
| year: "numeric", | |
| month: "long", | |
| day: "numeric" | |
| }), | |
| location_name:item.siteDetails.name, | |
| aqi_category: item.aqi_category, | |
| aqi_color:item.aqi_color.startsWith("#") ? item.aqi_color : `#${item.aqi_color}`, | |
| value:item.pm2_5 ?.value??0, | |
| } | |
| })) | |
| } | |
| } | |
| interface MeasurementData { | |
| measurements: Array<{ | |
| _id: string; | |
| site_id: string; | |
| time: string; | |
| siteDetails: { | |
| name: string; | |
| approximate_longitude: number; | |
| approximate_latitude: number; | |
| }; | |
| aqi_category: string; | |
| aqi_color: string; | |
| pm2_5?: { value: number }; | |
| }>; | |
| } | |
| export const ConvertToGeojson = (data: MeasurementData) => { | |
| if (!data?.measurements?.length) { | |
| throw new Error('Invalid data format'); | |
| } | |
| return { | |
| type: "FeatureCollection" as const, | |
| features: data.measurements.map((item: any) => ({ | |
| type: "Feature", | |
| geometry: { | |
| type: "Point", | |
| coordinates: [ | |
| item.siteDetails.approximate_longitude, | |
| item.siteDetails.approximate_latitude | |
| ] | |
| }, | |
| properties: { | |
| id: item._id, | |
| site_id: item.site_id, | |
| time: new Date(item.time).toLocaleDateString("en-US", { | |
| weekday: "long", | |
| year: "numeric", | |
| month: "long", | |
| day: "numeric" | |
| }), | |
| location_name: item.siteDetails.name, | |
| aqi_category: item.aqi_category, | |
| aqi_color: item.aqi_color.startsWith("#") ? item.aqi_color : `#${item.aqi_color}`, | |
| value: item.pm2_5?.value ?? 0, | |
| } | |
| })) | |
| } | |
| } |
| const token = process.env.NEXT_PUBLIC_MAP_API_TOKEN | ||
| const AirQoToken = process.env.NEXT_PUBLIC_AIRQO_DATA_TOKEN |
There was a problem hiding this comment.
Security concern: API tokens exposed in client-side code.
Consider moving token handling to a server-side API endpoint to prevent exposure.
| <IconButton | ||
| onClick={() => setIsOpen(true)} | ||
| title='Map Layers' | ||
| icon={<LayerIcon width={24} height={24} fill={""}/>} | ||
| /> | ||
|
|
||
| <IconButton | ||
| onClick={() => setIsOpen(true)} | ||
| title='Refresh Map' | ||
| icon={<RefreshIcon width={24} height={24} fill={""} />} | ||
| /> | ||
|
|
||
| <IconButton | ||
| onClick={() => setIsOpen(true)} | ||
| title='Refresh Map' | ||
| icon={<ShareIcon width={24} height={24} fill={""} />} | ||
| /> |
There was a problem hiding this comment.
Fix duplicate onClick handlers and titles.
Multiple IconButton components share the same onClick handler and have incorrect titles.
<IconButton
onClick={() => setIsOpen(true)}
title='Map Layers'
icon={<LayerIcon width={24} height={24} fill={""}/>}
/>
<IconButton
- onClick={() => setIsOpen(true)}
+ onClick={refreshMap}
title='Refresh Map'
icon={<RefreshIcon width={24} height={24} fill={""} />}
/>
<IconButton
- onClick={() => setIsOpen(true)}
+ onClick={handleShare}
- title='Refresh Map'
+ title='Share Map'
icon={<ShareIcon width={24} height={24} fill={""} />}
/>📝 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.
| <IconButton | |
| onClick={() => setIsOpen(true)} | |
| title='Map Layers' | |
| icon={<LayerIcon width={24} height={24} fill={""}/>} | |
| /> | |
| <IconButton | |
| onClick={() => setIsOpen(true)} | |
| title='Refresh Map' | |
| icon={<RefreshIcon width={24} height={24} fill={""} />} | |
| /> | |
| <IconButton | |
| onClick={() => setIsOpen(true)} | |
| title='Refresh Map' | |
| icon={<ShareIcon width={24} height={24} fill={""} />} | |
| /> | |
| <IconButton | |
| onClick={() => setIsOpen(true)} | |
| title='Map Layers' | |
| icon={<LayerIcon width={24} height={24} fill={""}/>} | |
| /> | |
| <IconButton | |
| onClick={refreshMap} | |
| title='Refresh Map' | |
| icon={<RefreshIcon width={24} height={24} fill={""} />} | |
| /> | |
| <IconButton | |
| onClick={handleShare} | |
| title='Share Map' | |
| icon={<ShareIcon width={24} height={24} fill={""} />} | |
| /> |
| const feature = e.features ? e.features[0] : null; | ||
| if (feature && feature.properties) { | ||
|
|
||
| } |
There was a problem hiding this comment.
Fix incomplete error handling in mouseenter event.
The mouseenter event handler has an incomplete condition block.
const feature = e.features ? e.features[0] : null;
if (feature && feature.properties) {
-
+ // Handle feature properties
+ const { location_name, value, aqi_category } = feature.properties;
+ if (!location_name || !aqi_category) {
+ console.warn('Invalid feature properties');
+ return;
+ }
}📝 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 feature = e.features ? e.features[0] : null; | |
| if (feature && feature.properties) { | |
| } | |
| const feature = e.features ? e.features[0] : null; | |
| if (feature && feature.properties) { | |
| // Handle feature properties | |
| const { location_name, value, aqi_category } = feature.properties; | |
| if (!location_name || !aqi_category) { | |
| console.warn('Invalid feature properties'); | |
| return; | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 92-92: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| onMapDetailsSelect: PropTypes.func.isRequired, | ||
| onStyleSelect: PropTypes.func.isRequired, | ||
| mapStyles: PropTypes.arrayOf( | ||
| PropTypes.shape({ | ||
| name: PropTypes.string.isRequired, | ||
| image: PropTypes.oneOfType([PropTypes.string, PropTypes.object]).isRequired, // Allow both string and object | ||
| }) | ||
| ).isRequired, | ||
| mapDetails: PropTypes.array, | ||
| disabled: PropTypes.string, | ||
| onMapDetailsSelect: PropTypes.func.isRequired, | ||
| onStyleSelect: PropTypes.func.isRequired, | ||
| }; |
There was a problem hiding this comment.
Fix duplicate PropTypes declarations.
The PropTypes for onMapDetailsSelect and onStyleSelect are declared twice in the LayerModal.propTypes object.
Apply this diff to remove the duplicate declarations:
LayerModal.propTypes = {
isOpen: PropTypes.bool.isRequired,
onClose: PropTypes.func.isRequired,
onMapDetailsSelect: PropTypes.func.isRequired,
onStyleSelect: PropTypes.func.isRequired,
mapStyles: PropTypes.arrayOf(
PropTypes.shape({
name: PropTypes.string.isRequired,
image: PropTypes.oneOfType([PropTypes.string, PropTypes.object]).isRequired,
})
).isRequired,
mapDetails: PropTypes.array,
disabled: PropTypes.string,
- onMapDetailsSelect: PropTypes.func.isRequired,
- onStyleSelect: PropTypes.func.isRequired,
};📝 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.
| onMapDetailsSelect: PropTypes.func.isRequired, | |
| onStyleSelect: PropTypes.func.isRequired, | |
| mapStyles: PropTypes.arrayOf( | |
| PropTypes.shape({ | |
| name: PropTypes.string.isRequired, | |
| image: PropTypes.oneOfType([PropTypes.string, PropTypes.object]).isRequired, // Allow both string and object | |
| }) | |
| ).isRequired, | |
| mapDetails: PropTypes.array, | |
| disabled: PropTypes.string, | |
| onMapDetailsSelect: PropTypes.func.isRequired, | |
| onStyleSelect: PropTypes.func.isRequired, | |
| }; | |
| LayerModal.propTypes = { | |
| isOpen: PropTypes.bool.isRequired, | |
| onClose: PropTypes.func.isRequired, | |
| onMapDetailsSelect: PropTypes.func.isRequired, | |
| onStyleSelect: PropTypes.func.isRequired, | |
| mapStyles: PropTypes.arrayOf( | |
| PropTypes.shape({ | |
| name: PropTypes.string.isRequired, | |
| image: PropTypes.oneOfType([PropTypes.string, PropTypes.object]).isRequired, | |
| }) | |
| ).isRequired, | |
| mapDetails: PropTypes.array, | |
| disabled: PropTypes.string, | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 158-158: This property value named onMapDetailsSelect is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named onMapDetailsSelect
(lint/suspicious/noDuplicateObjectKeys)
[error] 159-159: This property value named onStyleSelect is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named onStyleSelect
(lint/suspicious/noDuplicateObjectKeys)
| const [selectedStyle, setSelectedStyle] = useState(mapStyles[0]); | ||
| const [selectedMapDetail, setSelectedMapDetail] = useState(mapDetails[0]); | ||
|
|
||
| useEffect(() => { | ||
| if (mapStyles[0]) { | ||
| setSelectedStyle(mapStyles[0]); | ||
| } | ||
| if (mapDetails[0]) { | ||
| setSelectedMapDetail(mapDetails[0]); | ||
| } | ||
| }, [mapStyles, mapDetails]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding defaultProps for mapStyles and mapDetails.
The current implementation assumes these arrays will always have at least one element. This could cause runtime errors if empty arrays are passed.
Add defaultProps and update the initialization logic:
+ LayerModal.defaultProps = {
+ mapStyles: [],
+ mapDetails: [],
+ };
const LayerModal = ({...}) => {
- const [selectedStyle, setSelectedStyle] = useState(mapStyles[0]);
- const [selectedMapDetail, setSelectedMapDetail] = useState(mapDetails[0]);
+ const [selectedStyle, setSelectedStyle] = useState(mapStyles[0] || null);
+ const [selectedMapDetail, setSelectedMapDetail] = useState(mapDetails[0] || null);
useEffect(() => {
- if (mapStyles[0]) {
+ if (mapStyles.length > 0) {
setSelectedStyle(mapStyles[0]);
}
- if (mapDetails[0]) {
+ if (mapDetails.length > 0) {
setSelectedMapDetail(mapDetails[0]);
}
}, [mapStyles, mapDetails]);📝 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 [selectedStyle, setSelectedStyle] = useState(mapStyles[0]); | |
| const [selectedMapDetail, setSelectedMapDetail] = useState(mapDetails[0]); | |
| useEffect(() => { | |
| if (mapStyles[0]) { | |
| setSelectedStyle(mapStyles[0]); | |
| } | |
| if (mapDetails[0]) { | |
| setSelectedMapDetail(mapDetails[0]); | |
| } | |
| }, [mapStyles, mapDetails]); | |
| LayerModal.defaultProps = { | |
| mapStyles: [], | |
| mapDetails: [], | |
| }; | |
| const LayerModal = ({ mapStyles, mapDetails, ...props }) => { | |
| const [selectedStyle, setSelectedStyle] = useState(mapStyles[0] || null); | |
| const [selectedMapDetail, setSelectedMapDetail] = useState(mapDetails[0] || null); | |
| useEffect(() => { | |
| if (mapStyles.length > 0) { | |
| setSelectedStyle(mapStyles[0]); | |
| } | |
| if (mapDetails.length > 0) { | |
| setSelectedMapDetail(mapDetails[0]); | |
| } | |
| }, [mapStyles, mapDetails]); | |
| // ... rest of the component logic | |
| }; |
| export const GetAirQuoData = async (token: string) =>{ | ||
| try { | ||
| if(token){ | ||
| const response = await axios.get(`https://staging-analytics.airqo.net/api/v2/devices/readings/map?token=${token}`); | ||
| const data = response.data; | ||
| return data; | ||
| } | ||
| { | ||
| console.log("Error Token Is Missing") | ||
| return null; | ||
| } | ||
|
|
||
| } | ||
| catch (error) { | ||
| console.error('Error fetching GeoJSON:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and code structure in GetAirQuoData.
The function has several issues:
- Unnecessary block statement
- Inconsistent error handling
- Hard-coded URL
Apply this diff to improve the implementation:
+ const API_BASE_URL = 'https://staging-analytics.airqo.net/api/v2';
+
export const GetAirQuoData = async (token: string) => {
try {
- if(token){
- const response = await axios.get(`https://staging-analytics.airqo.net/api/v2/devices/readings/map?token=${token}`);
+ if (!token) {
+ throw new Error('Token is missing');
+ }
+
+ const response = await axios.get(`${API_BASE_URL}/devices/readings/map`, {
+ params: { token }
+ });
const data = response.data;
return data;
- }
- {
- console.log("Error Token Is Missing")
- return null;
- }
} catch (error) {
- console.error('Error fetching GeoJSON:', error);
+ console.error('Error in GetAirQuoData:', 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.
| export const GetAirQuoData = async (token: string) =>{ | |
| try { | |
| if(token){ | |
| const response = await axios.get(`https://staging-analytics.airqo.net/api/v2/devices/readings/map?token=${token}`); | |
| const data = response.data; | |
| return data; | |
| } | |
| { | |
| console.log("Error Token Is Missing") | |
| return null; | |
| } | |
| } | |
| catch (error) { | |
| console.error('Error fetching GeoJSON:', error); | |
| } | |
| } | |
| const API_BASE_URL = 'https://staging-analytics.airqo.net/api/v2'; | |
| export const GetAirQuoData = async (token: string) => { | |
| try { | |
| if (!token) { | |
| throw new Error('Token is missing'); | |
| } | |
| const response = await axios.get(`${API_BASE_URL}/devices/readings/map`, { | |
| params: { token } | |
| }); | |
| const data = response.data; | |
| return data; | |
| } catch (error) { | |
| console.error('Error in GetAirQuoData:', error); | |
| throw error; | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-13: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
| export const UserClick = async(access_token:string,sessionToken:string,locationid: string,)=>{ | ||
| if (!access_token) { | ||
| console.log("Missing Access Token"); | ||
| return; | ||
| } | ||
|
|
||
| const response = await axios.get((`https://api.mapbox.com/search/searchbox/v1/retrieve/${locationid}?&access_token=${access_token}&session_token=${sessionToken}`)) | ||
| const data = response.data; | ||
| if(data){ | ||
| return data; | ||
| } | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and type safety in UserClick.
The function has several issues:
- No try-catch block
- Inconsistent error handling
- Hard-coded URL
Apply this diff to improve the implementation:
- export const UserClick = async(access_token:string,sessionToken:string,locationid: string,)=>{
+ export const UserClick = async (
+ access_token: string,
+ sessionToken: string,
+ locationid: string,
+ ): Promise<any> => {
if (!access_token) {
- console.log("Missing Access Token");
- return;
+ throw new Error('Missing Access Token');
}
+ try {
+ const response = await axios.get(`${MAPBOX_API_URL}/retrieve/${locationid}`, {
+ params: {
+ access_token,
+ session_token: sessionToken
+ }
+ });
- const response = await axios.get((`https://api.mapbox.com/search/searchbox/v1/retrieve/${locationid}?&access_token=${access_token}&session_token=${sessionToken}`))
const data = response.data;
- if(data){
- return data;
- }
+ return data;
+ } catch (error) {
+ console.error('Error in UserClick:', 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.
| export const UserClick = async(access_token:string,sessionToken:string,locationid: string,)=>{ | |
| if (!access_token) { | |
| console.log("Missing Access Token"); | |
| return; | |
| } | |
| const response = await axios.get((`https://api.mapbox.com/search/searchbox/v1/retrieve/${locationid}?&access_token=${access_token}&session_token=${sessionToken}`)) | |
| const data = response.data; | |
| if(data){ | |
| return data; | |
| } | |
| } | |
| export const UserClick = async ( | |
| access_token: string, | |
| sessionToken: string, | |
| locationid: string, | |
| ): Promise<any> => { | |
| if (!access_token) { | |
| throw new Error('Missing Access Token'); | |
| } | |
| try { | |
| const response = await axios.get(`${MAPBOX_API_URL}/retrieve/${locationid}`, { | |
| params: { | |
| access_token, | |
| session_token: sessionToken, | |
| }, | |
| }); | |
| const data = response.data; | |
| return data; | |
| } catch (error) { | |
| console.error('Error in UserClick:', error); | |
| throw error; | |
| } | |
| } |
| export const FetchSuggestions=async(value:string,access_token:string,sessionToken:string,latitude?: number, longitude?: number):Promise<any[] | void>=>{ | ||
|
|
||
| if (!access_token) { | ||
| console.log("Missing Access Token"); | ||
| return; | ||
| } | ||
|
|
||
| if (latitude !== undefined && longitude !== undefined) { | ||
| try{ | ||
| const proximityParam = `${longitude},${latitude}`; | ||
| const response = await axios.get(`https://api.mapbox.com/search/searchbox/v1/suggest?q=${value.toLowerCase()}&access_token=${access_token}&proximity=${proximityParam}&session_token=${sessionToken}`) | ||
| const data = await response.data; | ||
|
|
||
| console.log("data :",data.suggestions) | ||
| if (data.suggestions) { | ||
| return data.suggestions; | ||
| }else{ | ||
| console.log("Error fetching suggestions"); | ||
| } | ||
|
|
||
| } | ||
| catch(error){ | ||
| console.error("Error fetching suggestions:", error); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and type safety in FetchSuggestions.
The function has several issues:
- Return type includes
void - Inconsistent error handling
- Hard-coded URL
- Unnecessary console.log statements
Apply this diff to improve the implementation:
+ const MAPBOX_API_URL = 'https://api.mapbox.com/search/searchbox/v1';
+
+ interface MapboxSuggestion {
+ // Add appropriate type definitions based on the API response
+ suggestions: any[];
+ }
+
- export const FetchSuggestions = async (value: string, access_token: string, sessionToken: string, latitude?: number, longitude?: number): Promise<any[] | void> => {
+ export const FetchSuggestions = async (value: string, access_token: string, sessionToken: string, latitude?: number, longitude?: number): Promise<any[]> => {
if (!access_token) {
- console.log("Missing Access Token");
- return;
+ throw new Error('Missing Access Token');
}
if (latitude !== undefined && longitude !== undefined) {
try {
const proximityParam = `${longitude},${latitude}`;
- const response = await axios.get(`https://api.mapbox.com/search/searchbox/v1/suggest?q=${value.toLowerCase()}&access_token=${access_token}&proximity=${proximityParam}&session_token=${sessionToken}`)
+ const response = await axios.get<MapboxSuggestion>(`${MAPBOX_API_URL}/suggest`, {
+ params: {
+ q: value.toLowerCase(),
+ access_token,
+ proximity: proximityParam,
+ session_token: sessionToken
+ }
+ });
const data = await response.data;
- console.log("data :", data.suggestions)
- if (data.suggestions) {
- return data.suggestions;
- } else {
- console.log("Error fetching suggestions");
- }
+ return data.suggestions || [];
} catch (error) {
- console.error("Error fetching suggestions:", error);
+ console.error('Error in FetchSuggestions:', error);
+ throw error;
}
}
+ return [];
}📝 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 const FetchSuggestions=async(value:string,access_token:string,sessionToken:string,latitude?: number, longitude?: number):Promise<any[] | void>=>{ | |
| if (!access_token) { | |
| console.log("Missing Access Token"); | |
| return; | |
| } | |
| if (latitude !== undefined && longitude !== undefined) { | |
| try{ | |
| const proximityParam = `${longitude},${latitude}`; | |
| const response = await axios.get(`https://api.mapbox.com/search/searchbox/v1/suggest?q=${value.toLowerCase()}&access_token=${access_token}&proximity=${proximityParam}&session_token=${sessionToken}`) | |
| const data = await response.data; | |
| console.log("data :",data.suggestions) | |
| if (data.suggestions) { | |
| return data.suggestions; | |
| }else{ | |
| console.log("Error fetching suggestions"); | |
| } | |
| } | |
| catch(error){ | |
| console.error("Error fetching suggestions:", error); | |
| } | |
| }} | |
| const MAPBOX_API_URL = 'https://api.mapbox.com/search/searchbox/v1'; | |
| interface MapboxSuggestion { | |
| // Add appropriate type definitions based on the API response | |
| suggestions: any[]; | |
| } | |
| export const FetchSuggestions = async ( | |
| value: string, | |
| access_token: string, | |
| sessionToken: string, | |
| latitude?: number, | |
| longitude?: number | |
| ): Promise<any[]> => { | |
| if (!access_token) { | |
| throw new Error('Missing Access Token'); | |
| } | |
| if (latitude !== undefined && longitude !== undefined) { | |
| try { | |
| const proximityParam = `${longitude},${latitude}`; | |
| const response = await axios.get<MapboxSuggestion>(`${MAPBOX_API_URL}/suggest`, { | |
| params: { | |
| q: value.toLowerCase(), | |
| access_token, | |
| proximity: proximityParam, | |
| session_token: sessionToken | |
| } | |
| }); | |
| const data = await response.data; | |
| return data.suggestions || []; | |
| } catch (error) { | |
| console.error('Error in FetchSuggestions:', error); | |
| throw error; | |
| } | |
| } | |
| return []; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/netmanager-app/core/apis/MapData.ts (4)
1-6:⚠️ Potential issueMove sensitive tokens to server-side endpoints.
Exposing API tokens in client-side code poses a security risk. Consider moving token handling to server-side API endpoints.
25-49: 🛠️ Refactor suggestionImprove error handling and type safety in FetchSuggestions.
The function needs improvements in error handling, type safety, and URL construction.
Apply this diff:
- export const FetchSuggestions = async (value: string, access_token: string, sessionToken: string, latitude?: number, longitude?: number): Promise<any[] | void> => { + export const FetchSuggestions = async (value: string, access_token: string, sessionToken: string, latitude?: number, longitude?: number): Promise<any[]> => { if (!access_token) { - console.log("Missing Access Token"); - return; + throw new Error('Missing Access Token'); } if (latitude !== undefined && longitude !== undefined) { try { const proximityParam = `${longitude},${latitude}`; - const response = await axios.get(`${SUGGESTIONS}?q=${value.toLowerCase()}&access_token=${access_token}&proximity=${proximityParam}&session_token=${sessionToken}`) + const response = await axios.get(SUGGESTIONS, { + params: { + q: value.toLowerCase(), + access_token, + proximity: proximityParam, + session_token: sessionToken + } + }); const data = await response.data; - console.log("data :", data.suggestions) - if (data.suggestions) { - return data.suggestions; - } else { - console.log("Error fetching suggestions"); - } + return data.suggestions || []; } catch (error) { - console.error("Error fetching suggestions:", error); + console.error('Error in FetchSuggestions:', error); + throw error; } } + return []; }🧰 Tools
🪛 Biome (1.9.4)
[error] 25-25: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
50-63: 🛠️ Refactor suggestionAdd error handling and improve URL construction in UserClick.
The function lacks proper error handling and uses direct URL parameter concatenation.
Apply this diff:
- export const UserClick = async(access_token:string,sessionToken:string,locationid: string,)=>{ + export const UserClick = async ( + access_token: string, + sessionToken: string, + locationid: string, + ): Promise<any> => { if (!access_token) { - console.log("Missing Access Token"); - return; + throw new Error('Missing Access Token'); } + try { + const response = await axios.get(SEARCH + '/' + locationid, { + params: { + access_token, + session_token: sessionToken + } + }); - const response = await axios.get((`${SEARCH}/${locationid}?&access_token=${access_token}&session_token=${sessionToken}`)) - const data = response.data; - if(data){ - return data; - } + return response.data; + } catch (error) { + console.error('Error in UserClick:', error); + throw error; + } }
7-23: 🛠️ Refactor suggestionImprove error handling and code structure.
The function has several issues that need to be addressed:
- Unnecessary block statement
- Inconsistent error handling
- URL parameters are concatenated directly
Apply this diff to improve the implementation:
export const GetAirQuoData = async (token: string) => { try { - if(token){ - const response = await axios.get(`${AirQuoData}?token=${token}`); - const data = response.data; - return data; - } - { - console.log("Error Token Is Missing") - return null; - } + if (!token) { + throw new Error('Token is missing'); + } + + const response = await axios.get(AirQuoData, { + params: { token } + }); + return response.data; } catch (error) { - console.error('Error fetching GeoJSON:', error); + console.error('Error in GetAirQuoData:', error); + throw error; } }🧰 Tools
🪛 Biome (1.9.4)
[error] 14-17: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
src/netmanager-app/components/NetManagerMap/page.tsx (1)
310-326:⚠️ Potential issueFix duplicate onClick handlers and titles.
Multiple IconButton components share the same onClick handler and have incorrect titles.
Apply this diff:
<IconButton onClick={() => setIsOpen(true)} title='Map Layers' icon={<LayerIcon width={24} height={24} fill={""}/>} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={refreshMap} title='Refresh Map' icon={<RefreshIcon width={24} height={24} fill={""} />} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={handleShare} - title='Refresh Map' + title='Share Map' icon={<ShareIcon width={24} height={24} fill={""} />} />
🧹 Nitpick comments (2)
src/netmanager-app/components/NetManagerMap/page.tsx (2)
180-229: Improve error handling and state management in search functionality.The search functionality needs improvements in error handling and map state management.
- Extract map navigation to a separate function
- Add proper error handling for geolocation
- Add loading state for suggestions
Apply this diff:
+ const navigateMap = (longitude: number, latitude: number, zoom: number) => { + if (mapRef.current) { + mapRef.current.flyTo({ + center: [longitude, latitude], + zoom, + essential: true + }); + } + }; const SearchSuggestions = (e: React.ChangeEvent<HTMLInputElement>) => { const value = e.target.value; setQuery(value); if (value.trim() === "") { setSuggestions([]); - if (mapRef.current) { - mapRef.current.flyTo({ - center: [18.5,5], - zoom: 3.0, - essential: true - }); - } + navigateMap(18.5, 5, 3.0); return; } const GetSuggestions = (latitude?: number, longitude?: number) => { + setSuggestions([]); // Clear previous suggestions FetchSuggestions(value, token ?? "", sessionToken ?? "", latitude, longitude) .then(data => { - if (data) { - console.log(data) - setSuggestions(data); - } - console.log("Number of Suggesstions", suggestions.length) + setSuggestions(data ?? []); }) .catch(error => { console.error("Error fetching suggestions:", error); + setSuggestions([]); }); };
96-109: Extract popup styles and content to separate components.The popup implementation uses inline styles and direct HTML injection, which makes it harder to maintain.
Consider creating a separate component for the popup content and moving styles to a CSS module:
// PopupContent.tsx interface PopupContentProps { time: string; locationName: string; value: number; aqiCategory: string; aqiColor: string; } const PopupContent: React.FC<PopupContentProps> = ({ time, locationName, value, aqiCategory, aqiColor }) => ( <div className="popup-container"> <div className="popup-time">{time}</div> <div className="popup-header"> <strong>{locationName}</strong> <div className="popup-value">{value.toFixed(2)}µg/m³</div> </div> <div className="popup-aqi"> <h1 style={{ color: aqiColor }}>Air Quality is {aqiCategory}</h1> <div className="popup-icon" style={{ backgroundColor: aqiColor }} /> </div> </div> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)src/netmanager-app/core/apis/MapData.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 90-90: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/netmanager-app/core/apis/MapData.ts
[error] 25-25: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 14-17: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
| useEffect(() => { | ||
| mapboxgl.accessToken = token; | ||
| if (mapContainerRef.current) { | ||
| mapRef.current = new mapboxgl.Map({ | ||
| container: mapContainerRef.current, | ||
| style:'mapbox://styles/mapbox/streets-v11', | ||
| center: [18.5, 3], | ||
| zoom: 3 | ||
| }); | ||
|
|
||
| mapRef.current.on('load', async () => { | ||
| if (mapRef.current) { | ||
|
|
||
| try { | ||
| const Data = await GetAirQuoData(AirQoToken?AirQoToken:"") | ||
| console.log(Data) | ||
| const geojsonData = ConvertToGeojson(Data); | ||
| console.log('GeoJson Data : ',geojsonData) | ||
|
|
||
| mapRef.current.addSource('data', { | ||
| type: 'geojson', | ||
| data: geojsonData | ||
| }); | ||
| const popup = new mapboxgl.Popup({ | ||
| closeButton: false, | ||
| closeOnClick: false, | ||
|
|
||
| }); | ||
| // Listen for when the mouse enters a feature in your 'data-layer' | ||
| mapRef.current.on('mouseenter', 'data-layer', (e) => { | ||
| if (mapRef.current) { | ||
| mapRef.current.getCanvas().style.cursor = 'pointer'; | ||
| } | ||
| // Get the feature under the mouse pointer (the first feature). | ||
| const feature = e.features ? e.features[0] : null; | ||
| if (feature && feature.properties) { | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling in map initialization and data loading.
The code has several issues that need attention:
- Map token is exposed in client-side code
- Error handling in data loading could be improved
- Incomplete mouseenter event handler
Apply this diff:
useEffect(() => {
- mapboxgl.accessToken = token;
+ if (!token) {
+ console.error('Map token is missing');
+ return;
+ }
+ mapboxgl.accessToken = token;
if (mapContainerRef.current) {
mapRef.current = new mapboxgl.Map({
container: mapContainerRef.current,
style:'mapbox://styles/mapbox/streets-v11',
center: [18.5, 3],
zoom: 3
});
mapRef.current.on('load', async () => {
if (mapRef.current) {
try {
- const Data = await GetAirQuoData(AirQoToken?AirQoToken:"")
- console.log(Data)
+ if (!AirQoToken) {
+ throw new Error('AirQo token is missing');
+ }
+ const Data = await GetAirQuoData(AirQoToken);
const geojsonData = ConvertToGeojson(Data);
- console.log('GeoJson Data : ',geojsonData)
mapRef.current.addSource('data', {
type: 'geojson',
data: geojsonData
});
// ... rest of the code
} catch (error) {
- console.error('Error fetching GeoJSON:', error);
+ console.error('Error initializing map:', error);
+ // Handle error appropriately
}
}
});
}
}, []);📝 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(() => { | |
| mapboxgl.accessToken = token; | |
| if (mapContainerRef.current) { | |
| mapRef.current = new mapboxgl.Map({ | |
| container: mapContainerRef.current, | |
| style:'mapbox://styles/mapbox/streets-v11', | |
| center: [18.5, 3], | |
| zoom: 3 | |
| }); | |
| mapRef.current.on('load', async () => { | |
| if (mapRef.current) { | |
| try { | |
| const Data = await GetAirQuoData(AirQoToken?AirQoToken:"") | |
| console.log(Data) | |
| const geojsonData = ConvertToGeojson(Data); | |
| console.log('GeoJson Data : ',geojsonData) | |
| mapRef.current.addSource('data', { | |
| type: 'geojson', | |
| data: geojsonData | |
| }); | |
| const popup = new mapboxgl.Popup({ | |
| closeButton: false, | |
| closeOnClick: false, | |
| }); | |
| // Listen for when the mouse enters a feature in your 'data-layer' | |
| mapRef.current.on('mouseenter', 'data-layer', (e) => { | |
| if (mapRef.current) { | |
| mapRef.current.getCanvas().style.cursor = 'pointer'; | |
| } | |
| // Get the feature under the mouse pointer (the first feature). | |
| const feature = e.features ? e.features[0] : null; | |
| if (feature && feature.properties) { | |
| } | |
| useEffect(() => { | |
| + if (!token) { | |
| + console.error('Map token is missing'); | |
| + return; | |
| + } | |
| mapboxgl.accessToken = token; | |
| if (mapContainerRef.current) { | |
| mapRef.current = new mapboxgl.Map({ | |
| container: mapContainerRef.current, | |
| style:'mapbox://styles/mapbox/streets-v11', | |
| center: [18.5, 3], | |
| zoom: 3 | |
| }); | |
| mapRef.current.on('load', async () => { | |
| if (mapRef.current) { | |
| try { | |
| + if (!AirQoToken) { | |
| + throw new Error('AirQo token is missing'); | |
| + } | |
| + const Data = await GetAirQuoData(AirQoToken); | |
| + const geojsonData = ConvertToGeojson(Data); | |
| - | |
| - const Data = await GetAirQuoData(AirQoToken?AirQoToken:"") | |
| - console.log(Data) | |
| - const geojsonData = ConvertToGeojson(Data); | |
| - console.log('GeoJson Data : ',geojsonData) | |
| mapRef.current.addSource('data', { | |
| type: 'geojson', | |
| data: geojsonData | |
| }); | |
| const popup = new mapboxgl.Popup({ | |
| closeButton: false, | |
| closeOnClick: false, | |
| }); | |
| // Listen for when the mouse enters a feature in your 'data-layer' | |
| mapRef.current.on('mouseenter', 'data-layer', (e) => { | |
| if (mapRef.current) { | |
| mapRef.current.getCanvas().style.cursor = 'pointer'; | |
| } | |
| // Get the feature under the mouse pointer (the first feature). | |
| const feature = e.features ? e.features[0] : null; | |
| if (feature && feature.properties) { | |
| } | |
| }); | |
| } catch (error) { | |
| - console.error('Error fetching GeoJSON:', error); | |
| + console.error('Error initializing map:', error); | |
| + // Handle error appropriately | |
| } | |
| } | |
| }); | |
| } | |
| }, []); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 90-90: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/netmanager-app/components/NetManagerMap/page.tsx (1)
95-98:⚠️ Potential issueFix incomplete error handling in mouseenter event.
The mouseenter event handler has an empty condition block.
const feature = e.features?.[0]; -if (feature && feature.properties) { - -} +if (!feature?.properties) { + console.warn('Invalid feature properties'); + return; +}🧰 Tools
🪛 Biome (1.9.4)
[error] 96-96: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🧹 Nitpick comments (1)
src/netmanager-app/components/NetManagerMap/page.tsx (1)
206-210: Remove unnecessary console.log statements.These debug logs should not be in production code.
- console.log(data) setSuggestions(data); } - console.log("Number of Suggesstions", suggestions.length)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 96-96: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
src/netmanager-app/components/NetManagerMap/page.tsx (3)
7-7: Security concern: API tokens exposed in client-side code.The tokens are imported directly from MapData.ts and exposed in the client-side code.
316-332: Fix duplicate onClick handlers and titles.Multiple IconButton components share the same onClick handler and have incorrect titles.
337-344: LGTM: LayerModel implementation.The LayerModel component is well-structured with proper prop passing and event handling.
| } | ||
| const GetSuggestions=(latitude?: number, longitude?: number)=>{ | ||
|
|
||
| FetchSuggestions(value,token?token:"",sessionToken?sessionToken:"",latitude,longitude) |
There was a problem hiding this comment.
Improve error handling for token validation.
The code uses nullish coalescing with empty strings, which could lead to invalid API calls.
-FetchSuggestions(value,token?token:"",sessionToken?sessionToken:"",latitude,longitude)
+if (!token || !sessionToken) {
+ console.error('Missing required tokens');
+ return;
+}
+FetchSuggestions(value, token, sessionToken, latitude, longitude)📝 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.
| FetchSuggestions(value,token?token:"",sessionToken?sessionToken:"",latitude,longitude) | |
| if (!token || !sessionToken) { | |
| console.error('Missing required tokens'); | |
| return; | |
| } | |
| FetchSuggestions(value, token, sessionToken, latitude, longitude) |
| const mapRef = useRef<mapboxgl.Map | null>(null); | ||
| const [query, setQuery] = useState(""); | ||
| const [locationId,setlocationId] =useState("") | ||
| const [suggestions, setSuggestions] = useState<any[]>([]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type any[] should be avoided for suggestions.
Define a proper interface for the suggestion items to improve type safety.
-const [suggestions, setSuggestions] = useState<any[]>([]);
+interface LocationSuggestion {
+ mapbox_id: string;
+ name: string;
+ place_formatted: string;
+}
+const [suggestions, setSuggestions] = useState<LocationSuggestion[]>([]);📝 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 [suggestions, setSuggestions] = useState<any[]>([]); | |
| interface LocationSuggestion { | |
| mapbox_id: string; | |
| name: string; | |
| place_formatted: string; | |
| } | |
| const [suggestions, setSuggestions] = useState<LocationSuggestion[]>([]); |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
src/netmanager-app/components/NetManagerMap/page.tsx (3)
97-100:⚠️ Potential issueFix incomplete mouseenter event handler.
The mouseenter event handler has an empty condition block which doesn't do anything.
const feature = e.features ? e.features[0] : null; if (feature && feature.properties) { - + // Handle feature properties + const { location_name, value, aqi_category } = feature.properties; + if (!location_name || !aqi_category) { + console.warn('Invalid feature properties'); + return; + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 98-98: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
323-339:⚠️ Potential issueFix duplicate onClick handlers and titles.
Multiple IconButton components share the same onClick handler and have incorrect titles.
<IconButton onClick={() => setIsOpen(true)} title='Map Layers' icon={<LayerIcon width={24} height={24} fill={""}/>} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={refreshMap} title='Refresh Map' icon={<RefreshIcon width={24} height={24} fill={""} />} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={handleShare} - title='Refresh Map' + title='Share Map' icon={<ShareIcon width={24} height={24} fill={""} />} />
243-243:⚠️ Potential issueFix token validation.
Don't use empty strings as fallbacks for missing tokens. Check if tokens exist before making the API call.
-UserClick(token?token:"",sessionToken?sessionToken:"",locationid) +if (!token || !sessionToken) { + console.error('Missing required tokens'); + return; +} +UserClick(token, sessionToken, locationid)src/netmanager-app/core/apis/MapData.ts (3)
16-32: 🛠️ Refactor suggestionImprove error handling and code structure in GetAirQuoData.
The function has several issues:
- Unnecessary block statement
- Inconsistent error handling
- Hard-coded URL
+const API_BASE_URL = process.env.NEXT_PUBLIC_AIRQUO_DATA; + export const GetAirQuoData = async (token: string) => { try { - if(token){ - const response = await axios.get(`${AirQuoData}?token=${token}`); - const data = response.data; - return data; - } - { - console.log("Error Token Is Missing") - return null; - } + if (!token) { + throw new Error('Token is missing'); + } + + const response = await axios.get(`${AirQuoData}`, { + params: { token } + }); + const data = response.data; + return data; } catch (error) { - console.error('Error fetching GeoJSON:', error); + console.error('Error in GetAirQuoData:', error); + throw error; } }🧰 Tools
🪛 Biome (1.9.4)
[error] 23-26: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
34-58: 🛠️ Refactor suggestionImprove error handling and type safety in FetchSuggestions.
The function has several issues:
- Return type includes
void- Inconsistent error handling
- Unnecessary console.log statements
+interface MapboxSuggestion { + suggestions: any[]; +} + -export const FetchSuggestions=async(value:string,access_token:string,sessionToken:string,latitude?: number, longitude?: number):Promise<any[] | void>=>{ +export const FetchSuggestions = async ( + value: string, + access_token: string, + sessionToken: string, + latitude?: number, + longitude?: number +): Promise<any[]> => { if (!access_token) { - console.log("Missing Access Token"); - return; + throw new Error('Missing Access Token'); } if (latitude !== undefined && longitude !== undefined) { try { const proximityParam = `${longitude},${latitude}`; - const response = await axios.get(`${SUGGESTIONS}?q=${value.toLowerCase()}&access_token=${access_token}&proximity=${proximityParam}&session_token=${sessionToken}`) - const data = await response.data; - - console.log("data :",data.suggestions) - if (data.suggestions) { - return data.suggestions; - }else{ - console.log("Error fetching suggestions"); - } + const response = await axios.get<MapboxSuggestion>(`${SUGGESTIONS}`, { + params: { + q: value.toLowerCase(), + access_token, + proximity: proximityParam, + session_token: sessionToken + } + }); + const data = response.data; + return data.suggestions || []; } catch(error) { - console.error("Error fetching suggestions:", error); + console.error('Error in FetchSuggestions:', error); + throw error; } - }} + } + return []; +}🧰 Tools
🪛 Biome (1.9.4)
[error] 34-34: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
59-72: 🛠️ Refactor suggestionImprove error handling and type safety in UserClick.
The function has several issues:
- No try-catch block
- Inconsistent error handling
- Ambiguous return type
-export const UserClick = async(access_token:string,sessionToken:string,locationid: string,)=>{ +export const UserClick = async ( + access_token: string, + sessionToken: string, + locationid: string +): Promise<any> => { if (!access_token) { - console.log("Missing Access Token"); - return; + throw new Error('Missing Access Token'); } + try { + const response = await axios.get(`${SEARCH}/${locationid}`, { + params: { + access_token, + session_token: sessionToken + } + }); - const response = await axios.get((`${SEARCH}/${locationid}?&access_token=${access_token}&session_token=${sessionToken}`)) const data = response.data; - if(data){ - return data; - } + return data; + } catch (error) { + console.error('Error in UserClick:', error); + throw error; + } }
🧹 Nitpick comments (4)
src/netmanager-app/components/NetManagerMap/page.tsx (4)
79-80: Remove console.log statement.Console log statements should be removed before deploying to production.
const geojsonData = ConvertToGeojson(Data); -console.log('GeoJson Data : ',geojsonData)
211-215: Remove console.log statements.Remove debugging log statements from production code.
if (data) { - console.log(data) setSuggestions(data); } -console.log("Number of Suggesstions", suggestions.length)
225-226: Remove console.log statement.Remove debugging log statement from production code.
const { latitude, longitude } = position.coords; -console.log("User Location:", latitude, longitude); GetSuggestions(latitude, longitude);
245-246: Add error handling for missing coordinates.Check if the coordinates exist before trying to access them.
-console.log(data.features[0].geometry.coordinates) -if (mapRef.current) { +if (mapRef.current && data.features && data.features[0]?.geometry?.coordinates) { + const coordinates = data.features[0].geometry.coordinates; mapRef.current.flyTo({ - center: data.features[0].geometry.coordinates, + center: coordinates, zoom: 12, essential: true }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)src/netmanager-app/core/apis/MapData.ts(1 hunks)src/netmanager-app/tsconfig.json(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/core/apis/MapData.ts
[error] 34-34: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 23-26: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 98-98: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
src/netmanager-app/tsconfig.json (1)
24-24: Include entry added for JavaScript component.The file
components/NetManagerMap/components/LayerModal.jshas been added to the TypeScript compiler's include array, which allows the JavaScript file to be properly processed within the TypeScript project.src/netmanager-app/components/NetManagerMap/page.tsx (3)
22-22: Typeany[]should be avoided for suggestions.Define a proper interface for the suggestion items to improve type safety.
-const [suggestions, setSuggestions] = useState<any[]>([]); +interface LocationSuggestion { + mapbox_id: string; + name: string; + place_formatted: string; +} +const [suggestions, setSuggestions] = useState<LocationSuggestion[]>([]);
40-47: Implement refreshMap functionality.The refreshMap functionality is commented out. Either implement it or remove the commented code.
-// const refreshMap = useRefreshMap( -// setToastMessage, -// mapRef, -// dispatch, -// selectedNode, -// ); +const refreshMap = () => { + if (mapRef.current && AirQoToken) { + GetAirQuoData(AirQoToken) + .then(data => { + if (data) { + const geojsonData = ConvertToGeojson(data); + if (mapRef.current.getSource('data')) { + mapRef.current.getSource('data').setData(geojsonData); + } + } + }) + .catch(error => console.error("Error refreshing map:", error)); + } +};
334-340: Add handleShare implementation.The Share button is missing its handler implementation.
+const handleShare = () => { + // Implementation for sharing the map view + // Example: Copy current map URL to clipboard + if (typeof window !== "undefined") { + navigator.clipboard.writeText(window.location.href) + .then(() => alert("Map URL copied to clipboard!")) + .catch(err => console.error("Error copying URL:", err)); + } +}; <IconButton -onClick={() => setIsOpen(true)} +onClick={handleShare} -title='Refresh Map' +title='Share Map' icon={<ShareIcon width={24} height={24} fill={""} />} />src/netmanager-app/core/apis/MapData.ts (1)
1-6: Security concern: API tokens exposed in client-side code.Consider moving token handling to a server-side API endpoint to prevent exposure.
The current implementation exposes API tokens in client-side code which could potentially lead to misuse. Consider implementing a server-side proxy to handle API requests securely.
#!/bin/bash # Verify how many places the API tokens are exposed rg --max-depth 3 "process.env.NEXT_PUBLIC_(MAP_API_TOKEN|AIRQO_DATA_TOKEN)"
| { loading ||!airdata &&(<div className="absolute inset-0 flex items-center justify-center z-[10000]"> | ||
| <div | ||
| className={`bg-white w-[64px] h-[64px] flex justify-center items-center rounded-md shadow-md p-3`} | ||
| > | ||
| <LoaderCircle className='animate-spin text-blue-600' width={32} height={32} /> | ||
| </div> | ||
| </div>)} |
There was a problem hiding this comment.
Fix conditional rendering syntax.
The conditional rendering syntax has an issue. The && operator is being used incorrectly with the || operator.
-{ loading ||!airdata &&(<div className="absolute inset-0 flex items-center justify-center z-[10000]">
+{ (loading || !airdata) && (<div className="absolute inset-0 flex items-center justify-center z-[10000]">
<div
className={`bg-white w-[64px] h-[64px] flex justify-center items-center rounded-md shadow-md p-3`}
>
<LoaderCircle className='animate-spin text-blue-600' width={32} height={32} />
</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.
| { loading ||!airdata &&(<div className="absolute inset-0 flex items-center justify-center z-[10000]"> | |
| <div | |
| className={`bg-white w-[64px] h-[64px] flex justify-center items-center rounded-md shadow-md p-3`} | |
| > | |
| <LoaderCircle className='animate-spin text-blue-600' width={32} height={32} /> | |
| </div> | |
| </div>)} | |
| { (loading || !airdata) && (<div className="absolute inset-0 flex items-center justify-center z-[10000]"> | |
| <div | |
| className={`bg-white w-[64px] h-[64px] flex justify-center items-center rounded-md shadow-md p-3`} | |
| > | |
| <LoaderCircle className='animate-spin text-blue-600' width={32} height={32} /> | |
| </div> | |
| </div>)} |
…frontend into NetManagerMapFunc
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
src/netmanager-app/components/NetManagerMap/page.tsx (5)
28-28: 🛠️ Refactor suggestionDefine a proper interface for the suggestion items.
The
any[]type for suggestions is too generic and lacks type safety. This makes the code less maintainable and prone to runtime errors.Create an interface for the location suggestions:
-const [suggestions, setSuggestions] = useState<any[]>([]); +interface LocationSuggestion { + mapbox_id: string; + name: string; + place_formatted: string; + // Add other properties as needed +} +const [suggestions, setSuggestions] = useState<LocationSuggestion[]>([]);
117-120:⚠️ Potential issueComplete the empty conditional block in mouseenter event handler.
The condition checks for feature properties but doesn't do anything with them, resulting in an incomplete implementation.
const feature = e.features ? e.features[0] : null; if (feature && feature.properties) { - + // Extract and validate necessary properties + const { location_name, aqi_category } = feature.properties; + if (!location_name || !aqi_category) { + console.warn('Invalid feature properties'); + return; + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 118-118: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
285-287:⚠️ Potential issueImprove token validation in handleLocationClick function.
Using a nullish coalescing operator with empty strings can lead to invalid API calls. Proper validation should be performed.
const handleLocationClick = (locationid: any) => { - UserClick(token?token:"",sessionToken?sessionToken:"",locationid) + if (!token || !sessionToken) { + console.error('Missing required tokens'); + return; + } + UserClick(token, sessionToken, locationid) .then(data => {
371-377:⚠️ Potential issueFix conditional rendering syntax.
The conditional rendering syntax has a precedence issue. The logical operators should be properly grouped with parentheses.
-{ (loading || !airdata) &&(<div className="absolute inset-0 flex items-center justify-center z-[10000]"> +{ (loading || !airdata) && ( + <div className="absolute inset-0 flex items-center justify-center z-[10000]"> <div className={`bg-white w-[64px] h-[64px] flex justify-center items-center rounded-md shadow-md p-3`} > <LoaderCircle className='animate-spin text-blue-600' width={32} height={32} /> </div> -</div>)} +</div> +)}
382-398:⚠️ Potential issueFix duplicate onClick handlers and incorrect button titles.
Multiple IconButton components share the same onClick handler and the third button has an incorrect title.
<IconButton onClick={() => setIsOpen(true)} title='Map Layers' icon={<LayerIcon width={24} height={24} fill={""}/>} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={() => { + // Implement refresh map functionality + if (mapRef.current) { + mapRef.current.getSource('data')?.setData(ConvertToGeojson([])); + // Re-fetch and update data + if (AirQoToken) { + GetAirQuoData(AirQoToken).then(data => { + if (mapRef.current && data) { + mapRef.current.getSource('data')?.setData(ConvertToGeojson(data)); + } + }); + } + } + }} title='Refresh Map' icon={<RefreshIcon width={24} height={24} fill={""} />} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={() => { + // Implement share map functionality + // For example: copy current map URL to clipboard + navigator.clipboard.writeText(window.location.href).then(() => { + alert('Map URL copied to clipboard'); + }); + }} - title='Refresh Map' + title='Share Map' icon={<ShareIcon width={24} height={24} fill={""} />} />src/netmanager-app/core/apis/MapData.ts (3)
18-34: 🛠️ Refactor suggestionImprove error handling and code structure in GetAirQuoData.
The function has several issues:
- Unnecessary block statement
- Inconsistent error handling
- Missing throw after error logging
+const API_BASE_URL = process.env.NEXT_PUBLIC_AIRQUO_DATA; export const GetAirQuoData = async (token: string) => { try { - if(token){ - const response = await axios.get(`${AirQuoData}?token=${token}`); - const data = response.data; - return data; - } - { - console.log("Error Token Is Missing") - return null; - } + if (!token) { + throw new Error('Token is missing'); + } + + const response = await axios.get(`${API_BASE_URL}`, { + params: { token } + }); + return response.data; } catch (error) { console.error('Error fetching GeoJSON:', error); + throw error; } }🧰 Tools
🪛 Biome (1.9.4)
[error] 25-28: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
36-60: 🛠️ Refactor suggestionImprove error handling and type safety in FetchSuggestions.
The function has type safety issues and inconsistent error handling.
-export const FetchSuggestions=async(value:string,access_token:string,sessionToken:string,latitude?: number, longitude?: number):Promise<any[] | void>=>{ +interface SuggestionResponse { + suggestions: Array<{ + mapbox_id: string; + name: string; + place_formatted: string; + [key: string]: any; + }>; +} + +export const FetchSuggestions = async ( + value: string, + access_token: string, + sessionToken: string, + latitude?: number, + longitude?: number +): Promise<Array<any>> => { if (!access_token) { - console.log("Missing Access Token"); - return; + throw new Error("Missing Access Token"); } if (latitude !== undefined && longitude !== undefined) { try { const proximityParam = `${longitude},${latitude}`; - const response = await axios.get(`${SUGGESTIONS}?q=${value.toLowerCase()}&access_token=${access_token}&proximity=${proximityParam}&session_token=${sessionToken}`) - const data = await response.data; - - console.log("data :",data.suggestions) - if (data.suggestions) { - return data.suggestions; - } else { - console.log("Error fetching suggestions"); - } + const response = await axios.get<SuggestionResponse>(`${SUGGESTIONS}`, { + params: { + q: value.toLowerCase(), + access_token, + proximity: proximityParam, + session_token: sessionToken + } + }); + return response.data.suggestions || []; } catch (error) { console.error("Error fetching suggestions:", error); + throw error; } } + return []; }🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
61-74: 🛠️ Refactor suggestionImprove error handling and type safety in UserClick.
The function lacks proper error handling with try-catch blocks and has inconsistent parameter usage.
-export const UserClick = async(access_token:string,sessionToken:string,locationid: string,)=>{ +interface LocationResponse { + features: Array<{ + geometry: { + coordinates: [number, number]; + }; + [key: string]: any; + }>; +} + +export const UserClick = async( + access_token: string, + sessionToken: string, + locationid: string +): Promise<LocationResponse> => { if (!access_token) { - console.log("Missing Access Token"); - return; + throw new Error("Missing Access Token"); } + try { - const response = await axios.get((`${SEARCH}/${locationid}?&access_token=${access_token}&session_token=${sessionToken}`)) - const data = response.data; - if(data){ - return data; - } + const response = await axios.get<LocationResponse>(`${SEARCH}/${locationid}`, { + params: { + access_token, + session_token: sessionToken + } + }); + return response.data; + } catch (error) { + console.error("Error in UserClick:", error); + throw error; + } }
🧹 Nitpick comments (3)
src/netmanager-app/components/NetManagerMap/page.tsx (2)
100-100: Remove console.log statements in production code.Console.log statements should be removed from production code as they can impact performance and expose sensitive information.
- console.log('GeoJson Data : ',geojsonData) + // Remove or replace with proper logging
20-22: Remove unused console.log statements.Unnecessary console.log statements should be removed from production code.
- // console.log(userGroup) - console.log(activeGroup) + // Remove console logssrc/netmanager-app/core/apis/MapData.ts (1)
1-6: Add validation for environment variables.The code directly uses environment variables without validation, which could lead to runtime errors if they are undefined.
import axios from "axios"; -const AirQuoData = process.env.NEXT_PUBLIC_AIRQUO_DATA -const SUGGESTIONS = process.env.NEXT_PUBLIC_SUGGESTIONS -const SEARCH = process.env.NEXT_PUBLIC_SEARCH -export const token = process.env.NEXT_PUBLIC_MAP_API_TOKEN -export const AirQoToken = process.env.NEXT_PUBLIC_AIRQO_DATA_TOKEN + +// Validate environment variables +const AirQuoData = process.env.NEXT_PUBLIC_AIRQUO_DATA || ''; +const SUGGESTIONS = process.env.NEXT_PUBLIC_SUGGESTIONS || ''; +const SEARCH = process.env.NEXT_PUBLIC_SEARCH || ''; +export const token = process.env.NEXT_PUBLIC_MAP_API_TOKEN || ''; +export const AirQoToken = process.env.NEXT_PUBLIC_AIRQO_DATA_TOKEN || ''; + +// Log warning if any critical environment variables are missing +if (!AirQuoData || !SUGGESTIONS || !SEARCH || !token || !AirQoToken) { + console.warn('Some environment variables are missing, map functionality may be limited'); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/platform/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
src/netmanager-app/app/(authenticated)/network-map/page.tsx(1 hunks)src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)src/netmanager-app/core/apis/MapData.ts(1 hunks)src/netmanager-app/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/netmanager-app/tsconfig.json
- src/netmanager-app/app/(authenticated)/network-map/page.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/core/apis/MapData.ts
[error] 36-36: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 25-28: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
[error] 116-119: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 118-118: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/netmanager-app/components/NetManagerMap/page.tsx (1)
123-143:⚠️ Potential issueSecurity concern: API tokens exposed in client-side code.
Tokens should not be directly exposed in client-side code. This could lead to unauthorized usage if extracted.
Consider implementing a server-side proxy for map and API interactions that would handle the token securely. This would prevent the token from being exposed in the client-side code.
Example implementation:
- Create a server-side API endpoint that proxies requests to Mapbox
- Use a relative URL to call your server endpoint instead of directly using the Mapbox API
🧹 Nitpick comments (11)
src/netmanager-app/components/NetManagerMap/components/LayerModal.tsx (6)
5-10: Improve MapStyle interface definition with proper spacing.The interface has inconsistent spacing and a hanging property with no trailing comma.
interface MapStyle { name: string; image: string; - url:string - + url: string; }
13-15: Define more specific image type for MapDetail interface.The
imageproperty has an ambiguousobjecttype which doesn't provide any type safety. Consider using a more specific type.interface MapDetail { name: string; - image: string | object; + image: string | { src: string; alt?: string }; }
17-26: Improve LayerModalProps interface formatting and add documentation.The interface has inconsistent indentation and would benefit from JSDoc comments.
interface LayerModalProps { - + /** Whether the modal is open */ isOpen: boolean; + /** Function to close the modal */ onClose: () => void; + /** Callback for when map details are selected */ onMapDetailsSelect: (name: string) => void; + /** Callback for when a map style is selected */ onStyleSelect: (style: MapStyle) => void; + /** Available map styles */ mapStyles: MapStyle[]; + /** Available map details */ mapDetails: MapDetail[]; + /** Optional name of disabled detail */ disabled?: string; }
37-47: Avoid redundant initialization in state and useEffect.The component initializes state with the first items and then does the same initialization in the useEffect, which is redundant.
- const [selectedStyle, setSelectedStyle] = useState<MapStyle>(mapStyles[0]); - const [selectedMapDetail, setSelectedMapDetail] = useState<MapDetail>(mapDetails[0]); + const [selectedStyle, setSelectedStyle] = useState<MapStyle | null>(null); + const [selectedMapDetail, setSelectedMapDetail] = useState<MapDetail | null>(null); useEffect(() => { if (mapStyles[0]) { setSelectedStyle(mapStyles[0]); } if (mapDetails[0]) { setSelectedMapDetail(mapDetails[0]); } }, [mapStyles, mapDetails]);Also ensure null checks in your handleApply function:
const handleApply = useCallback(() => { + if (!selectedStyle || !selectedMapDetail) return; onStyleSelect(selectedStyle); onMapDetailsSelect(selectedMapDetail.name); onClose(); }, [selectedStyle, selectedMapDetail, onStyleSelect, onMapDetailsSelect, onClose]);
109-118: Add error handling for image loading failures.Similar to the CountryList component, you should add error handling for image loading.
<div className="w-8 h-8 md:w-14 md:h-14 relative rounded-lg border-2"> <Image src={style.image} alt={style.name} layout="fill" objectFit="cover" className="rounded-lg" loading="eager" + onError={(e) => { + // Fallback in case the image fails to load + e.currentTarget.src = '/placeholder-map-style.png'; + }} /> </div>
63-142: Add keyboard accessibility for the modal.The modal lacks keyboard accessibility features - ensure users can navigate and interact using keyboard only.
Add the following to improve accessibility:
- Trap focus within the modal when open
- Add ESC key press handling to close the modal
- Make sure buttons have proper focus states
This would require adding a useEffect for keydown events:
useEffect(() => { if (!isOpen) return; const handleKeyDown = (e: KeyboardEvent) => { if (e.key === 'Escape') { onClose(); } }; window.addEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown); }, [isOpen, onClose]);And adding
tabIndexattributes to clickable elements if they're not already focusable.src/netmanager-app/components/NetManagerMap/components/CountryList.jsx (1)
74-101: TypeScript compatibility issues in JSX component.This is a JSX file using some TypeScript features but missing proper type definitions.
Consider:
- Converting the file to
.tsxextension- Adding proper type definitions for props
- Type-checking the event handlers
The component could also benefit from PropTypes if you prefer to keep it as JSX:
import PropTypes from 'prop-types'; // At the bottom of the file CountryList.propTypes = { siteDetails: PropTypes.array.isRequired, data: PropTypes.array, selectedCountry: PropTypes.object, setSelectedCountry: PropTypes.func.isRequired };src/netmanager-app/core/apis/MapData.ts (4)
1-6: Consider centralizing environment variables and using type safetyThe file starts with several environment variables without type checking or default values. Consider centralizing these in a config file with proper type safety to prevent runtime errors.
-const AirQuoData = process.env.NEXT_PUBLIC_AIRQUO_DATA -const SUGGESTIONS = process.env.NEXT_PUBLIC_SUGGESTIONS -const SEARCH = process.env.NEXT_PUBLIC_SEARCH -export const token = process.env.NEXT_PUBLIC_MAP_API_TOKEN -export const AirQoToken = process.env.NEXT_PUBLIC_AIRQO_DATA_TOKEN +// Create a config.ts file +interface EnvConfig { + AirQuoData: string; + SUGGESTIONS: string; + SEARCH: string; + MAP_API_TOKEN: string; + AIRQO_DATA_TOKEN: string; +} + +export const config: EnvConfig = { + AirQuoData: process.env.NEXT_PUBLIC_AIRQUO_DATA || '', + SUGGESTIONS: process.env.NEXT_PUBLIC_SUGGESTIONS || '', + SEARCH: process.env.NEXT_PUBLIC_SEARCH || '', + MAP_API_TOKEN: process.env.NEXT_PUBLIC_MAP_API_TOKEN || '', + AIRQO_DATA_TOKEN: process.env.NEXT_PUBLIC_AIRQO_DATA_TOKEN || '', +}; + +// Then import from config +import { config } from './config'; + +const { AirQuoData, SUGGESTIONS, SEARCH } = config; +export const token = config.MAP_API_TOKEN; +export const AirQoToken = config.AIRQO_DATA_TOKEN;
7-15: Consider using a more organized approach for image importsThe image imports are scattered and could be organized better for maintainability.
-import Node from '@/public/images/map/Node.webp'; -import Emoji from '@/public/images/map/Emoji.webp'; -import Heatmap from '@/public/images/map/Heatmap.webp'; -import Node_Number from '@/public/images/map/Node_number.webp'; - -import DarkMode from '@/public/images/map/dark.webp'; -import LightMode from '@/public/images/map/light.webp'; -import SatelliteMode from '@/public/images/map/satellite.webp'; -import StreetsMode from '@/public/images/map/street.webp'; +// Group imports by category +// Layer type images +import Node from '@/public/images/map/Node.webp'; +import Emoji from '@/public/images/map/Emoji.webp'; +import Heatmap from '@/public/images/map/Heatmap.webp'; +import Node_Number from '@/public/images/map/Node_number.webp'; + +// Map style images +import DarkMode from '@/public/images/map/dark.webp'; +import LightMode from '@/public/images/map/light.webp'; +import SatelliteMode from '@/public/images/map/satellite.webp'; +import StreetsMode from '@/public/images/map/street.webp';
75-94: Consider enhancing mapStyles with additional properties and type safetyThe current implementation of mapStyles could benefit from type definitions and potentially additional properties for better control over map styling.
+interface MapStyle { + url: string; + name: string; + image: any; // Ideally, use a more specific type for images + displayName?: string; // Optional for UI display + isDefault?: boolean; // To mark the default style +} + -export const mapStyles = [ +export const mapStyles: MapStyle[] = [ { url: 'mapbox://styles/mapbox/streets-v11', name: 'Streets', image: StreetsMode, + isDefault: true, // Mark this as default if appropriate }, { url: 'mapbox://styles/mapbox/light-v10', name: 'Light', image: LightMode }, { url: 'mapbox://styles/mapbox/dark-v10', name: 'Dark', image: DarkMode }, { url: 'mapbox://styles/mapbox/satellite-v9', name: 'Satellite', image: SatelliteMode, }, ];
96-113: Enhance mapDetails with type definitions and consistent formattingThe mapDetails array would benefit from proper type definitions and consistent object formatting.
+interface MapDetail { + name: string; + image: any; // Ideally, use a more specific type for images + description?: string; // Optional description of what this visualization shows +} + -export const mapDetails = [ +export const mapDetails: MapDetail[] = [ { name: 'Emoji', image: Emoji, + description: 'Displays air quality using emoji indicators' }, { name: 'Heatmap', image: Heatmap, + description: 'Shows air quality density as a heat map' }, { name: 'Node', image: Node, + description: 'Displays individual monitoring nodes' }, { name: 'Number', image: Node_Number, + description: 'Shows numerical readings at each node' }, ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/netmanager-app/app/(authenticated)/network-map/page.tsx(1 hunks)src/netmanager-app/app/globals.css(1 hunks)src/netmanager-app/components/NetManagerMap/components/CountryList.jsx(1 hunks)src/netmanager-app/components/NetManagerMap/components/LayerModal.tsx(1 hunks)src/netmanager-app/components/NetManagerMap/data/countries.json(1 hunks)src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)src/netmanager-app/core/apis/MapData.ts(1 hunks)src/netmanager-app/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/netmanager-app/components/NetManagerMap/data/countries.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/netmanager-app/app/(authenticated)/network-map/page.tsx
- src/netmanager-app/tsconfig.json
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/core/apis/MapData.ts
[error] 36-36: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 25-28: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
[error] 122-125: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 165-165: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 69-71: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (9)
src/netmanager-app/components/NetManagerMap/page.tsx (5)
33-33: Typeany[]should be avoided for suggestions.Define a proper interface for the suggestion items to improve type safety.
-const [suggestions, setSuggestions] = useState<any[]>([]); +interface LocationSuggestion { + mapbox_id: string; + name: string; + place_formatted: string; +} +const [suggestions, setSuggestions] = useState<LocationSuggestion[]>([]);
164-167: Fix incomplete error handling in mouseenter event.The mouseenter event handler has an incomplete condition block.
const feature = e.features ? e.features[0] : null; if (feature && feature.properties) { - + // Handle feature properties + const { location_name, value, aqi_category } = feature.properties; + if (!location_name || !aqi_category) { + console.warn('Invalid feature properties'); + return; + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 165-165: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
182-182: Fix incorrect image selection logic in tooltip.The tooltip always uses
AirQuality.goodairregardless of the actual air quality category, which is misleading.-<img src="${AirQuality.goodair}" style="background-color: ${feature.properties?.aqi_color ?? 'green'};width: 30px; height: 30px;border-radius: 50%;font-size: 18px;"></img> +<img src="${feature.properties?.aqi_category ? AirQuality[feature.properties.aqi_category.toLowerCase().replace(/\s+/g, '')] || AirQuality.unknownAQ : AirQuality.unknownAQ}" style="width: 30px; height: 30px;border-radius: 50%;font-size: 18px;background-color: ${feature.properties?.aqi_color ?? 'green'};"></img>
413-419: Fix conditional rendering syntax.The conditional rendering syntax has an issue with operator precedence.
-{ (loading || !airdata) &&(<div className="absolute inset-0 flex items-center justify-center z-[10000]"> +{(loading || !airdata) && ( + <div className="absolute inset-0 flex items-center justify-center z-[10000]"> <div className={`bg-white w-[64px] h-[64px] flex justify-center items-center rounded-md shadow-md p-3`} > <LoaderCircle className='animate-spin text-blue-600' width={32} height={32} /> </div> -</div>)} + </div> +)}
424-440: Fix duplicate onClick handlers and titles.Multiple IconButton components share the same onClick handler and have incorrect titles.
<IconButton onClick={() => setIsOpen(true)} title='Map Layers' icon={<LayerIcon width={24} height={24} fill={""}/>} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={() => { + // Add refresh functionality here + if (mapRef.current) { + mapRef.current.getSource('data')?.setData(/* refreshed data */); + } + }} title='Refresh Map' icon={<RefreshIcon width={24} height={24} fill={""} />} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={() => { + // Add share functionality here + const url = window.location.href; + navigator.clipboard.writeText(url); + // Consider adding a toast notification here + }} - title='Refresh Map' + title='Share Map' icon={<ShareIcon width={24} height={24} fill={""} />} />src/netmanager-app/core/apis/MapData.ts (4)
18-34: Improve error handling and code structure in GetAirQuoDataThe function has several issues:
- Unnecessary block statement
- Inconsistent error handling
- Hard-coded URL
🧰 Tools
🪛 Biome (1.9.4)
[error] 25-28: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
36-60: Improve error handling and type safety in FetchSuggestionsThe function has several issues:
- Return type includes
void- Inconsistent error handling
- Hard-coded URL
- Unnecessary console.log statements
🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
61-74: Improve error handling and type safety in UserClickThe function has several issues:
- No try-catch block
- Inconsistent error handling
- Hard-coded URL
115-131: Remove duplicated UserGroups functionThe UserGroups function is identical to GetAirQuoData, which creates unnecessary code duplication.
🧰 Tools
🪛 Biome (1.9.4)
[error] 122-125: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
src/netmanager-app/app/globals.css
Outdated
| .country-scroll-bar { | ||
| overflow-x: auto; | ||
| overflow-y: hidden; | ||
| cursor: pointer; | ||
|
|
||
| /* Hide the scrollbar by default */ | ||
| &::-webkit-scrollbar { | ||
| height: 4px; | ||
| display: none; | ||
| } | ||
|
|
||
| /* // Show the scrollbar when the element is focused */ | ||
| &:hover::-webkit-scrollbar { | ||
| height: 4px; | ||
| display: block; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix CSS nesting syntax for better browser compatibility.
The CSS nesting syntax using & selectors isn't supported in regular CSS files. This works in preprocessors like SASS/SCSS but requires proper CSS nesting syntax for broader browser compatibility.
Apply this fix:
.country-scroll-bar {
overflow-x: auto;
overflow-y: hidden;
cursor: pointer;
-/* Hide the scrollbar by default */
-&::-webkit-scrollbar {
+}
+/* Hide the scrollbar by default */
+.country-scroll-bar::-webkit-scrollbar {
height: 4px;
display: none;
}
/* // Show the scrollbar when the element is focused */
-&:hover::-webkit-scrollbar {
+.country-scroll-bar:hover::-webkit-scrollbar {
height: 4px;
display: block;
}
- }Also consider adding Firefox scrollbar styling for cross-browser compatibility:
.country-scroll-bar {
scrollbar-width: none; /* Firefox */
}
.country-scroll-bar:hover {
scrollbar-width: thin; /* Firefox */
scrollbar-color: rgba(0, 0, 0, 0.3) transparent; /* Firefox */
}| > | ||
| <div className="w-8 h-8 md:w-14 md:h-14 relative rounded-lg border-2"> | ||
| <Image | ||
| src={typeof detail.image === 'string' ? detail.image : ''} | ||
| alt={detail.name} | ||
| layout="fill" | ||
| objectFit="cover" | ||
| className="rounded-lg" | ||
| loading="eager" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential null image sources correctly.
The component tries to handle image sources that might be objects, but doesn't properly account for all cases.
<div className="w-8 h-8 md:w-14 md:h-14 relative rounded-lg border-2">
<Image
- src={typeof detail.image === 'string' ? detail.image : ''}
+ src={typeof detail.image === 'string' ? detail.image : (detail.image && 'src' in detail.image ? detail.image.src : '/placeholder-image.png')}
alt={detail.name}
layout="fill"
objectFit="cover"
className="rounded-lg"
loading="eager"
/>
</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="w-8 h-8 md:w-14 md:h-14 relative rounded-lg border-2"> | |
| <Image | |
| src={typeof detail.image === 'string' ? detail.image : ''} | |
| alt={detail.name} | |
| layout="fill" | |
| objectFit="cover" | |
| className="rounded-lg" | |
| loading="eager" | |
| /> | |
| </div> | |
| > | |
| <div className="w-8 h-8 md:w-14 md:h-14 relative rounded-lg border-2"> | |
| <Image | |
| src={typeof detail.image === 'string' ? detail.image : (detail.image && 'src' in detail.image ? detail.image.src : '/placeholder-image.png')} | |
| alt={detail.name} | |
| layout="fill" | |
| objectFit="cover" | |
| className="rounded-lg" | |
| loading="eager" | |
| /> | |
| </div> |
| const handleLocationClick = (locationid: any) => { | ||
| UserClick(token?token:"",sessionToken?sessionToken:"",locationid) | ||
| .then(data => { | ||
| console.log(data.features[0].geometry.coordinates) | ||
| if (mapRef.current) { | ||
| mapRef.current.flyTo({ | ||
| center: data.features[0].geometry.coordinates, | ||
| zoom: 12, | ||
| essential: true | ||
| }); | ||
| } | ||
| }) | ||
| .catch(error => console.error("Error fetching location:", error)); | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for UserClick function.
The UserClick function doesn't properly handle missing tokens or validate the response structure.
const handleLocationClick = (locationid: any) => {
- UserClick(token?token:"",sessionToken?sessionToken:"",locationid)
+ if (!token || !sessionToken) {
+ console.error('Missing required tokens');
+ return;
+ }
+
+ UserClick(token, sessionToken, locationid)
.then(data => {
+ if (!data || !data.features || !data.features.length || !data.features[0].geometry || !data.features[0].geometry.coordinates) {
+ console.error('Invalid response format');
+ return;
+ }
console.log(data.features[0].geometry.coordinates)
if (mapRef.current) {
mapRef.current.flyTo({
center: data.features[0].geometry.coordinates,
zoom: 12,
essential: true
});
}
})
- .catch(error => console.error("Error fetching location:", error));
+ .catch(error => {
+ console.error("Error fetching location:", error);
+ // Consider showing a user-friendly error message
+ });
}📝 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 handleLocationClick = (locationid: any) => { | |
| UserClick(token?token:"",sessionToken?sessionToken:"",locationid) | |
| .then(data => { | |
| console.log(data.features[0].geometry.coordinates) | |
| if (mapRef.current) { | |
| mapRef.current.flyTo({ | |
| center: data.features[0].geometry.coordinates, | |
| zoom: 12, | |
| essential: true | |
| }); | |
| } | |
| }) | |
| .catch(error => console.error("Error fetching location:", error)); | |
| } | |
| const handleLocationClick = (locationid: any) => { | |
| if (!token || !sessionToken) { | |
| console.error('Missing required tokens'); | |
| return; | |
| } | |
| UserClick(token, sessionToken, locationid) | |
| .then(data => { | |
| if ( | |
| !data || | |
| !data.features || | |
| !data.features.length || | |
| !data.features[0].geometry || | |
| !data.features[0].geometry.coordinates | |
| ) { | |
| console.error('Invalid response format'); | |
| return; | |
| } | |
| console.log(data.features[0].geometry.coordinates); | |
| if (mapRef.current) { | |
| mapRef.current.flyTo({ | |
| center: data.features[0].geometry.coordinates, | |
| zoom: 12, | |
| essential: true | |
| }); | |
| } | |
| }) | |
| .catch(error => { | |
| console.error("Error fetching location:", error); | |
| // Consider showing a user-friendly error message | |
| }); | |
| } |
| /> | ||
| </div> | ||
|
|
||
| <div className='flex border border-2' > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove empty border div.
This div only contains a border and has no content or purpose.
-<div className='flex border border-2' >
+<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='flex border border-2' > | |
| <div> |
| 'icon-image': [ | ||
| 'match', | ||
| ['get', 'aqi_category'], | ||
| 'Good', 'goodair', | ||
| 'Moderate', 'moderate', | ||
| 'Unhealthy', 'unhealthy', | ||
| 'Unhealthy for Sensitive Groups', 'unhealthySG', | ||
| 'Hazardous','Hazardous', | ||
| 'Very Unhealthy', 'veryunhealthy', | ||
| 'unknownAQ' | ||
| ], | ||
| 'icon-size': 0.05, | ||
| 'icon-padding': 0, | ||
| 'icon-allow-overlap': true, | ||
| 'icon-ignore-placement': true, | ||
| }, |
There was a problem hiding this comment.
Fix potential map symbol mismatch.
The layer definition includes a mismatch between the 'match' expression case value and the image name for 'Hazardous'.
mapRef.current.addLayer({
'id': 'data-layer',
'type': 'symbol',
'source': 'data',
'layout': {
'icon-image': [
'match',
['get', 'aqi_category'],
'Good', 'goodair',
'Moderate', 'moderate',
'Unhealthy', 'unhealthy',
'Unhealthy for Sensitive Groups', 'unhealthySG',
- 'Hazardous','Hazardous',
+ 'Hazardous','hazardous',
'Very Unhealthy', 'veryunhealthy',
'unknownAQ'
],
'icon-size': 0.05,
'icon-padding': 0,
'icon-allow-overlap': true,
'icon-ignore-placement': 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.
| 'icon-image': [ | |
| 'match', | |
| ['get', 'aqi_category'], | |
| 'Good', 'goodair', | |
| 'Moderate', 'moderate', | |
| 'Unhealthy', 'unhealthy', | |
| 'Unhealthy for Sensitive Groups', 'unhealthySG', | |
| 'Hazardous','Hazardous', | |
| 'Very Unhealthy', 'veryunhealthy', | |
| 'unknownAQ' | |
| ], | |
| 'icon-size': 0.05, | |
| 'icon-padding': 0, | |
| 'icon-allow-overlap': true, | |
| 'icon-ignore-placement': true, | |
| }, | |
| mapRef.current.addLayer({ | |
| 'id': 'data-layer', | |
| 'type': 'symbol', | |
| 'source': 'data', | |
| 'layout': { | |
| 'icon-image': [ | |
| 'match', | |
| ['get', 'aqi_category'], | |
| 'Good', 'goodair', | |
| 'Moderate', 'moderate', | |
| 'Unhealthy', 'unhealthy', | |
| 'Unhealthy for Sensitive Groups', 'unhealthySG', | |
| 'Hazardous', 'hazardous', | |
| 'Very Unhealthy', 'veryunhealthy', | |
| 'unknownAQ' | |
| ], | |
| 'icon-size': 0.05, | |
| 'icon-padding': 0, | |
| 'icon-allow-overlap': true, | |
| 'icon-ignore-placement': true, | |
| }, | |
| }); |
| onError={(e) => { | ||
| // Fallback in case the flag image fails to load | ||
| e.target.onerror = null; // Prevent infinite loop | ||
| e.target.src = '/path-to-default-flag-image.png'; // Provide default flag image path |
There was a problem hiding this comment.
Replace placeholder default flag image path.
The fallback image path is currently a placeholder and should be replaced with an actual path.
-e.target.src = '/path-to-default-flag-image.png'; // Provide default flag image path
+e.target.src = '/images/map/default-flag.png'; // Use an actual image path from your projectMake sure this image file exists in your project's public directory.
📝 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.
| e.target.src = '/path-to-default-flag-image.png'; // Provide default flag image path | |
| e.target.src = '/images/map/default-flag.png'; // Use an actual image path from your project |
| // import Button from '@material-ui/core/Button'; | ||
| // import { | ||
| // setLocation, | ||
| // addSuggestedSites, | ||
| // } from '@/lib/store/services/map/MapSlice'; |
There was a problem hiding this comment.
Uncomment required imports and remove unnecessary ones.
The component uses Redux actions and a Button component that are either commented out or missing imports.
-// import Button from '@material-ui/core/Button';
-// import {
-// setLocation,
-// addSuggestedSites,
-// } from '@/lib/store/services/map/MapSlice';
+import { Button } from '../../ui/button';
+import {
+ setLocation,
+ addSuggestedSites,
+} from '@/lib/store/services/map/MapSlice';If the Button component from Material-UI is not available, ensure you're using the correct Button component from your design system.
📝 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 Button from '@material-ui/core/Button'; | |
| // import { | |
| // setLocation, | |
| // addSuggestedSites, | |
| // } from '@/lib/store/services/map/MapSlice'; | |
| import { Button } from '../../ui/button'; | |
| import { | |
| setLocation, | |
| addSuggestedSites, | |
| } from '@/lib/store/services/map/MapSlice'; |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
src/netmanager-app/components/NetManagerMap/page.tsx (7)
7-7:⚠️ Potential issueSecurity concern: API tokens exposed in client-side code.
API tokens are imported directly in the client-side code, which poses a security risk. Consider implementing a server-side proxy or environment variables that can be accessed securely.
-import { GetAirQuoData,FetchSuggestions,UserClick,token,AirQoToken,mapStyles, mapDetails } from '@/core/apis/MapData'; +import { GetAirQuoData,FetchSuggestions,UserClick,mapStyles, mapDetails } from '@/core/apis/MapData'; +// Use environment variables or server-side API endpoints to securely handle tokens
160-163:⚠️ Potential issueFix incomplete error handling in mouseenter event.
The mouseenter event handler has an empty condition block that doesn't handle feature properties.
const feature = e.features ? e.features[0] : null; if (feature && feature.properties) { - + // Handle feature properties + const { location_name, value, aqi_category } = feature.properties; + if (!location_name || !aqi_category) { + console.warn('Invalid feature properties'); + return; + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 161-161: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
178-178:⚠️ Potential issueFix incorrect image selection in tooltip.
The tooltip always uses
AirQuality.goodairregardless of the actual air quality category, which provides misleading information to users.-<img src="${AirQuality.goodair}" style="background-color: ${feature.properties?.aqi_color ?? 'green'};width: 30px; height: 30px;border-radius: 50%;font-size: 18px;"></img> +<img src="${feature.properties?.aqi_category ? AirQuality[feature.properties.aqi_category.toLowerCase().replace(/\s+/g, '')] || AirQuality.unknownAQ : AirQuality.unknownAQ}" style="width: 30px; height: 30px;border-radius: 50%;font-size: 18px;background-color: ${feature.properties?.aqi_color ?? 'green'};"></img>
245-260:⚠️ Potential issueFix case mismatch in the map symbol definition.
There's a mismatch between the 'match' expression case value and the image name for 'Hazardous' - one is capitalized in the map layer but lowercase in the AirQuality object.
'icon-image': [ 'match', ['get', 'aqi_category'], 'Good', 'goodair', 'Moderate', 'moderate', 'Unhealthy', 'unhealthy', 'Unhealthy for Sensitive Groups', 'unhealthySG', - 'Hazardous','Hazardous', + 'Hazardous','hazardous', 'Very Unhealthy', 'veryunhealthy', 'unknownAQ' ],
328-342:⚠️ Potential issueImprove error handling for UserClick function.
The function doesn't properly validate tokens or check the response structure before using it.
const handleLocationClick = (locationid: any) => { - UserClick(token?token:"",sessionToken?sessionToken:"",locationid) + if (!token || !sessionToken) { + console.error('Missing required tokens'); + return; + } + + UserClick(token, sessionToken, locationid) .then(data => { + if (!data || !data.features || !data.features.length || !data.features[0].geometry || !data.features[0].geometry.coordinates) { + console.error('Invalid response format'); + return; + } console.log(data.features[0].geometry.coordinates) if (mapRef.current) { mapRef.current.flyTo({ center: data.features[0].geometry.coordinates, zoom: 12, essential: true }); } }) - .catch(error => console.error("Error fetching location:", error)); + .catch(error => { + console.error("Error fetching location:", error); + // Consider showing a user-friendly error message + }); }
408-414:⚠️ Potential issueFix conditional rendering syntax.
The conditional rendering has an issue with operator precedence that could cause unexpected behavior.
-{ loading ||!airdata &&(<div className="absolute inset-0 flex items-center justify-center z-[10000]"> +{ (loading || !airdata) && (<div className="absolute inset-0 flex items-center justify-center z-[10000]"> <div className={`bg-white w-[64px] h-[64px] flex justify-center items-center rounded-md shadow-md p-3`} > <LoaderCircle className='animate-spin text-blue-600' width={32} height={32} /> </div> </div>)}
419-435:⚠️ Potential issueFix duplicate onClick handlers and titles.
Multiple IconButton components share the same onClick handler and have incorrect titles.
<IconButton onClick={() => setIsOpen(true)} title='Map Layers' icon={<LayerIcon width={24} height={24} fill={""}/>} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={() => { /* Add refresh map functionality */ }} title='Refresh Map' icon={<RefreshIcon width={24} height={24} fill={""} />} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={() => { /* Add share functionality */ }} - title='Refresh Map' + title='Share Map' icon={<ShareIcon width={24} height={24} fill={""} />} />src/netmanager-app/components/NetManagerMap/components/CountryList.jsx (1)
3-7:⚠️ Potential issueFix component imports and uncomment required code.
The component is importing a custom Button component but using a native button element. Additionally, Redux actions are being used but their imports are commented out.
-import Button from './Button' +// If you're not using the Button component, remove this import -// import { -// setLocation, -// addSuggestedSites, -// } from '@/lib/store/services/map/MapSlice'; +import { + setLocation, + addSuggestedSites, +} from '@/lib/store/services/map/MapSlice';
🧹 Nitpick comments (5)
src/netmanager-app/app/globals.css (2)
78-83: Remove Duplicateoverflow-xDeclaration in.country-scroll-bar
The new.country-scroll-barclass currently declaresoverflow-x: auto;twice (lines 79 and 81). Removing the duplicate will simplify the code and avoid any potential confusion in future maintenance.[minor]
Suggested diff:
.country-scroll-bar { - overflow-x: auto; overflow-y: hidden; - overflow-x: auto; + overflow-x: auto; cursor: pointer; }
84-91: Verify and Standardize Scrollbar Hiding Styles
The newly introduced.scrollbar-hideclass and its pseudo-element rule effectively hide scrollbars in WebKit browsers while also addressing IE/Edge and Firefox compatibility via-ms-overflow-styleandscrollbar-width. Please confirm that these styles are meant to be applied separately from.country-scroll-bar. If the intent is always to hide the scrollbar for elements using.country-scroll-bar, consider merging these rules (or applying the pseudo-element style directly to.country-scroll-bar) to enhance clarity and maintainability. Additionally, standardize the indentation within this block to improve readability.Example of merged styling if applicable:
-.country-scroll-bar { - overflow-x: auto; - overflow-y: hidden; - cursor: pointer; -} -.scrollbar-hide::-webkit-scrollbar { - display: none; -} -.scrollbar-hide { - -ms-overflow-style: none; - scrollbar-width: none; -} +.country-scroll-bar { + overflow-x: auto; + overflow-y: hidden; + cursor: pointer; +} +/* If elements using .country-scroll-bar should also hide the scrollbar */ +.country-scroll-bar::-webkit-scrollbar { + display: none; +} +.country-scroll-bar { + -ms-overflow-style: none; + scrollbar-width: none; +}(Only apply this refactor if the intention is for both styles to always be used together.)
src/netmanager-app/components/NetManagerMap/components/Button/index.jsx (2)
51-51: Consider breaking down the long className string for better readability.The buttonClass string combines many CSS classes, making it difficult to read and maintain. Consider using an array and joining the elements for better readability.
- const buttonClass = `flex justify-center items-center cursor-pointer ${paddingStyles} rounded-xl sm:gap-1 ${className} ${textColor} ${backgroundColor} ${border} transition transform active:scale-95 ${disabled ? 'cursor-not-allowed opacity-50' : ''}`; + const buttonClasses = [ + 'flex justify-center items-center cursor-pointer', + paddingStyles, + 'rounded-xl sm:gap-1', + className, + textColor, + backgroundColor, + border, + 'transition transform active:scale-95', + disabled ? 'cursor-not-allowed opacity-50' : '' + ]; + const buttonClass = buttonClasses.join(' ');
64-76: Consider adding accessibility attributes.While the component handles basic functionality well, consider enhancing accessibility by adding appropriate aria attributes, especially for different button states.
<button onClick={onClick} className={buttonClass} data-testid={dataTestId} type={type} disabled={disabled} + aria-disabled={disabled} + aria-label={typeof children === 'string' ? children : undefined} {...rest} > {Icon && <Icon className="w-4 h-4 mr-2" />} {/* Icon before text */} {children} </button>src/netmanager-app/components/NetManagerMap/components/Button/TabButtons.jsx (1)
13-18: Add TypeScript typing or PropTypes for the component props.While you've defined PropTypes at the bottom, it would be good to convert this component to TypeScript or enhance the existing PropTypes to ensure complete type safety for all props.
-const CountryList = ({ +const CountryList: React.FC<{ + siteDetails: any[]; // Consider defining a more specific type + data: any[]; // Consider defining a more specific type + selectedCountry: any; // Consider defining a more specific type + setSelectedCountry: (country: any) => void; +}> = ({ siteDetails, data, selectedCountry, setSelectedCountry, }) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/netmanager-app/app/globals.css(1 hunks)src/netmanager-app/components/NetManagerMap/components/Button/TabButtons.jsx(1 hunks)src/netmanager-app/components/NetManagerMap/components/Button/index.jsx(1 hunks)src/netmanager-app/components/NetManagerMap/components/CountryList.jsx(1 hunks)src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 161-161: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (12)
src/netmanager-app/components/NetManagerMap/components/Button/index.jsx (3)
3-17: Well-structured button component with comprehensive props.The component accepts a good variety of props that make it versatile for different use cases throughout the application. The default values are sensible, and the spread operator for additional props provides flexibility.
19-45: Clean variant-based styling approach.Using an object to define variant styles is a good pattern that makes the code more maintainable and scalable. The approach clearly separates different visual states and allows for easy addition of new variants in the future.
54-61: Good conditional rendering based on path prop.The component intelligently renders either an anchor or button element based on the presence of the path prop, which is a good practice for flexibility.
src/netmanager-app/components/NetManagerMap/components/Button/TabButtons.jsx (2)
6-46: Clean use of memoization and callback for performance optimization.The component correctly uses
useMemofor sorting data anduseCallbackfor event handlers, which helps prevent unnecessary re-renders and recalculations. The dependency arrays are also properly defined.
84-94:⚠️ Potential issueReplace placeholder fallback image path.
The fallback image path is currently a placeholder and should be replaced with an actual path to an existing image in your project.
onError={(e) => { e.target.onerror = null; - e.target.src = '/path-to-default-flag-image.png'; + e.target.src = '/images/map/default-flag.png'; // Use an actual image path from your project }}src/netmanager-app/components/NetManagerMap/page.tsx (2)
33-33: Define proper interface for the suggestion items.Using
any[]for the suggestions state reduces type safety. Consider defining a proper interface for location suggestions.-const [suggestions, setSuggestions] = useState<any[]>([]); +interface LocationSuggestion { + mapbox_id: string; + name: string; + place_formatted: string; +} +const [suggestions, setSuggestions] = useState<LocationSuggestion[]>([]);
438-438: Remove empty border div.This div only contains border styling and serves no functional purpose.
-<div className='flex border border-2' > +<div>src/netmanager-app/components/NetManagerMap/components/CountryList.jsx (5)
21-26: Excellent use of useMemo for optimizing sorted data.The component properly memoizes the sorted data to prevent unnecessary re-sorting on each render, which is good for performance. The dependency array is correctly set to only the data prop.
28-46: Good use of useCallback for optimizing event handlers.The handleClick function is properly wrapped in useCallback with the correct dependency array, which helps prevent unnecessary function recreations on re-renders.
49-60: Good fallback UI for loading state.The component provides a clear loading indicator when data is not available, which is a good practice for user experience.
75-98: Well-structured country selection buttons.The button implementation provides clear visual feedback for the selected country and includes appropriate flag images with error handling.
84-94:❓ Verification inconclusive
Replace placeholder fallback image path.
The fallback image path is set to a placeholder that doesn't exist in your project.
onError={(e) => { e.target.onerror = null; - e.target.src = '/path-to-default-flag-image.png'; + e.target.src = '/images/map/default-flag.png'; // Use an actual image path from your project }}
🏁 Script executed:
#!/bin/bash # Check if the default flag image exists in the public directory fd "default-flag.png" public/images/mapLength of output: 144
#!/bin/bash # Re-run a search for "default-flag.png" anywhere in the public directory to better locate the file fd "default-flag.png" publicThe above script should help us determine if the fallback image file exists anywhere under the public folder. If nothing shows up, please manually verify whether the default flag image exists in your project and update its path accordingly.
| import React from 'react'; | ||
| import ChevronDownIcon from '@/icons/Common/chevron_downIcon'; | ||
| import PropTypes from 'prop-types'; | ||
| import Spinner from '../Spinner'; | ||
|
|
||
| /** | ||
| * TabButtons Component | ||
| * |
There was a problem hiding this comment.
Uncomment required Redux imports or remove unused code.
The component uses Redux dispatch actions (setLocation and addSuggestedSites) at lines 35 and 43, but their imports are commented out. This will cause runtime errors when those functions are called.
import React, { useCallback, useMemo } from 'react';
import { useDispatch } from 'react-redux';
import Button from './Button'
-// import {
-// setLocation,
-// addSuggestedSites,
-// } from '@/lib/store/services/map/MapSlice';
+import {
+ setLocation,
+ addSuggestedSites,
+} from '@/lib/store/services/map/MapSlice';📝 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 from 'react'; | |
| import ChevronDownIcon from '@/icons/Common/chevron_downIcon'; | |
| import PropTypes from 'prop-types'; | |
| import Spinner from '../Spinner'; | |
| /** | |
| * TabButtons Component | |
| * | |
| import React, { useCallback, useMemo } from 'react'; | |
| import { useDispatch } from 'react-redux'; | |
| import Button from './Button' | |
| import { | |
| setLocation, | |
| addSuggestedSites, | |
| } from '@/lib/store/services/map/MapSlice'; | |
| import ChevronDownIcon from '@/icons/Common/chevron_downIcon'; | |
| import PropTypes from 'prop-types'; | |
| import Spinner from '../Spinner'; | |
| /** | |
| * TabButtons Component | |
| * |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
src/netmanager-app/components/NetManagerMap/components/CountryList.jsx (1)
93-93:⚠️ Potential issueReplace placeholder default flag image path
The fallback image path is currently a placeholder and should be replaced with an actual path from your project.
-e.target.src = '/path-to-default-flag-image.png'; +e.target.src = '/images/map/default-flag.png'; // Use an actual image path from your projectsrc/netmanager-app/components/NetManagerMap/page.tsx (6)
7-7:⚠️ Potential issueSecurity concern: Exposed API tokens
API tokens are imported directly into client-side code, which poses a security risk. Consider moving token handling to server-side API endpoints.
-import { GetAirQuoData,token,AirQoToken,mapStyles, mapDetails } from '@/core/apis/MapData'; +import { GetAirQuoData, mapStyles, mapDetails } from '@/core/apis/MapData';Then create an API endpoint to handle token-secured requests:
// Example API endpoint approach async function getMapData(req, res) { // Tokens stored in environment variables on server const response = await fetch(url, { headers: { Authorization: `Bearer ${process.env.AIRQO_TOKEN}` } }); const data = await response.json(); return res.json(data); }
179-182:⚠️ Potential issueFix incomplete error handling in mouseenter event
The mouseenter event handler has an incomplete condition block with no action.
const feature = e.features ? e.features[0] : null; if (feature && feature.properties) { - + // Handle feature properties + const { location_name, aqi_category } = feature.properties; + if (!location_name || !aqi_category) { + console.warn('Invalid feature properties'); + return; + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 180-180: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
197-197:⚠️ Potential issueFix incorrect image selection in tooltip
The tooltip always uses
AirQuality.goodairregardless of the actual air quality category, which is misleading.-<img src="${AirQuality.goodair}" style="background-color: ${feature.properties?.aqi_color ?? 'green'};width: 30px; height: 30px;border-radius: 50%;font-size: 18px;"></img> +<img src="${feature.properties?.aqi_category ? AirQuality[feature.properties.aqi_category.toLowerCase().replace(/\s+/g, '')] || AirQuality.unknownAQ : AirQuality.unknownAQ}" style="width: 30px; height: 30px;border-radius: 50%;font-size: 18px;background-color: ${feature.properties?.aqi_color ?? 'green'};"></img>
264-279:⚠️ Potential issueFix map symbol mismatch for Hazardous air quality
There's a mismatch between the 'match' expression case value and the image name for 'Hazardous'.
'icon-image': [ 'match', ['get', 'aqi_category'], 'Good', 'goodair', 'Moderate', 'moderate', 'Unhealthy', 'unhealthy', 'Unhealthy for Sensitive Groups', 'unhealthySG', - 'Hazardous','Hazardous', + 'Hazardous','hazardous', 'Very Unhealthy', 'veryunhealthy', 'unknownAQ' ],
308-314:⚠️ Potential issueFix conditional rendering syntax
The conditional rendering syntax has a minor issue that could lead to unexpected behavior.
-{ ( loading || !airdata) &&(<div className="absolute inset-0 flex items-center justify-center z-[10000]"> +{ (loading || !airdata) && (<div className="absolute inset-0 flex items-center justify-center z-[10000]"> <div className={`bg-white w-[64px] h-[64px] flex justify-center items-center rounded-md shadow-md p-3`} > <LoaderCircle className='animate-spin text-blue-600' width={32} height={32} /> </div> </div>)}
319-335:⚠️ Potential issueFix duplicate onClick handlers and titles
Multiple IconButton components share the same onClick handler and incorrect titles.
<IconButton onClick={() => setIsOpen(true)} title='Map Layers' icon={<LayerIcon width={24} height={24} fill={""}/>} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={() => { + // Add refreshMap implementation + if (mapRef.current) { + mapRef.current.once('idle', () => { + setLoading(true); + const currentStyle = mapRef.current?.getStyle(); + mapRef.current?.setStyle(currentStyle); + setTimeout(() => setLoading(false), 1000); + }); + } + }} title='Refresh Map' icon={<RefreshIcon width={24} height={24} fill={""} />} /> <IconButton - onClick={() => setIsOpen(true)} + onClick={() => { + // Implement share functionality + const url = window.location.href; + navigator.clipboard.writeText(url).then(() => { + alert('Map URL copied to clipboard'); + }); + }} - title='Refresh Map' + title='Share Map' icon={<ShareIcon width={24} height={24} fill={""} />} />src/netmanager-app/components/NetManagerMap/components/Sidebar/page.tsx (1)
79-84:⚠️ Potential issueImprove error handling for UserClick function
The function doesn't properly handle missing tokens. Use proper error handling and consider providing user feedback.
const handleLocationClick = (locationid: any) => { - UserClick(token?token:"",sessionToken?sessionToken:"",locationid) + if (!token || !sessionToken) { + console.error('Missing required tokens'); + // Consider showing a user-facing error message + return; + } + + UserClick(token, sessionToken, locationid) .then(data => { + if (!data || !data.features || !data.features.length || !data.features[0].geometry || !data.features[0].geometry.coordinates) { + console.error('Invalid response format'); + return; + } console.log(data.features[0].geometry.coordinates) handleUserClick(data) }) - .catch(error => console.error("Error fetching location:", error)); + .catch(error => { + console.error("Error fetching location:", error); + // Consider showing a user-friendly error message + }); }
🧹 Nitpick comments (10)
src/netmanager-app/components/NetManagerMap/components/CountryList.jsx (1)
76-101: Consider using optional chaining and improve accessibilityThe JSX structure looks good, but consider a few improvements:
- Use optional chaining for country.country
- Add ARIA attributes for better accessibility
<button key={index} type="button" className={`flex items-center gap-2 px-4 py-2 rounded-xl bg-gray-100 hover:bg-gray-200 min-w-max transition-all ${ - selectedCountry?.country === country.country ? 'border-2 border-blue-400' : '' + selectedCountry?.country === country?.country ? 'border-2 border-blue-400' : '' }`} onClick={() => handleClick(country)} + aria-pressed={selectedCountry?.country === country?.country} + aria-label={`Select ${country?.country}`} >src/netmanager-app/components/NetManagerMap/page.tsx (1)
26-27: Remove console.log statementsRemove debug console.log statements before pushing to production. These statements expose implementation details and may impact performance.
-console.log(activeGroup) -console.log("GridsData Summary: ", sites); -console.log("unique Countries",uniqueCountries) -console.log('GeoJson Data : ',geojsonData)Also applies to: 45-45, 101-101, 162-162
src/netmanager-app/lib/map/MapSlice.js (4)
1-22: Convert to TypeScript for better type safetyConsider converting this file to TypeScript with proper type definitions to improve type safety and developer experience.
-import { createSlice } from '@reduxjs/toolkit'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; +interface Location { + country: string; + city: string; +} +interface MapCenter { + latitude: number; + longitude: number; +} +interface MapState { + location: Location; + center: MapCenter; + zoom: number; + showLocationDetails: boolean; + selectedLocation: any | null; + mapLoading: boolean; + suggestedSites: any[]; + selectedNode: any | null; + selectedWeeklyPrediction: any | null; + mapReadingsData: any[]; + waqData: any[]; +} -const initialState = { +const initialState: MapState = { location: { country: '', city: '', }, center: { latitude: 9.7037, longitude: 19.9573, }, zoom: 3.27, showLocationDetails: false, selectedLocation: null, mapLoading: false, suggestedSites: [], selectedNode: null, selectedWeeklyPrediction: null, mapReadingsData: [], waqData: [], };
27-38: Add type safety to reducer actionsImprove type safety by adding proper type definitions to action payloads.
reducers: { - setSelectedWeeklyPrediction: (state, action) => { + setSelectedWeeklyPrediction: (state, action: PayloadAction<any | null>) => { state.selectedWeeklyPrediction = action.payload; }, - setSelectedNode: (state, action) => { + setSelectedNode: (state, action: PayloadAction<any | null>) => { state.selectedNode = action.payload; }, - setCenter: (state, action) => { + setCenter: (state, action: PayloadAction<{latitude?: number, longitude?: number} | undefined>) => { const { latitude, longitude } = action.payload || {}; if (latitude && longitude) { state.center = { ...state.center, latitude, longitude }; } },
48-55: Consolidate reset functionsThere are two similar functions (
clearDataandreSetMap) that reset state. Consider consolidating them or clarifying their distinct purposes with comments.-clearData: (state) => { - return initialState; -}, -reSetMap: (state) => { - state.center = initialState.center; - state.zoom = initialState.zoom; - state.location = initialState.location; -}, +/** + * Completely resets all map state to initial values + */ +clearData: (state) => { + return initialState; +}, +/** + * Resets only the map position and location while preserving other state + */ +reSetMap: (state) => { + state.center = initialState.center; + state.zoom = initialState.zoom; + state.location = initialState.location; +},
65-67: Add validation for suggestedSitesAdd validation to ensure that the payload for suggestedSites is always an array.
-addSuggestedSites: (state, action) => { - state.suggestedSites = action.payload; +addSuggestedSites: (state, action: PayloadAction<any[]>) => { + state.suggestedSites = Array.isArray(action.payload) ? action.payload : []; },src/netmanager-app/components/NetManagerMap/components/Sidebar/page.tsx (4)
6-15: Improve type definitionsThe component uses 'any' type extensively. Define proper interfaces for your data structures to improve type safety.
+interface CountryData { + country: string; + code: string; + flag: string; + // Add other properties as needed +} +interface SiteDetail { + name: string; + country: string; + // Add other properties as needed +} +interface LocationSuggestion { + mapbox_id: string; + name: string; + place_formatted: string; +} type MapSideBarProps = { - token: string; - sessionToken: string; - countryData: any; - selectedCountry: any; - setSelectedCountry: (country: any) => void; + token: string; + sessionToken: string; + countryData: CountryData[]; + selectedCountry: CountryData | null; + setSelectedCountry: (country: CountryData) => void; reset: () => void; - handleUserClick: (data: any) => void; - siteDetails: any; + handleUserClick: (data: any) => void; // You can define a more specific type for data if available + siteDetails: SiteDetail[]; };
30-76: Extract search logic to a custom hookThe search functionality is quite complex. Consider extracting it to a custom hook to improve readability and reusability.
// Example custom hook function useLocationSearch(token: string, sessionToken: string, reset: () => void) { const [query, setQuery] = useState(""); const [suggestions, setSuggestions] = useState<LocationSuggestion[]>([]); const searchSuggestions = useCallback((value: string) => { setQuery(value); if (value.trim() === "") { setSuggestions([]); reset(); return; } // Rest of your search logic... }, [token, sessionToken, reset]); return { query, suggestions, searchSuggestions }; } // In your component const { query, suggestions, searchSuggestions } = useLocationSearch(token, sessionToken, reset);
102-117: Enhance button accessibilityThe "ALL" button lacks proper accessibility attributes and doesn't indicate its current state.
<Button type="button" className='flex py-[3px] px-[10px] border-none rounded-xl mb-3 text-sm font-medium' + aria-pressed={selectedCountry === null} + onClick={() => { + setSelectedCountry(null); + reset(); + }} > ALL </Button>
121-144: Improve suggestions list accessibility and structureThe suggestions list lacks proper ARIA attributes for accessibility. Also, consider using a proper list structure.
<div className='flex w-full '> { suggestions.length > 0? ( <div id="search-suggestions" - className='w-full' + className='w-full' + role="listbox" + aria-label="Location suggestions" > {suggestions.map((item, index) => ( <div key={index} + role="option" + aria-selected="false" className="bg-white w-full border border-gray-300 mt-1 rounded-md shadow-md max-h-40 overflow-y-auto p-2 hover:bg-gray-200 cursor-pointer" onClick={()=>handleLocationClick(item.mapbox_id)} > <h1 className='text'>{item.name}</h1> <h1 className='text-gray-300' >{item.place_formatted}</h1> </div> ))} </div> ) : ( <div className="text-gray-500 bg-white w-full border border-gray-300 mt-1 rounded-md shadow-md max-h-40 overflow-y-auto p-2 hover:bg-gray-200 cursor-pointer" + role="status" > Type to see Suggestions... </div> )} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/netmanager-app/components/NetManagerMap/components/CountryList.jsx(1 hunks)src/netmanager-app/components/NetManagerMap/components/Sidebar/page.tsx(1 hunks)src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)src/netmanager-app/lib/map/MapSlice.js(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 180-180: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
src/netmanager-app/components/NetManagerMap/components/CountryList.jsx (3)
21-46: Good use of memoization for sorting dataThe component correctly uses useMemo to optimize the sorting operation, preventing unnecessary re-sorting on each render. The handleClick function is also properly optimized with useCallback.
48-62: Well-implemented loading skeletonGood implementation of a loading skeleton for when data isn't available. This provides a better user experience while waiting for data to load.
7-7:✅ Verification successful
Verify Redux import path correctness
The component imports actions from
@/lib/map/MapSlice, but make sure this path aligns with your project structure. Confirm the MapSlice is properly configured at this location.
🏁 Script executed:
#!/bin/bash # Check if MapSlice exists at the specified path find src/netmanager-app/lib/map -name "MapSlice.js" -o -name "MapSlice.ts" -o -name "MapSlice.tsx"Length of output: 133
Redux import path verified.
I confirmed that the file exists at
src/netmanager-app/lib/map/MapSlice.js, so the import using@/lib/map/MapSlicealigns with your project structure. Just double-check that your module alias for@correctly points tosrc/netmanager-appas expected.src/netmanager-app/components/NetManagerMap/page.tsx (1)
338-338: Remove empty border divThis div only contains border styling and has no content or purpose.
-<div className='flex border border-2' > +<div>src/netmanager-app/components/NetManagerMap/components/Sidebar/page.tsx (1)
28-28: Use specific type for suggestionsReplace the 'any[]' type with a specific interface for location suggestions.
-const [suggestions, setSuggestions] = useState<any[]>([]); +const [suggestions, setSuggestions] = useState<LocationSuggestion[]>([]);
| @@ -0,0 +1,107 @@ | |||
| import React, { useCallback, useMemo } from 'react'; | |||
| import { useDispatch } from 'react-redux'; | |||
| import Button from './Button' | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Button import source
The component imports Button from './Button' but doesn't appear to use this imported component - instead it uses the native HTML button element on line 77. Make sure you're importing the correct component or adjust your implementation.
🏁 Script executed:
#!/bin/bash
# Check if there's a Button component in the same directory
find src/netmanager-app/components/NetManagerMap/components -name "Button.jsx" -o -name "Button.tsx"
# Check button usage in the component
rg -A 5 "Button" src/netmanager-app/components/NetManagerMap/components/CountryList.jsxLength of output: 291
Action Required: Resolve Unused Button Import
In src/netmanager-app/components/NetManagerMap/components/CountryList.jsx, the Button component is imported from ./Button (line 3), yet the component isn’t used anywhere in the file—instead, a native HTML <button> element is rendered (line 77). Please either update your implementation to use the imported Button or remove the unused import.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/netmanager-app/core/apis/MapData.ts (4)
16-32: 🛠️ Refactor suggestionImprove error handling and code structure in GetAirQuoData.
The function has several issues including unnecessary block statement, inconsistent error handling, and not propagating errors properly.
+ const API_BASE_URL = 'https://staging-analytics.airqo.net/api/v2'; + export const GetAirQuoData = async (token: string) => { try { - if(token){ - const response = await axios.get(`${AIRQUO_DATA}?token=${token}`); + if (!token) { + throw new Error('Token is missing'); + } + + const response = await axios.get(`${AIRQUO_DATA}`, { + params: { token } + }); const data = response.data; return data; - } - { - console.log("Error Token Is Missing") - return null; - } } catch (error) { - console.error('Error fetching GeoJSON:', error); + console.error('Error in GetAirQuoData:', error); + throw error; } }🧰 Tools
🪛 Biome (1.9.4)
[error] 23-26: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
34-58: 🛠️ Refactor suggestionEnhance type safety and fix error handling in FetchSuggestions.
The function has issues with return types, error handling, and console logs. It also doesn't handle the case when latitude or longitude is undefined.
+ interface MapboxSuggestion { + suggestions: any[]; + } + - export const FetchSuggestions=async(value:string,access_token:string,sessionToken:string,latitude?: number, longitude?: number):Promise<any[] | void>=>{ + export const FetchSuggestions = async( + value: string, + access_token: string, + sessionToken: string, + latitude?: number, + longitude?: number + ): Promise<any[]> => { if (!access_token) { - console.log("Missing Access Token"); - return; + throw new Error('Missing Access Token'); } if (latitude !== undefined && longitude !== undefined) { - try{ - const proximityParam = `${longitude},${latitude}`; - const response = await axios.get(`${MAP_BOX_SUGGESTIONS}?q=${value.toLowerCase()}&access_token=${access_token}&proximity=${proximityParam}&session_token=${sessionToken}`) - const data = await response.data; - - console.log("data :",data.suggestions) - if (data.suggestions) { - return data.suggestions; - }else{ - console.log("Error fetching suggestions"); - } - - } - catch(error){ - // console.error("Error fetching suggestions:", error); - } + try { + const proximityParam = `${longitude},${latitude}`; + const response = await axios.get<MapboxSuggestion>(`${MAP_BOX_SUGGESTIONS}`, { + params: { + q: value.toLowerCase(), + access_token, + proximity: proximityParam, + session_token: sessionToken + } + }); + return response.data.suggestions || []; + } catch(error) { + console.error("Error fetching suggestions:", error); + throw error; + } } + return []; }🧰 Tools
🪛 Biome (1.9.4)
[error] 34-34: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
59-72: 🛠️ Refactor suggestionImplement proper error handling in UserClick function.
The UserClick function lacks try/catch error handling and doesn't handle API errors properly. It also uses template literals inside parentheses unnecessarily.
- export const UserClick = async(access_token:string,sessionToken:string,locationid: string,)=>{ + export const UserClick = async ( + access_token: string, + sessionToken: string, + locationid: string + ): Promise<any> => { if (!access_token) { - console.log("Missing Access Token"); - return; + throw new Error('Missing Access Token'); } + try { + const response = await axios.get(`${MAP_BOX_SEARCH}/${locationid}`, { + params: { + access_token, + session_token: sessionToken + } + }); - const response = await axios.get((`${MAP_BOX_SEARCH}/${locationid}?&access_token=${access_token}&session_token=${sessionToken}`)) const data = response.data; - if(data){ - return data; - } + return data; + } catch (error) { + console.error('Error in UserClick:', error); + throw error; + } }
113-129: 🛠️ Refactor suggestionRemove duplicated UserGroups function.
The UserGroups function is identical to GetAirQuoData, creating unnecessary code duplication. Consider refactoring to eliminate this redundancy.
-export const UserGroups = async (token: string) =>{ - try { - if(token){ - const response = await axios.get(`${AIRQUO_DATA}?token=${token}`); - const data = response.data; - return data; - } - { - console.log("Error Token Is Missing") - return null; - } - - } - catch (error) { - // console.error('Error fetching GeoJSON:', error); - } -} +// Use GetAirQuoData instead of duplicating code +export const UserGroups = GetAirQuoData;🧰 Tools
🪛 Biome (1.9.4)
[error] 120-123: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
🧹 Nitpick comments (2)
src/netmanager-app/core/apis/MapData.ts (1)
1-14: Consider organizing imports and constants in a more structured manner.The file mixes direct imports, environment variables, and image imports without clear separation. Consider organizing them into logical groups (external libraries, environment variables, constants, image imports) to improve readability and maintainability.
import axios from "axios"; + +// Environment variables export const token = process.env.NEXT_PUBLIC_MAP_API_TOKEN export const AirQoToken = process.env.NEXT_PUBLIC_AIRQO_DATA_TOKEN + +// API URLs import {AIRQUO_DATA,MAP_BOX_SUGGESTIONS,MAP_BOX_SEARCH} from "../urls" + +// Map visualization assets import Node from '@/public/images/map/Node.webp'; import Emoji from '@/public/images/map/Emoji.webp'; import Heatmap from '@/public/images/map/Heatmap.webp'; import Node_Number from '@/public/images/map/Node_number.webp'; +// Map style assets import DarkMode from '@/public/images/map/dark.webp'; import LightMode from '@/public/images/map/light.webp'; import SatelliteMode from '@/public/images/map/satellite.webp'; import StreetsMode from '@/public/images/map/street.webp';src/netmanager-app/core/urls.tsx (1)
14-16: Consider adding documentation comments for new URL constants.These new URL constants would benefit from JSDoc comments explaining their purpose and expected usage, especially since they're likely to be used across multiple components.
+/** + * URL for fetching AirQuo device readings map data + */ export const AIRQUO_DATA = `${DEVICES_MGT_URL}/readings/map`; + +/** + * Mapbox API endpoint for location search suggestions + */ export const MAP_BOX_SUGGESTIONS = `${MAP_BOX_BASE_API_URL}/search/searchbox/v1/suggest`; + +/** + * Mapbox API endpoint for retrieving detailed location information + */ export const MAP_BOX_SEARCH = `${MAP_BOX_BASE_API_URL}/search/searchbox/v1/retrieve`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/netmanager-app/core/apis/MapData.ts(1 hunks)src/netmanager-app/core/urls.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/core/apis/MapData.ts
[error] 34-34: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 23-26: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
[error] 120-123: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (3)
src/netmanager-app/core/apis/MapData.ts (1)
73-111: Well-structured map configuration objects.The
mapStylesandmapDetailsarrays are well-structured and provide clear configuration for different map styles and visualization options.src/netmanager-app/core/urls.tsx (2)
5-7: Good use of environment variables for API URL configuration.Using environment variables for API URLs is a good practice as it allows for different configurations in different environments without code changes.
1-16:❓ Verification inconclusive
Verify that environment variables are properly set in all environments.
Ensure that the required environment variables (
NEXT_PUBLIC_MAPBOX_URL) are properly set in all deployment environments to avoid runtime errors.
🏁 Script executed:
#!/bin/bash # Check if the necessary environment variables are defined in any .env files echo "Checking for NEXT_PUBLIC_MAPBOX_URL in environment files..." if grep -q "NEXT_PUBLIC_MAPBOX_URL" .env* 2>/dev/null; then echo "✓ Found NEXT_PUBLIC_MAPBOX_URL in environment files" else echo "⚠️ NEXT_PUBLIC_MAPBOX_URL not found in any environment files" fi echo "Checking for NEXT_PUBLIC_API_URL in environment files..." if grep -q "NEXT_PUBLIC_API_URL" .env* 2>/dev/null; then echo "✓ Found NEXT_PUBLIC_API_URL in environment files" else echo "⚠️ NEXT_PUBLIC_API_URL not found in any environment files" fi # Look for usage of these environment variables in the codebase echo "Checking for usage of NEXT_PUBLIC_MAPBOX_URL in codebase..." rg "NEXT_PUBLIC_MAPBOX_URL" --type ts --type tsx echo "Checking if there are any direct references to mapbox URLs that should use the constant instead..." rg "api.mapbox.com" --type ts --type tsxLength of output: 1068
Action Required: Verify Environment Variable Configuration
The code in
src/netmanager-app/core/urls.tsxcorrectly accesses the environment variables usingprocess.env.NEXT_PUBLIC_API_URLandprocess.env.NEXT_PUBLIC_MAPBOX_URL. However, our initial check did not locate definitions for these variables in any local.envfiles. This might be expected if the values are injected via your deployment pipeline or hosting configuration, but please confirm that both variables are defined in all deployment environments to avoid any runtime issues.
- Verify that
NEXT_PUBLIC_API_URLandNEXT_PUBLIC_MAPBOX_URLare correctly configured in your deployment settings (CI/CD, hosting service, etc.).- Ensure these variables are consistently available so that the API endpoints (including the MAPBOX constants) work as intended.
- Review your documentation or environment configuration scripts to clarify where and how these values are set.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/netmanager-app/components/NetManagerMap/page.tsx (1)
267-267:⚠️ Potential issueEnsure consistent icon naming for 'Hazardous'.
Using
'Hazardous'in the match statement might not align with your actual image key ('hazardous'). This discrepancy can cause the map layer to fail for hazard-level data. Confirm the correct key and update accordingly.- 'Hazardous','Hazardous', + 'Hazardous','hazardous',
🧹 Nitpick comments (5)
src/netmanager-app/core/hooks/useMapData.ts (2)
8-8: Rename "Error" state variable to avoid shadowing.Declaring
Errorshadows the globalErrorobject, which can lead to confusion or unexpected behavior in your codebase. Consider using a more descriptive name likeerrorStateorerrorMessage.- const [Error, setError] = useState<string | null>(null); + const [errorMessage, setErrorMessage] = useState<string | null>(null);🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: 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)
15-15: Limit console logging in production.Frequent console logs may clutter logs in production. Consider removing them or using a more robust logging library.
- console.log("Fetched Data:", res); + // console.log("Fetched Data:", res); // Remove or use a logging librarysrc/netmanager-app/components/NetManagerMap/page.tsx (2)
45-45: Rename "Error" variable to prevent global shadowing.The destructured object includes an
Errorproperty that shadows the globalError, potentially leading to confusion. Rename it to something likemapErrorordataError.- const {MapData, Error, Loading} = useMapData(); + const { MapData, Error: mapError, Loading } = useMapData();🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: 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)
176-176: Use optional chaining for cleaner null checks.Replacing
if (feature && feature.properties)with optional chaining can simplify the code. However, ensure that subsequent references properly handleundefined.- if (feature && feature.properties) { + if (feature?.properties) {🧰 Tools
🪛 Biome (1.9.4)
[error] 176-176: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/netmanager-app/core/apis/MapData.ts (1)
25-25: Use a more explicit return type instead ofvoidin the union.Returning
voidin a union can be confusing. If you need a fallback return when suggestions are not found, consider returning an empty array orundefined.-export const FetchSuggestions=async(value:string,access_token:string,sessionToken:string,latitude?: number, longitude?: number):Promise<any[] | void>=>{ +export const FetchSuggestions=async(value:string,access_token:string,sessionToken:string,latitude?: number, longitude?: number):Promise<any[] | undefined>=>{🧰 Tools
🪛 Biome (1.9.4)
[error] 25-25: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/netmanager-app/components/NetManagerMap/data/constants.js(1 hunks)src/netmanager-app/components/NetManagerMap/page.tsx(1 hunks)src/netmanager-app/core/apis/MapData.ts(1 hunks)src/netmanager-app/core/hooks/useMapData.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/netmanager-app/components/NetManagerMap/data/constants.js
🧰 Additional context used
🪛 Biome (1.9.4)
src/netmanager-app/components/NetManagerMap/page.tsx
[error] 45-45: 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)
[error] 176-176: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/netmanager-app/core/hooks/useMapData.ts
[error] 8-8: 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)
src/netmanager-app/core/apis/MapData.ts
[error] 25-25: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 14-17: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
[error] 72-75: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (2)
src/netmanager-app/components/NetManagerMap/page.tsx (2)
315-331: Duplicate onClick handlers and titles.This issue has been mentioned before. Both the Refresh and Share buttons have the same onClick handler (
setIsOpen(true)) and the same title'Refresh Map'. Update one to its intended action and label.
334-334: Remove or repurpose empty bordered container.The bordered
<div>seems to have no functional content. It was flagged in a previous review. Consider removing it or adding relevant content.
| fetchData(); | ||
| }, [MapData]); |
There was a problem hiding this comment.
Avoid potential infinite data fetch loop.
Watching MapData in the dependency array re-triggers the effect whenever MapData updates, causing another fetch and potentially creating an infinite loop. If the goal is to fetch data only on component mount, remove MapData from the dependency array.
- }, [MapData]);
+ }, []); 📝 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.
| fetchData(); | |
| }, [MapData]); | |
| fetchData(); | |
| }, []); |
| { | ||
| console.log("Error Token Is Missing") | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant block statement.
This block can be converted into a straightforward else-clause, improving readability.
if(token){
const response = await axios.get(`${AIRQUO_DATA}?token=${token}`);
const data = response.data;
return data;
- }
- {
- console.log("Error Token Is Missing")
- return null;
- }
+ } else {
+ console.log("Error Token Is Missing");
+ return null;
+ }📝 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.
| { | |
| console.log("Error Token Is Missing") | |
| return null; | |
| } | |
| if(token){ | |
| const response = await axios.get(`${AIRQUO_DATA}?token=${token}`); | |
| const data = response.data; | |
| return data; | |
| } else { | |
| console.log("Error Token Is Missing"); | |
| return null; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 72-75: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
| { | ||
| console.log("Error Airquo Token Is Missing") | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant block statement.
The block containing the console.log("Error Airquo Token Is Missing") call can be combined into an else-clause or turned into an early return, making the code clearer and more concise.
if(token){
const response = await axios.get(`${AIRQUO_DATA}?token=${AirQoToken}`);
const data = response.data;
return data;
- }
- {
- console.log("Error Airquo Token Is Missing")
- return null;
- }
+ } else {
+ console.log("Error Airquo Token Is Missing");
+ return null;
+ }📝 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.
| { | |
| console.log("Error Airquo Token Is Missing") | |
| return null; | |
| } | |
| if(token){ | |
| const response = await axios.get(`${AIRQUO_DATA}?token=${AirQoToken}`); | |
| const data = response.data; | |
| return data; | |
| } else { | |
| console.log("Error Airquo Token Is Missing"); | |
| return null; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 14-17: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
New Features
UI Enhancements
Chores
Data Updates