feat: integrate Mapbox for enhanced mapping functionality#4
Conversation
- Added HomeMap component to display a Mapbox map with geolocation features. - Updated Home page to render the HomeMap component. - Introduced PrivateLayout component for consistent layout structure. - Updated TypeScript configuration to include additional type definitions. - Added Mapbox GL and related dependencies to package.json and package-lock.json. - Created global.d.ts for Mapbox CSS module declaration.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughHome now renders a new HomeMap component with Mapbox integration; a PrivateLayout wrapper was added; Mapbox dependency and CSS typings were introduced; ProfileCard now uses Next.js Image; Footer icon import removed; tokenVerify middleware return type and return behavior were made explicit. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/components/profile/ProfileCard.tsx (1)
28-33:⚠️ Potential issue | 🔴 CriticalNext.js
Imagecomponent is missing requiredwidth/heightorfillprop — this will error at runtime.The Next.js
Imagecomponent requires either explicitwidthandheightprops, or thefillprop (with a positioned parent). Without one of these, Next.js throws a runtime error at runtime. Yournext.config.tsalready hasremotePatternsconfigured forlh3.googleusercontent.com, so external image loading is handled.🐛 Proposed fix
- <Image - src={user.picture} - alt={user.name} - className="h-28 w-28 rounded-2xl border-4 border-white object-cover shadow-lg" - referrerPolicy="no-referrer" - /> + <Image + src={user.picture} + alt={user.name} + width={112} + height={112} + className="h-28 w-28 rounded-2xl border-4 border-white object-cover shadow-lg" + referrerPolicy="no-referrer" + />Alternatively, wrap the Image in a relatively positioned container and use
fillprop.
🤖 Fix all issues with AI agents
In `@client/components/home/HomeMap.tsx`:
- Around line 40-46: The Mapbox map is being initialized with
attributionControl: false (in the mapboxgl.Map call that creates the map
variable), which hides required attribution; change the initialization to either
set attributionControl: true or add a Mapbox attribution control explicitly
(e.g., add map.addControl(new mapboxgl.AttributionControl(...)) or a custom
control that includes the required Mapbox attribution text) so the map always
displays the Mapbox attribution in compliance with Terms of Service.
- Around line 78-104: The geolocation success/error callbacks from
navigator.geolocation.getCurrentPosition may run after the component unmounts
and attempt to call map.flyTo or add a mapboxgl.Marker to a destroyed map; add
an isCancelled (or isMounted) boolean captured by the effect to early-return
from both success and error callbacks, only calling map.flyTo, creating/adding
the marker, or calling setGeoError when isCancelled is false, and in the effect
cleanup set isCancelled = true and also remove the marker if it was created
(store marker in a ref or outer variable so cleanup can call marker.remove());
update uses of getCurrentPosition, map.flyTo, marker, and setGeoError
accordingly.
In `@server/src/middleware/tokenVerify.ts`:
- Around line 20-22: In tokenVerify's catch block remove the raw error from the
JSON response to avoid leaking internals: keep logging the error server-side
(e.g., console.error or processLogger in tokenVerify) but return only a generic
message/status like { errorMsg: "Internal server error" } and appropriate status
500; ensure the change is applied in the tokenVerify middleware catch branch
where res.status(500).json(...) is called so only non-sensitive information is
sent to clients.
🧹 Nitpick comments (2)
client/components/home/HomeMap.tsx (1)
131-309: Large component — consider extracting the static onboarding panel.The JSX is ~180 lines with two distinct UI regions: a search card (Lines 157–228) and an onboarding panel (Lines 231–305). Extracting the onboarding panel into its own component would improve readability and make each piece independently testable.
client/tsconfig.json (1)
31-32: The**/*.d.tspattern is redundant.The existing
**/*.tsglob already matches.d.tsfiles since they end with.ts. Adding**/*.d.tsis harmless but unnecessary.
| } catch (error) { | ||
| console.log("Error in tokenVerify:", error); | ||
| return res.status(500).json({ errorMsg: "Internal server error", error }); |
There was a problem hiding this comment.
Avoid leaking internal error details in the response.
Line 22 serializes the raw error object into the JSON response. In production, this could expose stack traces, internal paths, or library internals to the client.
Proposed fix
} catch (error) {
console.log("Error in tokenVerify:", error);
- return res.status(500).json({ errorMsg: "Internal server error", error });
+ return res.status(500).json({ errorMsg: "Internal server 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.
| } catch (error) { | |
| console.log("Error in tokenVerify:", error); | |
| return res.status(500).json({ errorMsg: "Internal server error", error }); | |
| } catch (error) { | |
| console.log("Error in tokenVerify:", error); | |
| return res.status(500).json({ errorMsg: "Internal server error" }); | |
| } |
🤖 Prompt for AI Agents
In `@server/src/middleware/tokenVerify.ts` around lines 20 - 22, In tokenVerify's
catch block remove the raw error from the JSON response to avoid leaking
internals: keep logging the error server-side (e.g., console.error or
processLogger in tokenVerify) but return only a generic message/status like {
errorMsg: "Internal server error" } and appropriate status 500; ensure the
change is applied in the tokenVerify middleware catch branch where
res.status(500).json(...) is called so only non-sensitive information is sent to
clients.
2 participants
Summary by CodeRabbit