Conversation
WalkthroughThis update introduces a comprehensive property search and filter feature to the web application. It adds new UI components for filtering, sorting, property listing, map integration, and detailed property views. Supporting libraries and styles for mapping and UI are included, alongside mock property data for development and testing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchBar
participant FilterSidebar
participant PropertyGrid
participant PropertyMap
participant Router
User->>SearchBar: Enter search criteria
SearchBar->>Router: Update URL parameters
Router->>SearchPage: Pass search params
SearchPage->>FilterSidebar: Provide filters, price range, etc.
SearchPage->>PropertyGrid: Provide filtered, sorted properties
PropertyGrid->>User: Display property cards
User->>FilterSidebar: Adjust filters (price, amenities, rating)
FilterSidebar->>SearchPage: Notify filter changes
SearchPage->>PropertyGrid: Update property list
User->>PropertyCard: Click "View details"
PropertyCard->>Router: Navigate to property detail page
Router->>PropertyDetailPage: Show property details
User->>FilterSidebar: (Mobile) Open map modal
FilterSidebar->>PropertyMap: Show map with markers
Possibly related issues
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Nitpick comments (16)
apps/web/src/app/page.tsx (1)
21-25: Enhance Suspense fallback accessibility & UXA plain text loading message is functional but could be improved:
- Add
role="status"andaria-live="polite"so screen-reader users are notified.- Consider a skeleton component for visual continuity.
- fallback={ - <div className="py-16 text-center">Loading properties...</div> - } + fallback={ + <div + role="status" + aria-live="polite" + className="py-16 text-center text-muted-foreground" + > + Loading properties… + </div> + }apps/web/src/components/search/PropertyGrid.tsx (2)
50-50: Use self-closing JSX element.JSX elements without children should be self-closing for better code consistency.
Apply this diff:
- <div ref={ref} className="h-10"></div> + <div ref={ref} className="h-10" />
27-31: Add dependency array validation for useEffect.The
onLoadMorefunction in the dependency array could cause unnecessary re-renders if not memoized in the parent component.Consider adding a useCallback wrapper in the parent component or add a comment explaining the expected behavior:
useEffect(() => { if (inView) { onLoadMore(); } + // Note: onLoadMore should be memoized in parent component to prevent unnecessary re-renders }, [inView, onLoadMore]);apps/web/src/components/search/PropertyCard.tsx (1)
35-68: Consider adding loading state for navigation.The navigation to property details could benefit from loading state indication.
Consider adding a loading state:
+ const [isNavigating, setIsNavigating] = useState(false); + const handleViewDetails = () => { + setIsNavigating(true); const params = new URLSearchParams(); params.set("propertyId", id); router.push(`/search/property?${params.toString()}`); };apps/web/src/components/search/SearchBar.tsx (2)
74-74: Use Number.parseInt instead of global parseInt.Modern JavaScript best practices recommend using the Number namespace for consistency.
Apply this diff:
- const paramsGuests = parseInt(searchParams.get("guests") || "2"); + const paramsGuests = Number.parseInt(searchParams.get("guests") || "2");
17-17: Improve date validation logic.The current date validation allows dates that are exactly today, which might not be the intended behavior for check-in dates.
Consider using a more precise date comparison:
checkIn: z .string() .optional() - .refine((date) => !date || new Date(date) >= new Date()), + .refine((date) => !date || new Date(date) > new Date(new Date().setHours(0, 0, 0, 0))), checkOut: z .string() .optional() - .refine((date) => !date || new Date(date) >= new Date()), + .refine((date) => !date || new Date(date) > new Date(new Date().setHours(0, 0, 0, 0))),Also applies to: 21-21
apps/web/public/mock-data.ts (1)
12-388: Consider improving mock data diversity.All properties have the same location "Luján, Buenos Aires" which reduces the realism of the mock data for testing different scenarios.
Consider adding location diversity:
{ id: "2", title: "Luxury Villa with Pool", - location: "Luján, Buenos Aires", + location: "Palermo, Buenos Aires", // ... rest of properties with varied locationsapps/web/src/app/search/page.tsx (3)
41-45: Extract hardcoded map data to constants or configurationThe map center coordinates and markers are hardcoded. Consider extracting these to a configuration file or deriving them from the actual property data.
+// Extract to a constants file or derive from filtered properties +const DEFAULT_MAP_CENTER: LatLngTuple = [-34.61, -58.39]; + export default function SearchPage() { // ... other state - const center: LatLngTuple = [-34.61, -58.39]; - const markers: { position: LatLngTuple; title: string }[] = [ - { position: [-34.61, -58.39], title: "Modern Apartment with Kitchen" }, - { position: [-34.6, -58.37], title: "Cozy Studio Apartment" } - ]; + const center: LatLngTuple = DEFAULT_MAP_CENTER; + + // Derive markers from filtered properties + const markers = useMemo(() => + filteredSortedProperties.slice(0, 10).map((property, index) => ({ + position: [center[0] + (index * 0.01), center[1] + (index * 0.01)] as LatLngTuple, + title: property.title + })), [filteredSortedProperties]);
81-82: Use Number.parseFloat for ES2015 consistencyUse
Number.parseFloatinstead of the globalparseFloatfor consistency with ES2015 standards.- const aDist = parseFloat(a.distance); - const bDist = parseFloat(b.distance); + const aDist = Number.parseFloat(a.distance); + const bDist = Number.parseFloat(b.distance);
141-143: Remove commented-out codeRemove the commented-out code block. If this functionality is needed later, it can be retrieved from version control.
- {/* <div className="w-full lg:w-[40%] h-[70vh] mt-6 lg:mt-12 rounded-2xl border m-0 lg:m-6 hidden lg:block"> - <PropertyMap center={center} markers={markers} /> - </div> */} -apps/web/src/components/features/search/SearchBar.tsx (1)
167-168: Remove console.log statementsRemove debug console.log statements before production deployment.
- console.log("Search with:", validated); - setInitialParams(validated); ... } else { setError("An unexpected error occurred"); - console.error("Search error:", error); }Also applies to: 181-182
apps/web/src/app/search/property/page.tsx (1)
447-452: Add explicit button typeAdd an explicit
type="button"to prevent form submission if this component is ever placed inside a form.<button + type="button" onClick={() => window.history.back()} className="mt-4 text-sm underline text-blue-600" >apps/web/src/components/search/FilterSidebar.tsx (3)
23-23: Remove unused importThe
SearchBarcomponent is imported but not used in this file.import "leaflet/dist/leaflet.css"; -import { SearchBar } from "../features/search/SearchBar";
65-66: Remove debug console.logRemove the debug console.log statement.
- console.log({ PropertyMap }); return (
158-169: Add explicit button typeAdd
type="button"to prevent form submission.<button + type="button" className="apps/web/src/components/ui/calendar.tsx (1)
9-9: Use import type for type-only importsThe
DayButtonis only used as a type. Useimport typeto ensure it's removed during compilation.import { ChevronDownIcon, ChevronLeftIcon, ChevronRightIcon } from "lucide-react"; -import { DayButton, DayPicker, getDefaultClassNames } from "react-day-picker"; +import { DayPicker, getDefaultClassNames } from "react-day-picker"; +import type { DayButton } from "react-day-picker";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/public/map-pointer.pngis excluded by!**/*.png
📒 Files selected for processing (16)
apps/web/package.json(3 hunks)apps/web/public/mock-data.ts(1 hunks)apps/web/src/app/globals.css(1 hunks)apps/web/src/app/layout.tsx(1 hunks)apps/web/src/app/page.tsx(2 hunks)apps/web/src/app/search/page.tsx(1 hunks)apps/web/src/app/search/property/page.tsx(1 hunks)apps/web/src/components/features/search/SearchBar.tsx(13 hunks)apps/web/src/components/search/FilterSidebar.tsx(1 hunks)apps/web/src/components/search/Map.tsx(1 hunks)apps/web/src/components/search/PropertyCard.tsx(1 hunks)apps/web/src/components/search/PropertyGrid.tsx(1 hunks)apps/web/src/components/search/SearchBar.tsx(1 hunks)apps/web/src/components/search/SortOptions.tsx(1 hunks)apps/web/src/components/ui/calendar.tsx(1 hunks)apps/web/src/components/ui/popover.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
apps/web/src/components/search/SortOptions.tsx (2)
apps/web/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(185-185)DropdownMenuTrigger(186-186)DropdownMenuContent(187-187)DropdownMenuItem(188-188)apps/web/src/components/ui/button.tsx (1)
Button(59-59)
apps/web/src/components/ui/popover.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/app/search/page.tsx (6)
apps/web/src/components/search/Map.tsx (1)
PropertyMap(37-74)apps/web/public/mock-data.ts (2)
FullProperyProps(1-10)MOCK_PROPERTIES(12-388)apps/web/src/components/search/FilterSidebar.tsx (1)
FilterSidebar(37-185)apps/web/src/components/search/SearchBar.tsx (1)
SearchBar(34-184)apps/web/src/components/search/SortOptions.tsx (1)
SortOptions(22-81)apps/web/src/components/search/PropertyGrid.tsx (1)
PropertyGrid(22-53)
apps/web/src/components/search/PropertyGrid.tsx (2)
apps/web/public/mock-data.ts (1)
FullProperyProps(1-10)apps/web/src/components/search/PropertyCard.tsx (1)
PropertyCard(18-70)
apps/web/src/app/search/property/page.tsx (1)
apps/web/public/mock-data.ts (2)
FullProperyProps(1-10)MOCK_PROPERTIES(12-388)
apps/web/src/components/ui/calendar.tsx (2)
apps/web/src/components/ui/button.tsx (2)
Button(59-59)buttonVariants(59-59)apps/web/src/lib/utils.ts (1)
cn(4-6)
🪛 Biome (1.9.4)
apps/web/src/components/search/SortOptions.tsx
[error] 34-34: This hook does not specify all of its dependencies: selected.value
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
apps/web/src/app/search/page.tsx
[error] 81-81: Use Number.parseFloat instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseFloat instead.
(lint/style/useNumberNamespace)
[error] 82-82: Use Number.parseFloat instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseFloat instead.
(lint/style/useNumberNamespace)
apps/web/src/components/search/SearchBar.tsx
[error] 101-106: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 74-74: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
apps/web/src/components/search/Map.tsx
[error] 14-15: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 15-15: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 54-54: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
apps/web/src/components/search/PropertyCard.tsx
[error] 61-64: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/web/src/components/search/PropertyGrid.tsx
[error] 50-50: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
apps/web/src/app/search/property/page.tsx
[error] 421-421: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 448-452: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/web/src/components/search/FilterSidebar.tsx
[error] 44-44: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 46-46: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 61-61: This hook does not specify all of its dependencies: onFiltersChange
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
[error] 158-166: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/web/src/components/ui/calendar.tsx
[error] 9-9: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
🔇 Additional comments (7)
apps/web/src/app/page.tsx (1)
1-7: Consistent import style adopted – looks goodSwitching to double-quoted paths aligns with the rest of the codebase and ESLint/Prettier defaults. No further action required.
apps/web/package.json (1)
18-18: LGTM! Dependencies align with search/filter functionality.The added dependencies (@radix-ui/react-slider, leaflet, react-leaflet, @types/leaflet) properly support the new search page features including price filtering and map integration.
Also applies to: 29-29, 39-39, 47-47
apps/web/src/app/globals.css (1)
1-2: LGTM! Leaflet CSS import is necessary for map functionality.The import is correctly placed before Tailwind imports to ensure proper CSS cascade order.
apps/web/src/app/layout.tsx (1)
2-2: Clarify the need for Mapbox CSS import.The PR objectives state that Leaflet was chosen over Mapbox due to integration constraints, but this change imports Mapbox GL CSS. This seems inconsistent and may lead to unnecessary bundle size or style conflicts.
Please verify if this Mapbox CSS import is actually needed, or if it should be removed since the implementation uses Leaflet.
apps/web/src/components/ui/popover.tsx (1)
1-34: LGTM! Well-structured popover component.The implementation correctly wraps Radix UI popover primitives with proper TypeScript typing, forwardRef pattern, and comprehensive styling. The component follows established patterns and will integrate well with the search/filter functionality.
apps/web/src/components/search/SortOptions.tsx (1)
22-81: LGTM! Well-implemented sort functionality with URL synchronization.The component effectively manages sorting state and URL synchronization using Next.js navigation hooks. The dropdown UI is responsive and properly styled for both light and dark modes.
apps/web/src/components/search/PropertyGrid.tsx (1)
44-46: Good key handling implementation.The fallback key logic
property.id || iis well implemented to handle cases where properties might not have IDs.
| const customIcon = new L.Icon({ | ||
| iconUrl: "/map-pointer.png", | ||
| iconSize: [32, 32], | ||
| iconAnchor: [16, 32], | ||
| popupAnchor: [0, -32], | ||
| shadowUrl: require("leaflet/dist/images/marker-shadow.png"), | ||
| shadowSize: [40, 40], | ||
| shadowAnchor: [13, 40], | ||
| className: "custom-map-icon" | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the custom icon image exists and use consistent imports.
The component references /map-pointer.png but there's no guarantee this file exists. Also, mixing require() with ES6 imports is inconsistent.
Apply this diff to use consistent imports:
shadowUrl: require("leaflet/dist/images/marker-shadow.png"),
+ // Consider: shadowUrl: "/images/marker-shadow.png",🏁 Script executed:
#!/bin/bash
# Check if the custom icon image exists
fd -t f "map-pointer.png" apps/web/public/Length of output: 74
Import the marker shadow image instead of using require
We’ve confirmed that apps/web/public/map-pointer.png exists, so the iconUrl: "/map-pointer.png" reference is valid. To keep imports consistent, switch the shadow image to an ES6 import:
• File: apps/web/src/components/search/Map.tsx
• At the top, add:
ts import markerShadow from 'leaflet/dist/images/marker-shadow.png';
• In your customIcon definition replace:
```diff
- shadowUrl: require("leaflet/dist/images/marker-shadow.png"),
- shadowUrl: markerShadow,
Ensure your build setup supports importing images from node_modules.
🤖 Prompt for AI Agents
In apps/web/src/components/search/Map.tsx around lines 18 to 27, replace the use
of require for the shadowUrl with an ES6 import. At the top of the file, add an
import statement for markerShadow from 'leaflet/dist/images/marker-shadow.png'.
Then update the shadowUrl property in the customIcon definition to use this
imported markerShadow variable instead of require. Confirm your build setup
supports importing images from node_modules.
| import "leaflet/dist/leaflet.css"; | ||
| import L from "leaflet"; | ||
|
|
||
| delete (L.Icon.Default.prototype as any)._getIconUrl; |
There was a problem hiding this comment.
Replace the delete operation with a safer alternative.
The delete operator can impact performance and using any type reduces type safety.
Apply this diff to use a safer approach:
-delete (L.Icon.Default.prototype as any)._getIconUrl;
+L.Icon.Default.prototype._getIconUrl = undefined as any;Or better yet, configure the icon paths explicitly:
-delete (L.Icon.Default.prototype as any)._getIconUrl;
+L.Icon.Default.mergeOptions({
+ iconRetinaUrl: require('leaflet/dist/images/marker-icon-2x.png'),
+ iconUrl: require('leaflet/dist/images/marker-icon.png'),
+ shadowUrl: require('leaflet/dist/images/marker-shadow.png'),
+});📝 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.
| delete (L.Icon.Default.prototype as any)._getIconUrl; | |
| // replace this line: | |
| // delete (L.Icon.Default.prototype as any)._getIconUrl; | |
| // with an explicit icon configuration: | |
| L.Icon.Default.mergeOptions({ | |
| iconRetinaUrl: require('leaflet/dist/images/marker-icon-2x.png'), | |
| iconUrl: require('leaflet/dist/images/marker-icon.png'), | |
| shadowUrl: require('leaflet/dist/images/marker-shadow.png'), | |
| }); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 14-15: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 15-15: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🤖 Prompt for AI Agents
In apps/web/src/components/search/Map.tsx at line 15, avoid using the delete
operator on (L.Icon.Default.prototype as any)._getIconUrl because it harms
performance and bypasses type safety. Instead, explicitly configure the icon
paths by setting the appropriate properties on L.Icon.Default or override the
method with a safer alternative that does not use delete. This maintains type
safety and improves code clarity.
| setError(null); | ||
| }; | ||
|
|
||
| const setInitialParams = async (valiadatedFormInputs: SearchFormData) => { |
There was a problem hiding this comment.
Fix typo in parameter name
The parameter name has a typo: "valiadatedFormInputs" should be "validatedFormInputs".
- const setInitialParams = async (valiadatedFormInputs: SearchFormData) => {
+ const setInitialParams = async (validatedFormInputs: SearchFormData) => {
const params = new URLSearchParams(Array.from(searchParams.entries()));
- params.set("location", valiadatedFormInputs.location);
- valiadatedFormInputs.checkIn &&
- params.set("checkIn", valiadatedFormInputs.checkIn);
- valiadatedFormInputs.checkOut &&
- params.set("checkOut", valiadatedFormInputs.checkOut);
- valiadatedFormInputs.guests &&
- params.set("guests", valiadatedFormInputs.guests.toString());
+ params.set("location", validatedFormInputs.location);
+ validatedFormInputs.checkIn &&
+ params.set("checkIn", validatedFormInputs.checkIn);
+ validatedFormInputs.checkOut &&
+ params.set("checkOut", validatedFormInputs.checkOut);
+ validatedFormInputs.guests &&
+ params.set("guests", validatedFormInputs.guests.toString());Also applies to: 119-119, 169-169
🤖 Prompt for AI Agents
In apps/web/src/components/features/search/SearchBar.tsx at lines 111, 119, and
169, fix the typo in the parameter name from "valiadatedFormInputs" to
"validatedFormInputs" to ensure correct spelling and consistency.
| {property.images.map((img, index) => ( | ||
| <div | ||
| key={index} | ||
| className={`relative overflow-hidden rounded-xl ${ | ||
| index === 0 | ||
| ? "col-span-2 row-span-2 sm:row-span-2 sm:col-span-2" | ||
| : "" | ||
| }`} | ||
| > | ||
| <Image | ||
| width={300} | ||
| height={300} | ||
| src={img} | ||
| alt={`${property.title} ${index + 1}`} | ||
| className="w-full h-full object-cover" | ||
| /> | ||
| </div> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using array index as React key
Using array indices as keys can lead to issues with component state when the list order changes. Since each image has a unique path, use that instead.
{property.images.map((img, index) => (
<div
- key={index}
+ key={`${property.id}-${img}`}
className={`relative overflow-hidden rounded-xl ${
index === 0
? "col-span-2 row-span-2 sm:row-span-2 sm:col-span-2"
: ""
}`}
>📝 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.
| {property.images.map((img, index) => ( | |
| <div | |
| key={index} | |
| className={`relative overflow-hidden rounded-xl ${ | |
| index === 0 | |
| ? "col-span-2 row-span-2 sm:row-span-2 sm:col-span-2" | |
| : "" | |
| }`} | |
| > | |
| <Image | |
| width={300} | |
| height={300} | |
| src={img} | |
| alt={`${property.title} ${index + 1}`} | |
| className="w-full h-full object-cover" | |
| /> | |
| </div> | |
| ))} | |
| </div> | |
| {property.images.map((img, index) => ( | |
| <div | |
| key={`${property.id}-${img}`} | |
| className={`relative overflow-hidden rounded-xl ${ | |
| index === 0 | |
| ? "col-span-2 row-span-2 sm:row-span-2 sm:col-span-2" | |
| : "" | |
| }`} | |
| > | |
| <Image | |
| width={300} | |
| height={300} | |
| src={img} | |
| alt={`${property.title} ${index + 1}`} | |
| className="w-full h-full object-cover" | |
| /> | |
| </div> | |
| ))} | |
| </div> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 421-421: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
🤖 Prompt for AI Agents
In apps/web/src/app/search/property/page.tsx around lines 418 to 436, the React
key for the image list items is currently set to the array index, which can
cause issues with component state when the list changes. Replace the key from
the index to the unique image path (img) to ensure stable and unique keys for
each element.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/web/src/components/search/Map.tsx (2)
15-15: Replace the delete operation with a safer alternative.
23-23: Import the marker shadow image instead of usingrequireapps/web/src/components/search/FilterSidebar.tsx (1)
68-70: Add missing dependency to useEffect.
🧹 Nitpick comments (1)
apps/web/src/components/search/SearchBar.tsx (1)
132-132: Consider using semantic button element.While the current implementation works, using a semantic
<button>element instead ofdivwithrole="button"would be more semantically correct and provide better built-in accessibility.- <div + <button + type="button" key={sug} className="px-2 py-1 hover:bg-muted cursor-pointer" onClick={() => selectSuggestion(sug)} onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); selectSuggestion(sug); } }} tabIndex={0} - role="button" > {sug} - </div> + </button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/public/mock-data.ts(1 hunks)apps/web/src/app/layout.tsx(1 hunks)apps/web/src/app/search/page.tsx(1 hunks)apps/web/src/app/search/property/page.tsx(1 hunks)apps/web/src/components/features/search/SearchBar.tsx(13 hunks)apps/web/src/components/search/FilterSidebar.tsx(1 hunks)apps/web/src/components/search/Map.tsx(1 hunks)apps/web/src/components/search/PropertyCard.tsx(1 hunks)apps/web/src/components/search/PropertyGrid.tsx(1 hunks)apps/web/src/components/search/SearchBar.tsx(1 hunks)apps/web/src/components/search/SortOptions.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/web/src/app/layout.tsx
- apps/web/src/app/search/page.tsx
- apps/web/src/components/search/PropertyGrid.tsx
- apps/web/src/components/search/SortOptions.tsx
- apps/web/src/components/search/PropertyCard.tsx
- apps/web/src/components/features/search/SearchBar.tsx
- apps/web/src/app/search/property/page.tsx
- apps/web/public/mock-data.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/web/src/components/search/FilterSidebar.tsx (2)
apps/web/src/components/search/Map.tsx (1)
PropertyMap(37-85)apps/web/src/components/ui/dialog.tsx (5)
Dialog(112-112)DialogTrigger(116-116)DialogContent(117-117)DialogHeader(118-118)DialogTitle(120-120)
apps/web/src/components/search/SearchBar.tsx (4)
apps/web/src/components/features/search/SearchBar.tsx (1)
SearchBar(50-325)apps/web/src/components/ui/popover.tsx (3)
Popover(33-33)PopoverTrigger(33-33)PopoverContent(33-33)apps/web/src/components/ui/button.tsx (1)
Button(59-59)apps/web/src/components/ui/calendar.tsx (1)
Calendar(210-210)
🪛 Biome (1.9.4)
apps/web/src/components/search/FilterSidebar.tsx
[error] 68-68: This hook does not specify all of its dependencies: onFiltersChange
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
apps/web/src/components/search/Map.tsx
[error] 14-15: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 15-15: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
apps/web/src/components/search/SearchBar.tsx
[error] 132-133: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
[error] 86-86: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 92-92: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (5)
apps/web/src/components/search/Map.tsx (2)
37-44: Good error handling implementation.The prop validation and fallback UI for missing center or markers is well implemented and improves the component's robustness.
64-64: Improved key generation for better React performance.The key now uses a stable combination of position and title, which is much better than the previous array index approach.
apps/web/src/components/search/SearchBar.tsx (2)
125-133: Good accessibility implementation.The keyboard event handling with Enter and Space keys, along with proper ARIA attributes, makes the suggestions accessible to keyboard users.
109-116: Improved blur handling eliminates race conditions.The relatedTarget approach is much more reliable than the previous setTimeout method and prevents premature hiding of suggestions.
apps/web/src/components/search/FilterSidebar.tsx (1)
24-39: Excellent TypeScript interface implementation.The proper type definitions for
FiltersandFilterSidebarPropssignificantly improve type safety compared to usinganytypes.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/src/app/search/property/page.tsx (1)
80-97: Fix React key to avoid using array indexUsing array index as React key can cause issues when the list order changes. This was flagged in previous reviews.
-key={`${property.id}-${index}`} +key={`${property.id}-${img}`}
🧹 Nitpick comments (3)
apps/web/src/components/features/properties/PropertyDetail.tsx (1)
174-185: Consider extracting shared booking logicThe
calculateNightsfunction is duplicated in bothPropertyDetail.tsxandapps/web/src/app/search/property/page.tsx. Consider extracting this to a shared utility function.Create a utility file like
utils/booking.ts:export const calculateNights = (checkIn: string, checkOut: string): number => { if (!checkIn || !checkOut) return 0; const checkInDate = new Date(checkIn); const checkOutDate = new Date(checkOut); // Validate dates if (checkInDate >= checkOutDate) return 0; const timeDiff = checkOutDate.getTime() - checkInDate.getTime(); return Math.ceil(timeDiff / (1000 * 60 * 60 * 24)); };Then import and use it in both components.
apps/web/src/app/search/property/page.tsx (2)
30-41: Extract duplicated booking logicThe
calculateNightsfunction is duplicated fromPropertyDetail.tsx. Consider extracting this to a shared utility.Same as suggested for
PropertyDetail.tsx- create a shared utility function to avoid duplication.
66-72: Consider more accessible back navigationUsing
window.history.back()may not work as expected in all scenarios (e.g., if the user navigated directly to this page). Consider using Next.js router for more reliable navigation.+import { useRouter } from 'next/navigation'; +const router = useRouter(); -onClick={() => window.history.back()} +onClick={() => router.back()}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/public/mock-data.ts(1 hunks)apps/web/src/app/search/property/page.tsx(1 hunks)apps/web/src/components/features/properties/PropertyDetail.tsx(11 hunks)apps/web/src/components/search/PropertyCard.tsx(1 hunks)apps/web/src/components/search/SearchBar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/components/search/SearchBar.tsx
- apps/web/src/components/search/PropertyCard.tsx
- apps/web/public/mock-data.ts
🔇 Additional comments (2)
apps/web/src/components/features/properties/PropertyDetail.tsx (1)
137-143: LGTM: Improved state organizationReordering the state hooks at the top improves readability and follows React best practices.
apps/web/src/app/search/property/page.tsx (1)
18-277: LGTM: Well-structured component with good UXThe component is well-organized with proper state management, responsive design, and good user experience features like sticky booking card and date validation.
| const mockProperties = [ | ||
| { | ||
| id: "1", | ||
| title: "Modern Apartment with Kitchen", | ||
| location: "Luján, Buenos Aires", | ||
| price: 2500, | ||
| image: "/images/house1.jpg", | ||
| rating: 4.1, | ||
| distance: "30km", | ||
| maxGuests: 4, | ||
| bedrooms: 2, | ||
| amenities: [ | ||
| "Wi-Fi", | ||
| "Air conditioning", | ||
| "Fully equipped kitchen", | ||
| "Washer & dryer" | ||
| ] | ||
| }, | ||
| { | ||
| id: "2", | ||
| title: "Luxury Villa with Pool", | ||
| location: "Luján, Buenos Aires", | ||
| price: 6000, | ||
| image: "/images/house2.jpg", | ||
| rating: 4.8, | ||
| distance: "6km", | ||
| maxGuests: 8, | ||
| bedrooms: 4, | ||
| amenities: [ | ||
| "Pool", | ||
| "Wi-Fi", | ||
| "Air conditioning", | ||
| "Fully equipped kitchen", | ||
| "Washer & dryer" | ||
| ] | ||
| }, | ||
| { | ||
| id: "3", | ||
| title: "Cozy Bedroom Suite", | ||
| location: "Luján, Buenos Aires", | ||
| price: 4500, | ||
| image: "/images/house3.jpg", | ||
| rating: 3.9, | ||
| distance: "14km", | ||
| maxGuests: 2, | ||
| bedrooms: 1, | ||
| amenities: ["Wi-Fi", "Air conditioning", "Fully equipped kitchen"] | ||
| }, | ||
| { | ||
| id: "4", | ||
| title: "Elegant Studio Apartment", | ||
| location: "Luján, Buenos Aires", | ||
| price: 5600, | ||
| image: "/images/house4.jpg", | ||
| rating: 4.5, | ||
| distance: "8km", | ||
| maxGuests: 3, | ||
| bedrooms: 1, | ||
| amenities: [ | ||
| "Wi-Fi", | ||
| "Air conditioning", | ||
| "Smart TV", | ||
| "Fully equipped kitchen" | ||
| ] | ||
| }, | ||
| { | ||
| id: "5", | ||
| title: "Charming Kitchen Loft", | ||
| location: "Luján, Buenos Aires", | ||
| price: 2100, | ||
| image: "/images/house5.jpg", | ||
| rating: 4.2, | ||
| distance: "12km", | ||
| maxGuests: 2, | ||
| bedrooms: 1, | ||
| amenities: ["Wi-Fi", "Fully equipped kitchen", "Free parking"] | ||
| }, | ||
| { | ||
| id: "6", | ||
| title: "Modern Architectural House", | ||
| location: "Luján, Buenos Aires", | ||
| price: 6500, | ||
| image: "/images/house.jpg", | ||
| rating: 4.7, | ||
| distance: "10km", | ||
| maxGuests: 6, | ||
| bedrooms: 3, | ||
| amenities: [ | ||
| "Wi-Fi", | ||
| "Air conditioning", | ||
| "Fully equipped kitchen", | ||
| "Washer & dryer", | ||
| "Free parking", | ||
| "Smart TV" | ||
| ] | ||
| } | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace local mock data with centralized import
The mockProperties array duplicates data that now exists in the centralized apps/web/public/mock-data.ts file. This creates inconsistency and makes maintenance harder.
-const mockProperties = [
- {
- id: "1",
- title: "Modern Apartment with Kitchen",
- location: "Luján, Buenos Aires",
- price: 2500,
- image: "/images/house1.jpg",
- rating: 4.1,
- distance: "30km",
- maxGuests: 4,
- bedrooms: 2,
- amenities: [
- "Wi-Fi",
- "Air conditioning",
- "Fully equipped kitchen",
- "Washer & dryer"
- ]
- },
- // ... rest of the array
-];
+import { MOCK_PROPERTIES } from '@/public/mock-data';Then update the getPropertyById function to use the imported data:
-const property = mockProperties.find((property) => property.id === id);
+const property = MOCK_PROPERTIES.find((property) => property.id === id);🤖 Prompt for AI Agents
In apps/web/src/components/features/properties/PropertyDetail.tsx around lines
23 to 119, replace the local mockProperties array with an import from the
centralized mock-data.ts file located at apps/web/public/mock-data.ts. Remove
the entire local mockProperties declaration and import the mock data at the top
of the file. Then update the getPropertyById function to reference the imported
mock data instead of the removed local array to ensure consistency and easier
maintenance.
| <div className="grid gap-2 sm:gap-4 grid-cols-2 sm:grid-cols-4 auto-rows-[150px] sm:auto-rows-[200px]"> | ||
| {property.images.map((img, index) => ( | ||
| <div | ||
| key={`${property.id}-${index}`} | ||
| className={`relative overflow-hidden rounded-xl ${ | ||
| index === 0 | ||
| ? "col-span-2 row-span-2 sm:row-span-2 sm:col-span-2" | ||
| : "" | ||
| }`} | ||
| > | ||
| <Image | ||
| width={300} | ||
| height={300} | ||
| src={img} | ||
| alt={`${property.title} ${index + 1}`} | ||
| className="w-full h-full object-cover" | ||
| /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ) : ( | ||
| <div className="w-full h-full bg-gray-200 dark:bg-gray-700 flex items-center justify-center"> | ||
| <p className="text-muted-foreground">Image not available</p> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for image loading
The component handles imageError state but doesn't actually set it when images fail to load. Add error handling for the image gallery.
<Image
width={300}
height={300}
src={img}
alt={`${property.title} ${index + 1}`}
className="w-full h-full object-cover"
+ onError={() => setImageError(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.
| <div className="grid gap-2 sm:gap-4 grid-cols-2 sm:grid-cols-4 auto-rows-[150px] sm:auto-rows-[200px]"> | |
| {property.images.map((img, index) => ( | |
| <div | |
| key={`${property.id}-${index}`} | |
| className={`relative overflow-hidden rounded-xl ${ | |
| index === 0 | |
| ? "col-span-2 row-span-2 sm:row-span-2 sm:col-span-2" | |
| : "" | |
| }`} | |
| > | |
| <Image | |
| width={300} | |
| height={300} | |
| src={img} | |
| alt={`${property.title} ${index + 1}`} | |
| className="w-full h-full object-cover" | |
| /> | |
| </div> | |
| ))} | |
| </div> | |
| ) : ( | |
| <div className="w-full h-full bg-gray-200 dark:bg-gray-700 flex items-center justify-center"> | |
| <p className="text-muted-foreground">Image not available</p> | |
| </div> | |
| )} | |
| <div className="grid gap-2 sm:gap-4 grid-cols-2 sm:grid-cols-4 auto-rows-[150px] sm:auto-rows-[200px]"> | |
| {property.images.map((img, index) => ( | |
| <div | |
| key={`${property.id}-${index}`} | |
| className={`relative overflow-hidden rounded-xl ${ | |
| index === 0 | |
| ? "col-span-2 row-span-2 sm:row-span-2 sm:col-span-2" | |
| : "" | |
| }`} | |
| > | |
| <Image | |
| width={300} | |
| height={300} | |
| src={img} | |
| alt={`${property.title} ${index + 1}`} | |
| className="w-full h-full object-cover" | |
| onError={() => setImageError(true)} | |
| /> | |
| </div> | |
| ))} | |
| </div> | |
| ) : ( | |
| <div className="w-full h-full bg-gray-200 dark:bg-gray-700 flex items-center justify-center"> | |
| <p className="text-muted-foreground">Image not available</p> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In apps/web/src/app/search/property/page.tsx around lines 79 to 103, the image
gallery renders images but does not handle image loading errors despite having
an imageError state. Add an onError event handler to each Image component that
sets the imageError state when an image fails to load. This will enable proper
error handling and fallback UI display for failed image loads.
| import type { FullPropertyProps } from "public/mock-data"; | ||
| import { MOCK_PROPERTIES } from "public/mock-data"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify import path for mock data
The import path "public/mock-data" may be incorrect. Typically, the public directory is not directly importable. Verify this path works correctly.
🏁 Script executed:
#!/bin/bash
# Check if the mock-data file exists and verify the correct import path
fd -t f "mock-data" --exec ls -la {}Length of output: 118
🏁 Script executed:
#!/bin/bash
# Search for TypeScript path aliases and baseUrl configurations
echo "=== Searching for baseUrl and paths in tsconfig files ==="
rg '"baseUrl"' -n .
rg '"paths"' -n .Length of output: 257
Address React key usage, extract duplicated booking logic, and add image error handling
The import path for public/mock-data is valid (there’s a apps/web/public/mock-data.ts file and baseUrl: "." in tsconfig), so you can remove that warning. The remaining issues to address:
• Avoid using array index as React key
– In apps/web/src/app/search/property/page.tsx, change
<div key={index}>
to something like
<div key={property.id}>
(or another stable unique identifier).
• Extract duplicated booking logic
– The booking functionality here mirrors PropertyDetail.tsx.
– Factor it out into a shared hook or utility (e.g. useBooking) to DRY up the code.
• Add error handling for image loading
– For each <img> tag, include an onError handler to display a fallback UI or placeholder if the image fails to load.
🤖 Prompt for AI Agents
In apps/web/src/app/search/property/page.tsx around lines 5 to 6, remove the
warning about the import path since it is valid. Then, replace the React key
currently using the array index with a stable unique identifier like property.id
to avoid key issues. Next, identify the duplicated booking logic shared with
PropertyDetail.tsx and extract it into a reusable hook or utility named
useBooking to reduce code duplication. Finally, add an onError handler to each
img tag to handle image load failures gracefully by displaying a fallback UI or
placeholder image.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/web/src/components/search/FilterSidebar.tsx (1)
69-71: useEffect dependency issue remains unresolvedDespite being marked as addressed in previous reviews, the
onFiltersChangedependency is still missing from the useEffect dependency array. This could cause the effect to use stale references.useEffect(() => { onFiltersChange({ price, amenities, rating }); - }, [price, amenities, rating]); + }, [price, amenities, rating, onFiltersChange]);Ensure the parent component wraps
onFiltersChangeinuseCallbackto prevent unnecessary re-renders.
🧹 Nitpick comments (4)
apps/web/src/components/search/FilterSidebar.tsx (2)
61-63: Improve price state initialization logicThe current logic assumes that
filters.price === 0means "use minimum price," but this could be problematic if 0 is a valid price value (e.g., free properties). Consider using a more explicit check for undefined/null values.- const [price, setPrice] = useState( - filters.price === 0 ? minAndMaxPrice[0] : filters.price - ); + const [price, setPrice] = useState( + filters.price ?? minAndMaxPrice[0] + );This approach only uses the default minimum price when the filter price is explicitly undefined or null.
64-66: Consider more specific type for amenities stateThe current type
{ [key: string]: boolean }is generic. Consider using a more specific type based on the actual amenity names for better type safety.+ type AmenityKey = 'wifi' | 'pool' | 'pet friendly' | 'parking space' | 'garden'; + - const [amenities, setAmenities] = useState<{ [key: string]: boolean }>( + const [amenities, setAmenities] = useState<Partial<Record<AmenityKey, boolean>>>( filters.amenities || {} );This provides better type checking and autocomplete support for amenity keys.
apps/web/src/components/search/SearchBar.tsx (2)
46-50: Optimize URL parameter updates to avoid unnecessary navigation.The
updateParamsfunction always navigates to/searcheven when already on the search page, which can cause unnecessary re-renders and navigation history pollution.Apply this diff to optimize navigation:
const updateParams = (key: string, value: string) => { const params = new URLSearchParams(Array.from(searchParams.entries())); params.set(key, value); - router.replace(`/search?${params.toString()}`); + const newUrl = `/search?${params.toString()}`; + if (window.location.pathname + window.location.search !== newUrl) { + router.replace(newUrl); + } };
25-32: Consider making locations dynamic.The hardcoded LOCATIONS array limits the search functionality to specific locations. Consider fetching this data from an API or configuration.
+// Consider fetching from API or configuration +const [locations, setLocations] = useState(LOCATIONS); + +useEffect(() => { + // Fetch locations from API + // setLocations(fetchedLocations); +}, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/search/FilterSidebar.tsx(1 hunks)apps/web/src/components/search/SearchBar.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/src/components/search/FilterSidebar.tsx (2)
apps/web/src/components/search/Map.tsx (1)
PropertyMap(37-85)apps/web/src/components/ui/dialog.tsx (5)
Dialog(112-112)DialogTrigger(116-116)DialogContent(117-117)DialogHeader(118-118)DialogTitle(120-120)
🪛 Biome (1.9.4)
apps/web/src/components/search/FilterSidebar.tsx
[error] 69-69: This hook does not specify all of its dependencies: onFiltersChange
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
🔇 Additional comments (1)
apps/web/src/components/search/FilterSidebar.tsx (1)
25-40: Well-structured TypeScript interfacesThe interfaces are properly defined with clear types, addressing the previous concerns about using
anytypes. The type definitions provide good structure for the component's props and state.
| <div className="w-full lg:w-[40%] h-[80vh] md:hidden mt-4 lg:mt-12 rounded-2xl border m-0 lg:m-6 block"> | ||
| <PropertyMap center={center} markers={markers} /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up inconsistent responsive styling
The styling classes contain conflicting responsive behaviors: md:hidden hides the element on medium screens and up, but lg:w-[40%] and lg:mt-12 apply styles for large screens.
- <div className="w-full lg:w-[40%] h-[80vh] md:hidden mt-4 lg:mt-12 rounded-2xl border m-0 lg:m-6 block">
+ <div className="w-full h-[80vh] mt-4 rounded-2xl border">Since this is inside a mobile-only dialog (block md:hidden on the parent), the responsive classes for larger screens are unnecessary.
📝 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-full lg:w-[40%] h-[80vh] md:hidden mt-4 lg:mt-12 rounded-2xl border m-0 lg:m-6 block"> | |
| <PropertyMap center={center} markers={markers} /> | |
| </div> | |
| <div className="w-full h-[80vh] mt-4 rounded-2xl border"> | |
| <PropertyMap center={center} markers={markers} /> | |
| </div> |
🤖 Prompt for AI Agents
In apps/web/src/components/search/FilterSidebar.tsx around lines 185 to 187,
remove the responsive classes lg:w-[40%] and lg:mt-12 from the div since the
parent container is already mobile-only with block md:hidden, making these large
screen styles redundant and inconsistent. Keep only the necessary mobile styles
to clean up the responsive styling.
| onSelect={(d: Date) => { | ||
| setCheckIn(d); | ||
| if (d) updateParams("checkIn", d.toISOString()); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix Calendar onSelect handler type annotation.
The Calendar component's onSelect handler receives Date | undefined, but the type annotation specifies only Date.
Apply this diff to fix the type annotations:
- onSelect={(d: Date) => {
+ onSelect={(d: Date | undefined) => {
setCheckIn(d);
if (d) updateParams("checkIn", d.toISOString());
}}- onSelect={(d: Date) => {
+ onSelect={(d: Date | undefined) => {
setCheckOut(d);
if (d) updateParams("checkOut", d.toISOString());
}}Also applies to: 182-185
🤖 Prompt for AI Agents
In apps/web/src/components/search/SearchBar.tsx around lines 157 to 160 and 182
to 185, the onSelect handler for the Calendar component is incorrectly typed to
accept only Date, but it can receive Date or undefined. Update the type
annotation of the onSelect parameter from (d: Date) to (d: Date | undefined) to
correctly reflect the possible input types and avoid type errors.
| onChange={(e) => { | ||
| const val = Number(e.target.value); | ||
| setGuests(val); | ||
| updateParams("guests", val.toString()); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for guests input.
The guests input doesn't validate the entered value, which could result in invalid guest counts being set.
Apply this diff to add validation:
onChange={(e) => {
- const val = Number(e.target.value);
- setGuests(val);
- updateParams("guests", val.toString());
+ const val = Number(e.target.value);
+ const validVal = Number.isNaN(val) ? 1 : Math.max(1, Math.min(16, val));
+ setGuests(validVal);
+ updateParams("guests", validVal.toString());
}}📝 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.
| onChange={(e) => { | |
| const val = Number(e.target.value); | |
| setGuests(val); | |
| updateParams("guests", val.toString()); | |
| }} | |
| onChange={(e) => { | |
| const val = Number(e.target.value); | |
| const validVal = Number.isNaN(val) ? 1 : Math.max(1, Math.min(16, val)); | |
| setGuests(validVal); | |
| updateParams("guests", validVal.toString()); | |
| }} |
🤖 Prompt for AI Agents
In apps/web/src/components/search/SearchBar.tsx around lines 201 to 205, the
onChange handler for the guests input does not validate the entered value,
allowing invalid guest counts. Add validation to ensure the value is a positive
integer within an acceptable range before calling setGuests and updateParams. If
the value is invalid, prevent updating the state and parameters.
| // const searchSchema = z.object({ | ||
| // location: z.string().min(2).max(100), | ||
| // checkIn: z | ||
| // .string() | ||
| // .optional() | ||
| // .refine((date) => !date || new Date(date) > new Date(new Date().setHours(0, 0, 0, 0))), | ||
| // checkOut: z | ||
| // .string() | ||
| // .optional() | ||
| // .refine((date) => !date || new Date(date) >= new Date()), | ||
| // guests: z.number().int().min(1).max(16).optional(), | ||
| // }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider implementing the validation schema.
The Zod validation schema is commented out, leaving the form without client-side validation. This could lead to invalid data being processed or poor user experience.
Apply this diff to implement basic validation:
-// const searchSchema = z.object({
-// location: z.string().min(2).max(100),
-// checkIn: z
-// .string()
-// .optional()
-// .refine((date) => !date || new Date(date) > new Date(new Date().setHours(0, 0, 0, 0))),
-// checkOut: z
-// .string()
-// .optional()
-// .refine((date) => !date || new Date(date) >= new Date()),
-// guests: z.number().int().min(1).max(16).optional(),
-// });
+const searchSchema = z.object({
+ location: z.string().min(2, "Location must be at least 2 characters").max(100),
+ checkIn: z
+ .string()
+ .optional()
+ .refine((date) => !date || new Date(date) > new Date(new Date().setHours(0, 0, 0, 0)), "Check-in date must be in the future"),
+ checkOut: z
+ .string()
+ .optional()
+ .refine((date) => !date || new Date(date) >= new Date(), "Check-out date cannot be in the past"),
+ guests: z.number().int().min(1, "At least 1 guest required").max(16, "Maximum 16 guests").optional(),
+});📝 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 searchSchema = z.object({ | |
| // location: z.string().min(2).max(100), | |
| // checkIn: z | |
| // .string() | |
| // .optional() | |
| // .refine((date) => !date || new Date(date) > new Date(new Date().setHours(0, 0, 0, 0))), | |
| // checkOut: z | |
| // .string() | |
| // .optional() | |
| // .refine((date) => !date || new Date(date) >= new Date()), | |
| // guests: z.number().int().min(1).max(16).optional(), | |
| // }); | |
| const searchSchema = z.object({ | |
| location: z.string().min(2, "Location must be at least 2 characters").max(100), | |
| checkIn: z | |
| .string() | |
| .optional() | |
| .refine( | |
| (date) => | |
| !date || new Date(date) > new Date(new Date().setHours(0, 0, 0, 0)), | |
| "Check-in date must be in the future" | |
| ), | |
| checkOut: z | |
| .string() | |
| .optional() | |
| .refine( | |
| (date) => !date || new Date(date) >= new Date(), | |
| "Check-out date cannot be in the past" | |
| ), | |
| guests: z | |
| .number() | |
| .int() | |
| .min(1, "At least 1 guest required") | |
| .max(16, "Maximum 16 guests") | |
| .optional(), | |
| }); |
🤖 Prompt for AI Agents
In apps/web/src/components/search/SearchBar.tsx between lines 12 and 23, the Zod
validation schema is currently commented out, resulting in no client-side
validation for the search form. To fix this, uncomment and integrate the
searchSchema definition into the form handling logic to enforce validation rules
on location, check-in/out dates, and guest count, ensuring invalid inputs are
caught before submission.
|
|
||
| useEffect(() => { | ||
| const paramsLocation = searchParams.get("location") || ""; | ||
| const paramsGuests = Number.parseInt(searchParams.get("guests") || "2"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for guests parameter parsing.
The guests parameter parsing doesn't handle cases where the URL parameter might be an invalid number string.
Apply this diff to add error handling:
- const paramsGuests = Number.parseInt(searchParams.get("guests") || "2");
+ const guestsParam = searchParams.get("guests");
+ const paramsGuests = guestsParam ? Number.parseInt(guestsParam, 10) : 2;
+ const validGuests = Number.isNaN(paramsGuests) ? 2 : Math.max(1, paramsGuests);And update the state setting:
- setGuests(paramsGuests);
+ setGuests(validGuests);📝 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 paramsGuests = Number.parseInt(searchParams.get("guests") || "2"); | |
| const guestsParam = searchParams.get("guests"); | |
| const paramsGuests = guestsParam ? Number.parseInt(guestsParam, 10) : 2; | |
| const validGuests = Number.isNaN(paramsGuests) ? 2 : Math.max(1, paramsGuests); | |
| setGuests(validGuests); |
🤖 Prompt for AI Agents
In apps/web/src/components/search/SearchBar.tsx at line 75, the parsing of the
"guests" URL parameter does not handle invalid number strings, which can cause
unexpected behavior. Modify the code to check if the parsed number is a valid
integer and fallback to a default value (e.g., 2) if parsing fails or results in
NaN. This ensures robust error handling and prevents invalid input from
affecting the application state.
2 participants
Pull Request | StellarRent
📝 Summary
This PR implements the /search results page in line with the provided specifications. The page allows users to explore properties using various filters and sorting options. It is responsive, supports dark/light mode, and includes a map (via Leaflet) for spatial interaction.
🔗 Related Issues
Closes #(#86 ) (Replace with the actual issue number).
🔄 Changes Made
view detailson property cardℹ️ The map is integrated using the Leaflet library as an alternative to Mapbox or Google Maps due to integration constraints
📍 Route Behavior
🖼️ Current Output
🧪 Testing
If applicable, describe the tests performed. Include screenshots, test outputs, or any resources that help reviewers understand how the changes were tested.
✅ Testing Checklist
List any possible issues that might arise with this change.
🚀 Next Steps & Improvements
This change lays a solid foundation for further optimizations. Some areas that could benefit from future improvements include:
💬 Comments
Any additional context, questions, or considerations for reviewers.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style
Chores