Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces a comprehensive revamp of the property details feature. It adds a modular image gallery, interactive calendar, reviews section, and map component. The PropertyCard now visually indicates availability. The PropertyDetail component is refactored for richer data and interactivity. Supporting UI components and styles are added or updated for a cohesive experience. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PropertyPage
participant PropertyDetail
participant PropertyImageGallery
participant PropertyCalendar
participant PropertyMap
participant PropertyReviewsSection
User->>PropertyPage: Navigate to /property/[id]
PropertyPage->>PropertyDetail: Render with property id
PropertyDetail->>PropertyImageGallery: Show property images
PropertyDetail->>PropertyCalendar: Show availability and select dates
PropertyDetail->>PropertyMap: Show location and map info
PropertyDetail->>PropertyReviewsSection: Show reviews and ratings
User->>PropertyDetail: Interact (expand description, select dates, etc.)
PropertyDetail->>User: Display booking card and interactive UI
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✨ 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: 6
🧹 Nitpick comments (9)
apps/web/src/components/features/properties/PropertyImageGallery.tsx (1)
154-161: Improve thumbnail key generation for better uniqueness.The current key generation approach using regex replacement could lead to collisions if multiple images have similar URLs after sanitization.
Consider using a more robust key generation approach:
- // Create a unique key using image URL hash or fallback to index - const imageKey = `thumbnail-${image.replace(/[^a-zA-Z0-9]/g, '')}-${index}`; + // Create a unique key using index and a hash of the URL + const imageKey = `thumbnail-${index}-${btoa(image).slice(0, 8)}`;Alternatively, if the image URLs are stable, you could simply use the index:
- const imageKey = `thumbnail-${image.replace(/[^a-zA-Z0-9]/g, '')}-${index}`; + const imageKey = `thumbnail-${index}`;apps/web/src/components/features/properties/PropertyMap.tsx (1)
22-22: Remove or implement unused mapError state.The
mapErrorstate is initialized but never used (indicated by the_setMapErrornaming). Either implement error handling or remove this unused state.If error handling isn't needed yet:
- const [mapError, _setMapError] = useState(false);Or if you plan to implement error handling:
- const [mapError, _setMapError] = useState(false); + const [mapError, setMapError] = useState(false);And add proper error handling logic where needed.
apps/web/src/components/features/properties/PropertyCalendar.tsx (2)
105-146: Consider extracting complex date selection logic.The
handleDateClickfunction is quite complex and handles multiple scenarios. Consider extracting this logic into smaller, more focused functions for better maintainability and testing.+ const createNormalizedDate = (date: Date) => { + const normalized = new Date(date); + normalized.setHours(0, 0, 0, 0); + return normalized; + }; + + const calculateDaysDiff = (from: Date, to: Date) => { + return Math.ceil((to.getTime() - from.getTime()) / (1000 * 60 * 60 * 24)); + }; + + const handleNewSelection = (date: Date) => { + onDateSelect?.({ from: createNormalizedDate(date), to: undefined }); + }; + + const handleRangeSelection = (date: Date, existingFrom: Date) => { + const normalizedDate = createNormalizedDate(date); + const normalizedFrom = createNormalizedDate(existingFrom); + + // Rest of the logic... + }; const handleDateClick = (date: Date) => { if (getDateStatus(date) === 'unavailable') return; if (!selectedDates?.from || (selectedDates.from && selectedDates.to)) { - onDateSelect?.({ from: normalizedDate, to: undefined }); + handleNewSelection(date); return; } if (selectedDates.from && !selectedDates.to) { - // Complex logic here... + handleRangeSelection(date, selectedDates.from); } };
157-172: Consider internationalization for month and day names.The month and day names are hardcoded in English. For better internationalization support, consider using the browser's built-in localization or a localization library.
- const monthNames = [ - 'January', 'February', 'March', 'April', 'May', 'June', - 'July', 'August', 'September', 'October', 'November', 'December', - ]; - - const dayNames = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat']; + const monthNames = Array.from({ length: 12 }, (_, i) => + new Date(2000, i, 1).toLocaleDateString('en-US', { month: 'long' }) + ); + + const dayNames = Array.from({ length: 7 }, (_, i) => + new Date(2000, 0, i + 1).toLocaleDateString('en-US', { weekday: 'short' }) + );apps/web/src/components/features/properties/FeaturedProperties.tsx (1)
173-177: Consider using property data for quick info section.The guest capacity and bedroom count are hardcoded. Consider using actual property data or adding a TODO comment for future implementation.
If property data is available in the type definition, use it:
- <span>Up to 4 guests</span> - <span>2 bedrooms</span> + <span>Up to {property.maxGuests || 4} guests</span> + <span>{property.bedrooms || 2} bedrooms</span>Or add a TODO comment:
+ {/* TODO: Replace with actual property data when available */} <div className="flex items-center justify-between text-xs text-muted-foreground mb-3">apps/web/src/components/features/properties/PropertyReviewsSection.tsx (1)
1-7: Maintain consistent import style.The imports mix absolute paths (
@/components) and tilde aliases (~/components). Use a consistent approach.-import { Button } from '@/components/ui/button'; -import { Card } from '@/components/ui/card'; +import { Button } from '~/components/ui/button'; +import { Card } from '~/components/ui/card'; import { MessageCircle, Shield, Star, ThumbsUp } from 'lucide-react'; import { Badge } from '~/components/ui/badge';apps/web/src/components/features/properties/PropertyDetail.tsx (3)
119-121: Add more mock properties or TODO comment.Only one property is defined in the mock data, limiting testing capabilities.
Add a TODO comment or more mock properties:
}, - // Add more mock properties as needed + // TODO: Add more mock properties for testing different scenarios + '2': { + // ... property data + }, };
186-196: Add icon mapping for "Washer & dryer" amenity.The mock data includes "Washer & dryer" but it's not in the icon map, so it will show the default Shield icon.
Add the missing mapping:
const iconMap: Record<string, React.ReactNode> = { 'Wi-Fi': <Wifi className="w-5 h-5" />, 'Air conditioning': <Wind className="w-5 h-5" />, 'Fully equipped kitchen': <Utensils className="w-5 h-5" />, + 'Washer & dryer': <Home className="w-5 h-5" />, 'Free parking': <Car className="w-5 h-5" />, 'Smart TV': <Tv className="w-5 h-5" />, Pool: <Waves className="w-5 h-5" />, };
229-238: Consider fallback for browsers without share API.The share button uses the Web Share API but doesn't provide feedback when it's not supported.
Add fallback behavior:
<Button variant="outline" size="sm" onClick={() => - navigator.share?.({ title: property.title, url: window.location.href }) + navigator.share?.({ title: property.title, url: window.location.href }) || + navigator.clipboard.writeText(window.location.href).then(() => { + // You could show a toast notification here + console.log('Link copied to clipboard'); + }) } >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/src/app/property/[id]/page.tsx(2 hunks)apps/web/src/components/features/properties/FeaturedProperties.tsx(1 hunks)apps/web/src/components/features/properties/PropertyCalendar.tsx(1 hunks)apps/web/src/components/features/properties/PropertyDetail.tsx(3 hunks)apps/web/src/components/features/properties/PropertyImageGallery.tsx(1 hunks)apps/web/src/components/features/properties/PropertyMap.tsx(1 hunks)apps/web/src/components/features/properties/PropertyReviewsSection.tsx(1 hunks)apps/web/src/components/ui/badge.tsx(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
apps/web/src/app/property/[id]/page.tsx (1)
apps/backend/src/services/property.service.ts (1)
getPropertyById(218-301)
apps/web/src/components/ui/badge.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/features/properties/PropertyReviewsSection.tsx (3)
apps/web/src/components/ui/badge.tsx (1)
Badge(33-33)apps/web/src/components/ui/card.tsx (1)
Card(85-85)apps/web/src/components/ui/button.tsx (1)
Button(59-59)
apps/web/src/components/features/properties/PropertyMap.tsx (2)
apps/web/src/components/ui/card.tsx (1)
Card(85-85)apps/web/src/components/ui/button.tsx (1)
Button(59-59)
apps/web/src/components/features/properties/PropertyCalendar.tsx (2)
apps/web/src/components/ui/card.tsx (1)
Card(85-85)apps/web/src/components/ui/button.tsx (1)
Button(59-59)
🔇 Additional comments (11)
apps/web/src/components/ui/badge.tsx (1)
1-34: LGTM! Well-implemented Badge component.The Badge component follows excellent patterns with proper TypeScript typing, variant-based styling using class-variance-authority, and good accessibility features including focus ring styling. The implementation is clean and reusable.
apps/web/src/components/features/properties/PropertyImageGallery.tsx (1)
29-51: LGTM! Excellent keyboard navigation implementation.The keyboard event handling is well-implemented with proper cleanup, preventDefault calls, and modal state checking. The useEffect dependency array correctly includes all necessary dependencies.
apps/web/src/components/features/properties/PropertyMap.tsx (1)
25-33: LGTM! Proper URL encoding for Google Maps integration.The Google Maps URL generation functions correctly encode the address parameter to handle special characters and spaces safely.
apps/web/src/components/features/properties/PropertyCalendar.tsx (1)
33-51: LGTM! Excellent use of useMemo for calendar optimization.The calendar days generation is properly memoized and efficiently creates a 6-week grid. The dependency array correctly includes only
currentMonth.apps/web/src/app/property/[id]/page.tsx (2)
12-12: LGTM! Correct Next.js 15 async params implementation.The change from synchronous
{ id: string }toPromise<{ id: string }>correctly reflects the Next.js 15 pattern for dynamic route parameters.
16-16: LGTM! Proper async handling of params.Both functions correctly await the params promise before destructuring the
idvalue, maintaining the existing functionality while supporting the new async parameter pattern.Also applies to: 27-27
apps/web/src/components/features/properties/FeaturedProperties.tsx (1)
124-159: Well-implemented UI enhancements!The hover effects, image optimizations, and overlay gradients improve the user experience. Good use of Next.js Image component features.
apps/web/src/components/features/properties/PropertyReviewsSection.tsx (1)
72-211: Excellent component implementation!Well-structured component with:
- Proper accessibility with semantic HTML
- Clear empty states
- Good visual hierarchy
- Blockchain verification emphasis aligns with PR objectives
apps/web/src/components/features/properties/PropertyDetail.tsx (3)
3-4: Good use of TypeScript type imports!Using
import typefor type-only imports helps with tree-shaking and build optimization.
165-184: Well-implemented booking calculations!The date calculations and cost breakdown are correctly implemented with clear structure.
394-429: Excellent accessibility implementation!Great use of:
- Proper label associations
- ARIA labels for buttons
- aria-live for dynamic content updates
- Semantic HTML structure
| }, | ||
| "devDependencies": { | ||
| "@biomejs/biome": "1.9.4", | ||
| "@biomejs/biome": "^1.9.4", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Loosened version constraint may reduce build determinism—confirm lock-file coverage
Changing @biomejs/biome from an exact version to a caret range will auto-upgrade to future minor/patch releases. While this keeps tooling current, it can also introduce unexpected lint/format differences and impair reproducibility if the lock-file isn’t committed or regenerated in CI.
Please verify that:
- A lock-file (
bun.lockb,package-lock.json, etc.) is checked into the repo. - CI regenerates/validates the lock-file to catch breaking changes early.
Run this quick check:
🏁 Script executed:
#!/bin/bash
# Verify a lock-file exists and pins a specific biome version.
fd -H bun.lockb -t f || echo "⚠️ bun.lockb not found"
grep -A2 '"@biomejs/biome"' bun.lockb | head -n 5Length of output: 117
Here’s a broader check across all common lock-file formats:
🏁 Script executed:
#!/bin/bash
for lockfile in bun.lockb package-lock.json yarn.lock pnpm-lock.yaml; do
if [ -f "$lockfile" ]; then
echo "🔍 Found $lockfile – scanning for @biomejs/biome…
"
grep -R "@biomejs/biome" "$lockfile" || echo "⚠️ No reference to @biomejs/biome in $lockfile"
else
echo "⚠️ $lockfile not found"
fi
doneLength of output: 634
Missing lockfile: build reproducibility risk
Loosening @biomejs/biome to a caret range (^1.9.4) means future minor/patch upgrades will be pulled in automatically, which can introduce formatting or lint changes and break builds if no lock-file pins dependencies.
Checks across all common lock-file formats found none committed:
⚠️ bun.lockb not found⚠️ package-lock.json not found⚠️ yarn.lock not found⚠️ pnpm-lock.yaml not found
Please ensure:
- A lock-file is checked into version control (e.g.,
bun.lockb,package-lock.json,yarn.lock, orpnpm-lock.yaml). - CI validates or regenerates the lock-file on every run to catch unexpected upgrades early.
🤖 Prompt for AI Agents
In package.json at line 22, the dependency version for @biomejs/biome is
specified with a caret range (^1.9.4) but no lockfile is committed, risking
build reproducibility. To fix this, generate and commit a lockfile appropriate
for your package manager (e.g., package-lock.json for npm, yarn.lock for Yarn,
pnpm-lock.yaml for pnpm, or bun.lockb for Bun) to version control. Additionally,
update your CI configuration to validate or regenerate this lockfile on each run
to detect unexpected dependency upgrades early.
apps/web/src/components/features/properties/PropertyImageGallery.tsx
Outdated
Show resolved
Hide resolved
| // // Fallback loading component | ||
| // const PropertyCardSkeleton = () => ( | ||
| // <Card className="overflow-hidden h-full"> | ||
| // <div className="h-48 w-full bg-muted animate-pulse" /> | ||
| // <div className="p-4"> | ||
| // <div className="h-6 bg-muted animate-pulse rounded mb-2" /> | ||
| // <div className="h-4 bg-muted animate-pulse rounded mb-4 w-3/4" /> | ||
| // <div className="h-10 bg-muted animate-pulse rounded" /> | ||
| // </div> | ||
| // </Card> | ||
| // ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or restore the skeleton loader.
The skeleton loader is commented out without explanation. Loading states improve perceived performance.
Either remove the commented code entirely or restore it for better UX:
-// // Fallback loading component
-// const PropertyCardSkeleton = () => (
-// <Card className="overflow-hidden h-full">
-// <div className="h-48 w-full bg-muted animate-pulse" />
-// <div className="p-4">
-// <div className="h-6 bg-muted animate-pulse rounded mb-2" />
-// <div className="h-4 bg-muted animate-pulse rounded mb-4 w-3/4" />
-// <div className="h-10 bg-muted animate-pulse rounded" />
-// </div>
-// </Card>
-// );Or restore it and use it for loading states when fetching real data.
📝 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.
| // // Fallback loading component | |
| // const PropertyCardSkeleton = () => ( | |
| // <Card className="overflow-hidden h-full"> | |
| // <div className="h-48 w-full bg-muted animate-pulse" /> | |
| // <div className="p-4"> | |
| // <div className="h-6 bg-muted animate-pulse rounded mb-2" /> | |
| // <div className="h-4 bg-muted animate-pulse rounded mb-4 w-3/4" /> | |
| // <div className="h-10 bg-muted animate-pulse rounded" /> | |
| // </div> | |
| // </Card> | |
| // ); |
🤖 Prompt for AI Agents
In apps/web/src/components/features/properties/FeaturedProperties.tsx around
lines 190 to 200, the PropertyCardSkeleton component is commented out, which
removes the loading state UI. To fix this, either delete the commented code
entirely if the skeleton loader is not needed, or uncomment and restore the
PropertyCardSkeleton component and integrate it into the loading state rendering
logic to improve user experience during data fetching.
apps/web/src/components/features/properties/FeaturedProperties.tsx
Outdated
Show resolved
Hide resolved
|
@respp please review. |
|
Hey @respp |
respp
left a comment
respp
left a comment
There was a problem hiding this comment.
Hey @CMI-James great job on this contribution, the property page looks really good
This could be ready to merge after addressing a couple of small things:
Just fix the messages flagged by Code Rabbit to avoid potential silent bugs down the line
The key used in the thumbnail component works, but it could be simplified a bit
Once that’s done, we’re good to go. Keep it up
Thanks for the review. I will work on the requested changes. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/web/src/components/features/properties/PropertyDetail.tsx (1)
3-3: Remove unused React type import.The
Reacttype import is not used in the component.-import type React from "react";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
apps/web/src/components/features/properties/FeaturedProperties.tsx(1 hunks)apps/web/src/components/features/properties/PropertyCalendar.tsx(1 hunks)apps/web/src/components/features/properties/PropertyDetail.tsx(2 hunks)apps/web/src/components/features/properties/PropertyImageGallery.tsx(1 hunks)apps/web/src/components/features/properties/PropertyMap.tsx(1 hunks)apps/web/src/components/features/properties/PropertyReviewsSection.tsx(1 hunks)apps/web/src/lib/data/properties.ts(1 hunks)apps/web/src/lib/types/property.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/lib/data/properties.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/features/properties/PropertyCalendar.tsx
- apps/web/src/components/features/properties/PropertyReviewsSection.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/web/src/components/features/properties/PropertyMap.tsx (3)
apps/web/src/lib/types/property.ts (1)
PropertyMapProps(74-79)apps/web/src/components/ui/card.tsx (1)
Card(85-85)apps/web/src/components/ui/button.tsx (1)
Button(59-59)
apps/web/src/components/features/properties/PropertyDetail.tsx (9)
apps/web/src/lib/types/property.ts (1)
PropertyDetailProps(70-72)apps/web/src/lib/data/properties.ts (1)
getPropertyById(327-329)apps/web/src/components/ui/button.tsx (1)
Button(59-59)apps/web/src/components/features/properties/PropertyImageGallery.tsx (1)
PropertyImageGallery(10-256)apps/web/src/components/ui/card.tsx (1)
Card(85-85)apps/web/src/components/features/properties/PropertyCalendar.tsx (1)
PropertyCalendar(16-333)apps/web/src/components/features/properties/PropertyMap.tsx (1)
PropertyMap(9-152)apps/web/src/components/features/properties/PropertyReviewsSection.tsx (1)
PropertyReviewsSection(10-198)apps/web/src/components/ui/badge.tsx (1)
Badge(33-33)
apps/web/src/components/features/properties/PropertyImageGallery.tsx (3)
apps/web/src/lib/types/property.ts (1)
PropertyImageGalleryProps(96-100)apps/web/src/components/ui/button.tsx (1)
Button(59-59)apps/web/src/components/ui/dialog.tsx (3)
Dialog(112-112)DialogContent(117-117)DialogTitle(120-120)
🪛 Biome (1.9.4)
apps/web/src/components/features/properties/PropertyImageGallery.tsx
[error] 162-162: 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/lib/types/property.ts
[error] 90-90: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 91-91: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🔇 Additional comments (9)
apps/web/src/components/features/properties/FeaturedProperties.tsx (2)
22-22: LGTM! Availability status is now deterministic.The use of
getAvailabilityStatus(property.id)resolves the previous issue where random status generation caused UI flickering on each render.
128-147: Excellent refactor to dynamic data.The transition from hardcoded mock data to
getFeaturedProperties()provides better consistency across the application and sets up the component for real data integration.apps/web/src/components/features/properties/PropertyImageGallery.tsx (1)
28-50: Excellent keyboard navigation implementation.The keyboard event handling for arrow keys and escape provides great accessibility and user experience.
apps/web/src/components/features/properties/PropertyMap.tsx (3)
12-19: Neighborhood data is now configurable via props.Good improvement from the previous hardcoded approach. The default values provide sensible fallbacks while allowing dynamic data to be passed in.
68-87: Placeholder map implementation is appropriate for current scope.The gradient placeholder with coordinates display provides a good interim solution until full map integration is implemented.
22-22: Unused state variable is properly indicated.The underscore prefix on
_setMapErrorcorrectly indicates this is intentionally unused, likely reserved for future error handling implementation.apps/web/src/lib/types/property.ts (1)
1-103: Excellent type organization and comprehensive coverage.The type definitions provide strong typing for the entire property feature set, promoting type safety and consistency across components.
apps/web/src/components/features/properties/PropertyDetail.tsx (2)
160-292: Excellent component integration and responsive design.The integration of PropertyImageGallery, PropertyCalendar, PropertyMap, and PropertyReviewsSection creates a comprehensive property detail experience. The responsive layout with mobile-specific calendar placement shows thoughtful UX consideration.
68-88: Well-implemented calculation helpers.The
calculateNightsandcalculateTotalfunctions provide clean separation of concerns and make the pricing logic easy to understand and maintain.
|
|
||
| const getImageSrc = (image: string, index: number) => { | ||
| if (imageErrors.has(index)) { | ||
| return "/images/house1.jpg?height=400&width=600"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify fallback image exists.
The component uses /images/house1.jpg as fallback when images fail to load. Ensure this file exists in the public directory to prevent broken image displays.
Also applies to: 86-86, 173-173, 202-202
🏁 Script executed:
#!/bin/bash
# Check if the fallback image exists
if [ -f "public/images/house1.jpg" ]; then
echo "Fallback image exists"
else
echo "Fallback image not found at public/images/house1.jpg"
echo "Looking for alternative images in public/images/:"
fd -t f "house1" public/images/ 2>/dev/null || echo "No house1 images found"
fiLength of output: 348
Fallback image missing – add or correct /images/house1.jpg
The component’s hard-coded fallback at /images/house1.jpg can’t be found in public/images/ (checked via shell; no matching files). Without it, any failed load will show a broken image.
Please address this in apps/web/src/components/features/properties/PropertyImageGallery.tsx at:
- Line 58
- Line 86
- Line 173
- Line 202
Either:
- Add
house1.jpgtopublic/images/ - Or update the returned path to point at an existing image resource
🤖 Prompt for AI Agents
In apps/web/src/components/features/properties/PropertyImageGallery.tsx at lines
58, 86, 173, and 202, the fallback image path "/images/house1.jpg" is used but
the file does not exist in public/images/. To fix this, either add the missing
house1.jpg image file to the public/images/ directory or update all fallback
image paths at these lines to point to an existing image resource that is
available in the public/images/ folder.
apps/web/src/components/features/properties/PropertyImageGallery.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/src/components/features/properties/PropertyDetail.tsx (3)
76-77: Consider making fees configurable instead of hardcoded.The cleaning fee and service fee are hardcoded values that should ideally be configurable or come from the property data.
Consider moving these to property data or configuration:
- const cleaningFee = 150; - const serviceFee = 100; + const cleaningFee = property.fees?.cleaning ?? 150; + const serviceFee = property.fees?.service ?? 100;This would require updating the property type definition to include optional fees.
101-385: Consider breaking down this large component into smaller, focused components.The component is quite large (285 lines of JSX) and handles multiple responsibilities. Consider extracting some sections into separate components for better maintainability and reusability.
Consider extracting these sections into separate components:
PropertyHeader(lines 113-155)PropertyInfoGrid(lines 164-183)PropertyDescription(lines 186-212)PropertyAmenities(lines 215-227)PropertyPolicies(lines 230-251)BookingCard(lines 280-380)This would make the component more modular and easier to test and maintain.
310-341: Enhance accessibility for the guest selector.The guest selector has good accessibility attributes but could be improved with better labeling and keyboard navigation support.
Consider these accessibility improvements:
<label htmlFor="guest-selector" className="block text-sm font-medium mb-2"> Guests </label> <div id="guest-selector" className="flex items-center justify-between border rounded-lg p-3" + role="group" + aria-labelledby="guest-selector-label" > + <span id="guest-selector-label">Guests</span> - <span>Guests</span> <div className="flex items-center gap-3"> <Button variant="outline" size="sm" onClick={() => setGuests(Math.max(1, guests - 1))} disabled={guests <= 1} aria-label="Decrease number of guests" + aria-describedby="guest-count" > - </Button> <span + id="guest-count" className="w-8 text-center" aria-live="polite" aria-label={`${guests} guests selected`} > {guests} </span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/components/features/properties/PropertyDetail.tsx(2 hunks)apps/web/src/components/features/properties/PropertyImageGallery.tsx(1 hunks)apps/web/src/lib/types/property.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/features/properties/PropertyImageGallery.tsx
- apps/web/src/lib/types/property.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/src/components/features/properties/PropertyDetail.tsx (3)
74-86: Consider making fee calculations configurable.The hardcoded cleaning fee ($150) and service fee ($100) values reduce flexibility and make the component less reusable. These values should ideally come from property data or configuration.
Consider making fees configurable:
const calculateTotal = () => { const nights = calculateNights(); const subtotal = property.price * nights; - const cleaningFee = 150; - const serviceFee = 100; + const cleaningFee = property.fees?.cleaning || 150; + const serviceFee = property.fees?.service || 100; return { nights, subtotal, cleaningFee, serviceFee, total: subtotal + cleaningFee + serviceFee, }; };
88-98: Improve amenity icon fallback logic.The fallback Shield icon for unknown amenities may not be semantically appropriate. Consider using a more generic icon or implementing a "no icon" state.
Consider using a more appropriate fallback:
const getAmenityIcon = (amenity: string) => { const iconMap: Record<string, React.ReactNode> = { 'Wi-Fi': <Wifi className="w-5 h-5" />, 'Air conditioning': <Wind className="w-5 h-5" />, 'Fully equipped kitchen': <Utensils className="w-5 h-5" />, 'Free parking': <Car className="w-5 h-5" />, 'Smart TV': <Tv className="w-5 h-5" />, Pool: <Waves className="w-5 h-5" />, }; - return iconMap[amenity] || <Shield className="w-5 h-5" />; + return iconMap[amenity] || <div className="w-5 h-5 rounded-full bg-gray-200 dark:bg-gray-700" />; };
37-392: Consider breaking down the large component into smaller, focused components.The PropertyDetail component is quite large (355+ lines) and handles multiple responsibilities. Consider extracting some sections into separate components for better maintainability and reusability.
Consider extracting these sections into separate components:
- Property header with title, rating, and action buttons
- Property info cards (guests, bedrooms, bathrooms, payment)
- Booking card with calendar and pricing
- Policies section
This would improve code organization and make testing easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/components/features/properties/PropertyDetail.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/src/components/features/properties/PropertyDetail.tsx (9)
apps/web/src/lib/types/property.ts (1)
PropertyDetailProps(72-74)apps/web/src/lib/data/properties.ts (1)
getPropertyById(327-329)apps/web/src/components/ui/button.tsx (1)
Button(59-59)apps/web/src/components/features/properties/PropertyImageGallery.tsx (1)
PropertyImageGallery(10-241)apps/web/src/components/ui/card.tsx (1)
Card(85-85)apps/web/src/components/features/properties/PropertyCalendar.tsx (1)
PropertyCalendar(16-333)apps/web/src/components/features/properties/PropertyMap.tsx (1)
PropertyMap(9-152)apps/web/src/components/features/properties/PropertyReviewsSection.tsx (1)
PropertyReviewsSection(10-198)apps/web/src/components/ui/badge.tsx (1)
Badge(33-33)
🔇 Additional comments (3)
apps/web/src/components/features/properties/PropertyDetail.tsx (3)
134-148: Great improvement on share functionality error handling.The share functionality now properly handles feature detection and includes error handling as suggested in previous reviews. This addresses the potential runtime errors in unsupported browsers.
182-182: Good use of Bath icon component.The Bath icon is now properly imported and used instead of an emoji, which addresses the previous review comment about consistency and cross-platform rendering.
324-325: Add validation for guest count against property maximum.The current validation only checks against 1 as minimum, but should also validate against the property's maximum guest capacity during decrement to prevent inconsistent state.
<Button variant="outline" size="sm" - onClick={() => setGuests(Math.max(1, guests - 1))} + onClick={() => { + const newGuests = Math.max(1, guests - 1); + if (newGuests <= property.maxGuests) { + setGuests(newGuests); + } + }} disabled={guests <= 1} aria-label="Decrease number of guests" >Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/src/components/features/properties/PropertyDetail.tsx (2)
116-116: Optimize cost calculation with useMemo.The
calculateTotal()function is called on every render, which could impact performance. Consider memoizing this calculation.+ import { useMemo } from 'react'; // ... other code ... - const costs = calculateTotal(); + const costs = useMemo(() => calculateTotal(), [selectedDates, property.price]);
118-390: Consider breaking down this large component.The component is well-structured and functional, but at 390+ lines, it could benefit from being split into smaller, more focused components for better maintainability and reusability.
Consider extracting these sections into separate components:
- Property header with actions (lines 130-161)
- Property info cards (lines 170-187)
- Property description section (lines 189-216)
- Property amenities section (lines 218-231)
- Property policies section (lines 233-255)
- Booking card (lines 282-384)
This would make the component more modular and easier to test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/package.json(1 hunks)apps/web/src/components/features/properties/PropertyDetail.tsx(2 hunks)apps/web/src/components/features/properties/PropertyImageGallery.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/components/features/properties/PropertyImageGallery.tsx
🔇 Additional comments (6)
apps/web/src/components/features/properties/PropertyDetail.tsx (6)
3-36: Well-organized imports with proper TypeScript patterns.The imports are comprehensive and properly structured. Good use of
import typefor type-only imports and logical grouping of related imports.
38-44: Clean state management with proper TypeScript typing.The state variables are well-organized and properly typed. The synchronous property fetching is appropriate for the current mock data implementation.
89-99: Well-implemented amenity icon mapping.Good use of Record type for type-safe icon mapping with a fallback icon for unmapped amenities.
100-115: Excellent share functionality with proper error handling.The share functionality correctly implements feature detection, proper error handling, and user feedback via toast notifications. This addresses all the concerns from previous reviews.
308-346: Excellent accessibility implementation.Great use of accessibility attributes like
aria-label,aria-live, andhtmlForfor the guest selector controls. This ensures the component is usable by screen readers and other assistive technologies.
372-378: Good UX with disabled state and clear call-to-action.The booking button correctly disables when dates aren't selected and uses a clear, branded color scheme. The Link integration ensures proper navigation.
|
Hey, @respp done, Kindly review. |
|
@respp, kindly review. |
2 participants
Pull Request | StellarRent
📝 Summary
Provide a brief description of what this PR accomplishes.
🔗 Related Issues
Closes #87 .
🔄 Changes Made
Implemented a complete, responsive Property Detail page with all essential components to enhance the user experience and prepare for future blockchain integration. This includes:
==
🖼️ Current Output
Provide visual evidence of the changes:
https://www.loom.com/share/8a3fb93476a94b2a9fd34dc2f3b29761?sid=15c73797-981e-4d25-b552-8590ef351e68
🧪 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
New Features
Enhancements
Chores