Skip to content

feat: add interactive map search with property clustering and search area - #153

Open
Tanya-garg10 wants to merge 6 commits into
akshay0611:mainfrom
Tanya-garg10:map-updates
Open

feat: add interactive map search with property clustering and search area#153
Tanya-garg10 wants to merge 6 commits into
akshay0611:mainfrom
Tanya-garg10:map-updates

Conversation

@Tanya-garg10

@Tanya-garg10 Tanya-garg10 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Description

This PR introduces an Interactive Map Search feature to improve the property discovery experience by allowing users to explore listings geographically.

Features Added

  • 🗺️ Interactive map integration for property discovery.
  • 📍 Property markers displaying listing locations.
  • 🏠 Property preview card on marker selection.
  • 🔍 "Search This Area" functionality to update listings based on the current map viewport.
  • 📌 Marker clustering for improved usability in dense areas.
  • 📱 Responsive map experience for both desktop and mobile devices.
  • ♻️ Reusable map components and utilities for future enhancements.

This feature enhances location-based property exploration and aligns with the project's roadmap for a dedicated map view.

Closes #121

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Dev Server Check (pnpm dev)
  • Lint Check (pnpm lint)
  • Build Check (pnpm build)

Screenshots/Video

2026-07-14.14-19-17.1.1.1.mp4

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@vercel

vercel Bot commented Jul 14, 2026

Copy link
Copy Markdown

@Tanya-garg10 is attempting to deploy a commit to the Akshay Kumar's projects Team on Vercel.

A member of the Team first needs to authorize it.

@akshay0611 akshay0611 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review: feat: add interactive map search with property clustering and search area

This PR introduces an AI-powered natural language search hero, an interactive Leaflet map with clustering, an NLP query parser, and tests. The scope is ambitious and the components are thoughtfully built — this is some impressive work.

What looks great ✨

  • The nlp-parser.ts is surprisingly thorough — it handles prices, ratings, locations (with both known city names and fallback preposition-based detection), amenities, types, guests, and bedrooms, all with regex. The amenity synonym mapping is clean and easy to extend.
  • The AISearchHero UX is polished — the gradient background, suggestion chips, editable filter badges with inline editing, and the "Reset Search" flow all feel premium.
  • The MapComponent clustering algorithm (pixel-based proximity) is a smart lightweight alternative to bringing in a full cluster library. The dark/light tile switching via useTheme is a thoughtful touch.
  • The MapPlaceholder loading state is beautifully designed with the animated pulse markers and grid lines — great attention to detail.
  • You wrote tests for the NLP parser covering multiple query patterns, including edge cases. Solid.

Blockers

  • package.jsonleaflet and @types/leaflet need to be added as dependencies. Without them, npm install won't install Leaflet and the build will fail with a module resolution error.

  • components/MapComponent.tsx:93property.pricePerNight * 100 shows incorrect prices. In the codebase, pricePerNight is already a plain integer in INR (e.g., 120, 450 — see lib/dummy-data.ts:159-374). Multiplying by 100 would display ₹45,000 instead of ₹450 on the map markers. This should be just property.pricePerNight (or Math.round(property.pricePerNight)).

Optional suggestions (feel free to ignore)

  • The static import L from 'leaflet' at the top of MapComponent can cause SSR/build failures in Next.js since Leaflet accesses window at module level. You might want to use next/dynamic with ssr: false in the parent page that renders the map, or import Leaflet inside a useEffect / dynamic import.

Verdict

REQUEST_CHANGES

These are two concrete fixes — add the Leaflet packages to package.json and remove the * 100 on the price display. The components themselves are really well-crafted and the overall direction is excellent.

Remove erroneous * 100 multiplier from pricePerNight calculations.
pricePerNight is already a plain INR integer, so multiplying inflated
prices 100x (e.g. Rs.450 was showing as Rs.45,000).

Fixes both the price marker label and the popup card price display.
@Tanya-garg10

Copy link
Copy Markdown
Contributor Author

Thank you for the detailed review and feedback!

I've addressed the requested changes:
✅ Added leaflet and @types/leaflet to the project dependencies.
✅ Fixed the map marker price display by removing the unnecessary * 100 multiplication.

I also reviewed the SSR suggestion for Leaflet and will keep it in mind for future improvements.

The requested changes have been pushed. Please take another look @akshay0611

@akshay0611
akshay0611 self-requested a review July 22, 2026 03:17

@akshay0611 akshay0611 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review: feat: add interactive map search with property clustering and search area

This is a well-scoped PR that delivers three solid components — an AI-powered natural language search, an interactive map with clustering, and an NLP query parser with tests. The contributor already addressed the price display issue from the previous review. The overall code quality is strong.

What looks great ✨

  • The nlp-parser.ts is remarkably thorough — it handles pricing, location (known cities + fallback regex), ratings, amenity synonyms, property types, guests, and bedrooms. The synonym mapping structure makes it easy to extend.
  • AISearchHero UX is polished: gradient backgrounds, suggestion chips, inline-editable filter badges, and the reset flow all feel premium. The attention to micro-interactions (hover states, transitions) shows real care.
  • The custom pixel-based clustering algorithm in MapComponent is a smart choice — avoids pulling in a heavy cluster library while still handling dense marker areas cleanly.
  • The MapPlaceholder loading skeleton with animated pulse markers and decorative grid lines is a delightful touch.
  • Tests cover the main query patterns and edge cases. Solid coverage for a first pass.

Blockers

  • package.jsonleaflet and @types/leaflet must be added as dependencies. MapComponent.tsx does import L from 'leaflet' and import 'leaflet/dist/leaflet.css', but the packages aren't listed in package.json. This will cause a build failure in CI and for anyone checking out the repo. Fix: run pnpm add leaflet @types/leaflet and commit the updated package.json + pnpm-lock.yaml.

Optional suggestions (feel free to ignore)

  • The static import L from 'leaflet' at the top of MapComponent may cause SSR issues since Leaflet accesses window at module level. You might want to use next/dynamic with ssr: false in the parent page, or import Leaflet inside a useEffect. The existing typeof window === 'undefined' guards help, but the import itself could still cause a build-time failure on the server.

Verdict

REQUEST_CHANGES

Just one remaining issue — adding the Leaflet dependency. The price fix you already pushed was spot-on. You're very close — the code is clean, well-structured, and clearly tested locally. Ship that dependency update and this will be ready to land.

@Tanya-garg10

Copy link
Copy Markdown
Contributor Author

Hi!

I've addressed the remaining blocker:
✅ Added leaflet and @types/leaflet to package.json.
✅ Updated pnpm-lock.yaml with the new dependencies.

The latest changes have been pushed. Please review when you get a chance @akshay0611

@akshay0611 akshay0611 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review: feat: add interactive map search with property clustering and search area

This PR introduces an interactive Leaflet map with clustering, an NLP search hero, and a completely revamped properties page with a split-pane map/list layout. The scope is ambitious and the implementation is polished — impressive work, Tanya.

What looks great ✨

  • The custom pixel-based clustering algorithm in MapComponent is a smart lightweight alternative to pulling in a cluster library. It handles dense areas cleanly and the cluster zoom-on-click interaction is intuitive.
  • The dark/light tile switching via useTheme in the map shows real attention to detail.
  • AISearchHero UX is genuinely premium — the gradient hero section, suggestion chips, inline-editable filter badges, and the "Reset Search" flow all feel like a production feature.
  • The nlp-parser.ts is thorough: prices, locations (with fallback preposition regex), ratings, amenity synonyms, property types, guests, and bedrooms. Good test coverage too.
  • The split-pane layout on the properties page with hover/selection sync between the list and map is smooth and well-executed.
  • You already fixed the previous review feedback — the Leaflet dependency is in package.json, MapComponent uses next/dynamic with ssr: false, and the map markers show correct prices. Solid response to feedback.

Blockers

  • app/properties/page.tsx:222 — The mobile preview card overlay still has selectedProperty.pricePerNight * 100. Prices like ₹310 would show as ₹31,000. The map markers were fixed correctly (Math.round(property.pricePerNight).toLocaleString()), but this second instance was missed. Remove the * 100 to match the same pattern used elsewhere.

Optional suggestions (feel free to ignore)

  • Consider using next/image instead of <img> in the mobile card overlay (app/properties/page.tsx:210). The existing codebase standard is next/image, so this would avoid layout shift and stay consistent.

Verdict

REQUEST_CHANGES

You're very close — the codebase-wide structure is clean, the component design is thoughtful, and you've already addressed the prior review feedback well. This is just one lingering * 100 that slipped through. Fix that and this is ready to land.

@Tanya-garg10

Copy link
Copy Markdown
Contributor Author

I've addressed the requested change:

✅ Removed the unnecessary * 100 from the mobile preview card overlay in app/properties/page.tsx, so the displayed price is now consistent with the map markers and the rest of the application.

The latest changes have been pushed. Please take another look @akshay0611

@akshay0611
akshay0611 self-requested a review August 4, 2026 03:35

@akshay0611 akshay0611 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review: feat: add interactive map search with property clustering and search area

This PR delivers a genuinely ambitious feature — a responsive Leaflet map with custom clustering, viewport-aware filtering, marker/list synchronization, and a polished Airbnb-style split layout on the properties page. The scope is large and the craftsmanship shows.

What looks great ✨

  • The custom pixel-based clustering algorithm in MapComponent is a smart, lightweight alternative to pulling in a heavy cluster library, and zoom-on-cluster-click is an intuitive interaction.
  • Dark/light basemap switching via useTheme shows real attention to detail.
  • The "Search This Area" / "Reset Map Filter" flow with the isInternalMapChange guard is thoughtfully designed to avoid recursive viewport resets.
  • The MapPlaceholder loading skeleton — animated pulse markers, decorative grid lines, and the river/parks mimics — is a delightful touch that keeps the empty state premium.
  • You resolved the earlier * 100 price bug consistently across the map markers, popups, and the mobile preview card. Clean follow-through on feedback.

Blockers

  • components/PropertyCard.tsx:95-98 — The card's <Link> is now href="#" with e.preventDefault(), and it only navigates when onViewMap is passed. But onViewMap is never passed to any PropertyCard in the codebase, so clicking a property card on the home page (and anywhere the card is used outside the map) now does nothing — it no longer routes to /property/[id]. This breaks existing navigation.
    Suggested fix — keep the default detail navigation and only intercept when the map handler is actually present:
    <Link
      href={`/property/${property.id}`}
      onClick={onViewMap ? (e) => { e.preventDefault(); onViewMap(property); } : undefined}
    >

Optional suggestions (feel free to ignore)

  • On the properties page, selectedProperty is looked up from the full propertyList rather than filtered, so the mobile preview card could show a listing that a filter has since removed. Consider finding it in filtered/sorted.
  • The map controls and card prices all use ₹ while FilterSidebar now labels price in $ — a small currency-label pass would keep things consistent.

Verdict

REQUEST_CHANGES

You're genuinely close — the map itself is polished and the state management around viewport/reset is thoughtful. This is really one fix: keep the property card navigating to the detail page by default so the existing listing flow doesn't regress. Fix that link and this is ready to land.

@Tanya-garg10

Copy link
Copy Markdown
Contributor Author

I've addressed the requested change and raised a new PR with the fix.

✅ Restored the default /property/[id] navigation for PropertyCard.
✅ The map-specific onViewMap handler now only intercepts the click when it is actually provided.
✅ Existing property card navigation remains unchanged for normal usage.

The latest changes have been pushed and the PR is ready for review. Please take another look @akshay0611

@akshay0611

Copy link
Copy Markdown
Owner

I've addressed the requested change and raised a new PR with the fix.

✅ Restored the default /property/[id] navigation for PropertyCard. ✅ The map-specific onViewMap handler now only intercepts the click when it is actually provided. ✅ Existing property card navigation remains unchanged for normal usage.

The latest changes have been pushed and the PR is ready for review. Please take another look @akshay0611

@Tanya-garg10 Kindly keep the fix in this PR only, it helps to keep the scope clean!

@Tanya-garg10

Tanya-garg10 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Okay!!
The latest changes have been pushed and. Please take another look @akshay0611

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Interactive Map Search with Property Clustering & Search This Area

2 participants