Skip to content

64 create map component in react#67

Merged
YoungMame merged 4 commits into
mainfrom
64-create-map-component-in-react
Dec 31, 2025
Merged

64 create map component in react#67
YoungMame merged 4 commits into
mainfrom
64-create-map-component-in-react

Conversation

@YoungMame
Copy link
Copy Markdown
Owner

/map page

UsersMap component using

Fixing seeding for proper UX

@YoungMame YoungMame self-assigned this Dec 31, 2025
Copilot AI review requested due to automatic review settings December 31, 2025 16:58
@YoungMame YoungMame added the enhancement New feature or request label Dec 31, 2025
@YoungMame YoungMame linked an issue Dec 31, 2025 that may be closed by this pull request
6 tasks
@YoungMame YoungMame merged commit be64abe into main Dec 31, 2025
5 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a map feature for displaying users on an interactive map using MapLibre GL. The implementation includes a new React component for rendering user locations and clusters, integrates with a backend API to fetch location data, and updates the seeding logic to generate more realistic test data centered around Paris.

  • Added interactive map component with user markers and clustering support
  • Integrated MapLibre GL and react-map-gl libraries for map rendering
  • Fixed field naming inconsistency (profilePictureUrl → profilePicture) between frontend and backend
  • Enhanced database queries with distance-based sorting and result limiting
  • Updated seed data to use Paris-centered coordinates for better UX testing

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 17 comments.

Show a summary per file
File Description
nextjs/matcha/src/types/api/usersMap.ts Added TypeScript interfaces for map user data, clusters, and API responses
nextjs/matcha/src/lib/api/map.ts Created API client for fetching nearby users from the map endpoint
nextjs/matcha/src/components/map/UsersMap.tsx Implemented main map component with user markers, clustering, and viewport management
nextjs/matcha/src/app/(logged)/map/page.tsx Added map page that fetches user profile and renders the UsersMap component
nextjs/matcha/src/app/(logged)/me/page.tsx Removed unused refetch variable from useMyProfile hook destructuring
fastify/assets/srcs/models/User/index.ts Fixed field naming, added distance sorting, LIMIT clause, and clustering logic improvements
fastify/assets/seed/seed.ts Added random name generation and Paris-centered coordinate generation for seed data
fastify/assets/seed/dataset.json Updated all location coordinates to be centered around Paris
nextjs/matcha/package.json Added maplibre-gl and react-map-gl dependencies
nextjs/matcha/pnpm-lock.yaml Updated lock file with new map-related dependencies
Files not reviewed (1)
  • nextjs/matcha/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

import { useMyProfile } from "@/hooks/useProfile";
import UsersMap from "@/components/map/UsersMap";

export default function MyProfilePage() {
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name is 'MyProfilePage' but this is actually the map page, not a profile page. This should be renamed to something like 'MapPage' to accurately reflect its purpose.

Suggested change
export default function MyProfilePage() {
export default function MapPage() {

Copilot uses AI. Check for mistakes.
setFirstFetchDone(true);
queryClient.invalidateQueries({ queryKey: ['map'] });
}
}, [center, zoom, lastSignificantMove, radius]);
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing dependency array includes 'queryClient' and 'firstFetchDone'. The useEffect depends on 'queryClient' (used on line 83) and 'firstFetchDone' (used on line 78), which should be included in the dependency array to avoid stale closures.

Suggested change
}, [center, zoom, lastSignificantMove, radius]);
}, [center, zoom, lastSignificantMove, radius, firstFetchDone, queryClient]);

Copilot uses AI. Check for mistakes.
parseTags(u.tags),
u.fame_rate,
u.isProfileCompleted ,
u.isProfileCompleted ,
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation with tabs and spaces mixed. Line 96 uses tabs while other lines use spaces, which violates consistent code formatting practices.

Suggested change
u.isProfileCompleted ,
u.isProfileCompleted,

Copilot uses AI. Check for mistakes.
AND 6371 * acos(least(1, greatest(-1, cos(radians(locations.latitude)) * cos(radians($1)) * cos(radians($2) - radians(locations.longitude)) + sin(radians(locations.latitude)) * sin(radians($1))))) < $3`,
AND 6371 * acos(least(1, greatest(-1, cos(radians(locations.latitude)) * cos(radians($1)) * cos(radians($2) - radians(locations.longitude)) + sin(radians(locations.latitude)) * sin(radians($1))))) < $3
ORDER BY distance ASC
LIMIT 100`,
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number 100 for LIMIT should be defined as a named constant. Hard-coding the limit makes it difficult to maintain and unclear why 100 was chosen. Consider defining it as a constant like MAX_MAP_USERS_PER_QUERY at the top of the class.

Copilot uses AI. Check for mistakes.
@@ -127,31 +127,50 @@ export default class UserModel {

getUsersFromLocation = async (lat: number, lgn: number, radius: number): Promise<MapUser[]> => {
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent parameter name. The parameter is named 'lgn' (line 128) but 'lng' would be more standard and consistent with the frontend code which uses 'lng'. This inconsistency could lead to confusion.

Copilot uses AI. Check for mistakes.
[lat, lgn, radius]
);
let result;
if (level == 1 && radius < 300) {
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number 300 for radius threshold should be defined as a named constant. Hard-coding this radius value makes it unclear what it represents (presumably kilometers) and makes it harder to tune this threshold later.

Copilot uses AI. Check for mistakes.
const onZoomEnd = (evt: { viewState: { zoom: number } }) => {
const zoom = evt.viewState.zoom;
setZoom(zoom);
setLevel(zoom < LEVEL1_ZOOM ? zoom < LEVEL2_ZOOM ? 2 : 1 : 0 );
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The level logic is reversed. When zoom is 13 (initial value), zoom < LEVEL1_ZOOM (5) is false, so level will be 0. When zooming out, the zoom value decreases, but the ternary operator sets level=2 when zoom < LEVEL2_ZOOM (3) and level=1 when zoom >= LEVEL2_ZOOM but < LEVEL1_ZOOM. This seems inverted - typically level should increase as you zoom out (lower zoom values).

Copilot uses AI. Check for mistakes.
latitude={user.latitude}
onClick={() => router.push(`/profile/${user.id}`)}
>
<Image className='rounded-full w-24 h-24' unoptimized src={user.profilePicture || '/default-profile.svg'} alt={user.firstName} width={24} height={24} />
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing alt text fallback. When user.firstName is undefined or empty, the alt attribute will be empty, which is poor for accessibility. Consider using a default value like 'User profile picture'.

Suggested change
<Image className='rounded-full w-24 h-24' unoptimized src={user.profilePicture || '/default-profile.svg'} alt={user.firstName} width={24} height={24} />
<Image className='rounded-full w-24 h-24' unoptimized src={user.profilePicture || '/default-profile.svg'} alt={user.firstName || 'User profile picture'} width={24} height={24} />

Copilot uses AI. Check for mistakes.
Comment on lines +138 to +140
{clusters.map((cluster, index) => (
<Marker
key={`cluster-${index}`}
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing key or unique identifier for clusters. Using array index as a key for clusters (key={cluster-${index}}) can cause issues with React reconciliation if the cluster order changes. If clusters don't have unique IDs, consider using a combination of latitude and longitude as the key.

Suggested change
{clusters.map((cluster, index) => (
<Marker
key={`cluster-${index}`}
{clusters.map((cluster) => (
<Marker
key={`cluster-${cluster.latitude}-${cluster.longitude}`}

Copilot uses AI. Check for mistakes.
}, [center, zoom, lastSignificantMove, radius]);


const { isPending, error, data, isFetching } = useQuery({
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable isPending.

Suggested change
const { isPending, error, data, isFetching } = useQuery({
const { error, data, isFetching } = useQuery({

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create Map component in react

2 participants