Conversation
… professional pages.
WalkthroughThe PR adds comprehensive Add/Edit functionality for Service Pros with a new AddEditServicePro component accessible via dedicated routes, replacing modal-based workflows. Additionally, Tenant management UI receives styling and layout refinements including improved card design and responsive grid adjustments. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/pages/Dashboard/features/ServicePros/AddEditServicePro.tsx (1)
493-502: Expand country dropdown options or use a library.The country dropdown only includes USA and India, which is very limiting for a Service Pro management system that may need to support providers from various countries.
Consider using the
country-state-citylibrary (already used in AddEditTenant.tsx) for comprehensive country/state/city selection:+import { Country, State, City } from 'country-state-city'; +import type { ICountry } from 'country-state-city'; + +// In component: +const [countries, setCountries] = useState<ICountry[]>([]); + +useEffect(() => { + setCountries(Country.getAllCountries()); +}, []); + +const countryOptions = useMemo(() => { + return countries.map(country => ({ + value: country.isoCode, + label: country.name + })).sort((a, b) => a.label.localeCompare(b.label)); +}, [countries]); + <CustomDropdown value={formData.address.country} onChange={(value) => handleDropdownChange('address', 'country', value)} - options={[ - { value: 'USA', label: 'USA' }, - { value: 'India', label: 'India' } - ]} + options={countryOptions} placeholder="Country" + searchable={true} buttonClassName="w-full bg-white border border-gray-200 text-gray-800 px-6 py-2.5 rounded-lg outline-none focus:ring-2 focus:ring-[#3A6D6C]/20 transition-all font-medium" />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/App.tsx(2 hunks)src/pages/Dashboard/features/ServicePros/AddEditServicePro.tsx(1 hunks)src/pages/Dashboard/features/ServicePros/ServicePros.tsx(4 hunks)src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx(1 hunks)src/pages/Dashboard/features/ServicePros/components/ServiceProCard.tsx(1 hunks)src/pages/Dashboard/features/Tenants/AddEditTenant.tsx(9 hunks)src/pages/Dashboard/features/Tenants/Tenants.tsx(4 hunks)src/pages/Dashboard/features/Tenants/components/TenantCard.tsx(3 hunks)
🔇 Additional comments (16)
src/pages/Dashboard/features/ServicePros/components/ServiceProCard.tsx (1)
45-45: LGTM! Navigation-based edit flow implemented correctly.The Edit action now navigates to the dedicated edit route, consistent with the new AddEditServicePro component and routing structure introduced in this PR.
src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx (1)
193-193: LGTM! Consistent with the navigation-based edit pattern.The Edit action correctly navigates to the edit route, matching the implementation in ServiceProCard and leveraging the new AddEditServicePro component.
src/pages/Dashboard/features/Tenants/AddEditTenant.tsx (1)
562-562: Verify thatcircularCrop={false}produces the intended rounded-square output.The profile photo styling has been updated from circular (
rounded-full) to rounded-square (rounded-2xl), andcircularCropis nowfalse. Ensure the ImageCropModal component produces cropped images that align with therounded-2xldisplay.Run a quick visual test to confirm the cropped photo preview matches the rounded-2xl styling when saved.
Also applies to: 565-565, 586-586, 934-934
src/pages/Dashboard/features/ServicePros/ServicePros.tsx (3)
116-116: LGTM! Layout adjusted to support navigation-based flow.The flex column container enables proper alignment of pagination at the bottom via
mt-auto, improving the layout consistency.
133-133: LGTM! Modal replaced with navigation-based flow.The "Add service pro" button now correctly navigates to the dedicated add route, consistent with the new AddEditServicePro component.
183-189: LGTM! Pagination positioning improved.Wrapping pagination in a container with
mt-autoensures it stays at the bottom of the flex column layout.src/App.tsx (1)
67-67: LGTM! Routes correctly configured for Service Pros add/edit flow.The new routes follow the same pattern as the existing Tenants routes, using a shared AddEditServicePro component for both add and edit operations.
Also applies to: 171-172
src/pages/Dashboard/features/Tenants/Tenants.tsx (4)
103-103: LGTM! Layout adjusted to support pagination positioning.The flex column container enables proper alignment of pagination at the bottom, consistent with the Service Pros layout.
140-150: LGTM! Sort indicator enhanced with visual chevron.The explicit "Abc" label with rotating SVG chevron improves UX by making the sort direction more obvious.
180-180: LGTM! Grid layout adjusted to two columns.The responsive grid now shows 2 columns at large breakpoints instead of 3, likely to accommodate the larger TenantCard design.
198-204: LGTM! Pagination positioned at the bottom.The
mt-autoclass ensures pagination stays at the bottom of the flex column container.src/pages/Dashboard/features/Tenants/components/TenantCard.tsx (2)
11-11: LGTM! NewpropertyNameprop added correctly.The optional
propertyNameprop allows displaying rental information when available, enhancing the card's context.Also applies to: 20-20
62-141: LGTM! Card layout refactored for improved visual hierarchy.The UI changes enhance the card design with:
- Rounded-square profile images
- Redesigned context menu with improved accessibility
- Info pill displaying contact details
- Prominent action buttons
- Conditional rental information display
The refactor aligns with the PR objectives for improved card layout.
src/pages/Dashboard/features/ServicePros/AddEditServicePro.tsx (3)
299-313: LGTM! Validation and submit flow implemented correctly.The form validates required fields (firstName, lastName, category) and provides clear error feedback. The mock submit logic is appropriate for this stage of development.
244-270: LGTM! Dynamic email/phone fields implemented well.The implementation correctly limits additional fields to 2 each with add/remove controls, providing a clean UX for managing multiple contact methods.
273-282: Add memory leak prevention for blob URLs.The profile photo handler creates a blob URL via
FileReader.readAsDataURLbut doesn't track or revoke it. When users change the photo multiple times, blob URLs accumulate in memory.Apply this pattern from AddEditTenant.tsx to manage blob URL cleanup:
+const currentBlobUrlRef = useRef<string | null>(null); + const handleProfilePhotoSelect = (e: React.ChangeEvent<HTMLInputElement>) => { if (e.target.files && e.target.files[0]) { const file = e.target.files[0]; + + // Revoke previous blob URL + if (currentBlobUrlRef.current) { + URL.revokeObjectURL(currentBlobUrlRef.current); + } + const reader = new FileReader(); reader.onload = (ev) => { - if (ev.target?.result) setProfilePhoto(ev.target.result as string); + if (ev.target?.result) { + const blobUrl = ev.target.result as string; + currentBlobUrlRef.current = blobUrl; + setProfilePhoto(blobUrl); + } }; reader.readAsDataURL(file); } }; + +// Add cleanup effect +useEffect(() => { + return () => { + if (currentBlobUrlRef.current) { + URL.revokeObjectURL(currentBlobUrlRef.current); + } + }; +}, []);Likely an incorrect or invalid review comment.
| const handleProfilePhotoSelect = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| if (e.target.files && e.target.files[0]) { | ||
| const file = e.target.files[0]; | ||
| const reader = new FileReader(); | ||
| reader.onload = (ev) => { | ||
| if (ev.target?.result) setProfilePhoto(ev.target.result as string); | ||
| }; | ||
| reader.readAsDataURL(file); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add file validation for profile photo uploads.
The profile photo handler lacks validation for file type and size, which could lead to errors or poor UX if users upload inappropriate files.
Add validation similar to AddEditTenant.tsx:
const handleProfilePhotoSelect = (e: React.ChangeEvent<HTMLInputElement>) => {
if (e.target.files && e.target.files[0]) {
const file = e.target.files[0];
+
+ // Validate file type
+ const allowedTypes = ['image/jpeg', 'image/jpg', 'image/png', 'image/gif', 'image/webp'];
+ if (!allowedTypes.includes(file.type)) {
+ alert('Please select a valid image file (JPEG, PNG, GIF, or WebP)');
+ if (profileInputRef.current) profileInputRef.current.value = '';
+ return;
+ }
+
+ // Validate file size (max 10MB)
+ const maxSize = 10 * 1024 * 1024;
+ if (file.size > maxSize) {
+ alert('Image size must not exceed 10MB');
+ if (profileInputRef.current) profileInputRef.current.value = '';
+ return;
+ }
+
const reader = new FileReader();
reader.onload = (ev) => {
if (ev.target?.result) setProfilePhoto(ev.target.result as string);
};
reader.readAsDataURL(file);
}
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/Dashboard/features/ServicePros/AddEditServicePro.tsx around lines
273 to 282, the profile photo handler reads any file without validation; add
file type and size checks before creating the FileReader. Reject files that are
not images (check file.type startsWith "image/") and enforce a max size (e.g.,
2MB or match AddEditTenant.tsx), set an error state or show a toast when
invalid, clear the input selection, and only call reader.readAsDataURL(file)
after the file passes validation.
| key={tenant.id} | ||
| {...tenant} | ||
| image={tenant.image || ''} | ||
| propertyName="Sunset Apartments, Unit 4B" |
There was a problem hiding this comment.
Replace hardcoded propertyName with dynamic tenant data.
The property name is hardcoded as "Sunset Apartments, Unit 4B" for all tenants. This should be dynamically sourced from the tenant's lease or unit assignment data.
Consider updating the tenant data structure to include property/unit information:
-propertyName="Sunset Apartments, Unit 4B"
+propertyName={tenant.propertyName || tenant.unit?.property?.name || undefined}If the backend doesn't yet provide this information, verify whether the tenant object includes lease or unit relationships that can be used to derive the property name.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/Dashboard/features/Tenants/Tenants.tsx around line 187 the
propertyName is hardcoded to "Sunset Apartments, Unit 4B"; replace this static
string with the tenant's actual property/unit value (e.g. use
tenant.propertyName or derive it from tenant.unit?.property?.name or
tenant.lease?.unit?.property?.name depending on the shape of your data), add a
safe fallback (like "Unknown property") if the nested fields are missing, and if
the backend doesn't yet supply property/unit info update the tenant data fetch
or model to include lease/unit/property relationships so the component can
render the real property name.
1 participant
Summary by CodeRabbit
New Features
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.