Conversation
… add Service Pros detail and statement features.
WalkthroughAdds a new ProviderStatement component and protected route (/dashboard/reports/statement); updates ServiceProsDetail with report navigation plus Send Connection / Archive / Delete modals; converts tenant UIs to grid layouts and adds multi-select behavior to tenant transactions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 7
🧹 Nitpick comments (5)
src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx (2)
8-9: Duplicate comment.Line 8 and 9 have the same comment
// Mock Data - keyed by Service Pro ID. Remove one of them.🔎 Proposed fix
// Mock Data - keyed by Service Pro ID -// Mock Data - keyed by Service Pro ID const SERVICE_PRO_DETAILS: Record<number, any> = {
316-358: Modal accessibility improvements.Consider adding:
role="dialog"andaria-modal="true"to modal containersaria-labelledbypointing to the modal title- ESC key handler to close modals
- Focus trap to keep focus within the modal
This applies to all three modals (Send Connection, Archive, Delete).
src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx (3)
40-48: Filter dropdowns lack click-outside-to-close behavior.Unlike the action menu in
ServiceProsDetail.tsxwhich has a click-outside handler, these filter dropdowns only close when clicking the same filter button again. Consider adding a ref and click-outside listener for better UX.
189-199: Consider preventing all columns from being hidden.Users can currently hide all columns, leaving an empty table. Consider requiring at least one visible column.
🔎 Proposed fix
const toggleColumn = (columnId: ColumnId) => { setVisibleColumns(prev => { if (prev.includes(columnId)) { + // Prevent hiding the last column + if (prev.length === 1) return prev; return prev.filter(id => id !== columnId); } else { // Maintain order const newSet = new Set([...prev, columnId]); return ALL_COLUMNS.filter(col => newSet.has(col.id)).map(col => col.id); } }); };
507-536: Column modal lacks accessibility attributes.Add
role="dialog",aria-modal="true",aria-labelledby, and consider ESC key handling for accessibility compliance.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/App.tsx(2 hunks)src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx(1 hunks)src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx (1)
src/pages/basewebsite/resources/sections/resourceIcons.tsx (1)
Check(426-433)
🔇 Additional comments (3)
src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx (2)
384-390: Archive confirmation lacks actual archive logic.The "Yes I'm Sure" button only closes the modal without performing the archive operation. Ensure backend integration is planned.
426-431: Delete confirmation lacks actual delete logic.The Delete button only closes the modal. The warning message mentions removing from maintenance requests and deleting transactions, but no such logic exists.
src/App.tsx (1)
71-71: LGTM!Import follows the existing pattern for dashboard feature components.
| const parseDate = (dateStr: string) => { | ||
| if (!dateStr || dateStr === '-') return null; | ||
| return new Date(dateStr); | ||
| }; |
There was a problem hiding this comment.
Date parsing may fail for display format.
new Date("01 Dec 2024") relies on browser-specific parsing. The mock data uses "DD MMM YYYY" format which may not parse consistently across browsers.
🔎 Proposed fix with explicit parsing
const parseDate = (dateStr: string) => {
if (!dateStr || dateStr === '-') return null;
- return new Date(dateStr);
+ // Parse "DD MMM YYYY" format explicitly
+ const parts = dateStr.split(' ');
+ if (parts.length !== 3) return new Date(dateStr);
+ const months: Record<string, number> = {
+ 'Jan': 0, 'Feb': 1, 'Mar': 2, 'Apr': 3, 'May': 4, 'Jun': 5,
+ 'Jul': 6, 'Aug': 7, 'Sep': 8, 'Oct': 9, 'Nov': 10, 'Dec': 11
+ };
+ const day = parseInt(parts[0], 10);
+ const month = months[parts[1]];
+ const year = parseInt(parts[2], 10);
+ if (month === undefined || isNaN(day) || isNaN(year)) return new Date(dateStr);
+ return new Date(year, month, day);
};| <div className="bg-white px-4 py-2 rounded-full text-sm font-medium text-gray-700 cursor-pointer hover:bg-gray-50"> | ||
| Save Filters | ||
| </div> |
There was a problem hiding this comment.
"Save Filters" button has no functionality.
This button renders but has no onClick handler. Either implement filter persistence or remove until ready.
| {activeColumns.map(col => ( | ||
| <div key={col.id} className={col.hasSort ? "flex items-center gap-1 cursor-pointer" : ""}> | ||
| {col.label} | ||
| {col.hasSort && <ChevronUp className="w-3 h-3" />} | ||
| </div> |
There was a problem hiding this comment.
Sort indicators shown but sorting not implemented.
Columns display ChevronUp sort icons when hasSort: true, but clicking them does nothing. Either implement sorting or remove the icons until ready.
| const [isSendConnectionModalOpen, setIsSendConnectionModalOpen] = useState(false); | ||
| const [isArchiveModalOpen, setIsArchiveModalOpen] = useState(false); | ||
| const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); | ||
| const [isConnected, setIsConnected] = useState(false); | ||
| const actionMenuRef = useRef<HTMLDivElement>(null); | ||
|
|
||
| const handleSendConnection = () => { | ||
| setIsConnected(true); | ||
| setIsSendConnectionModalOpen(false); | ||
| }; |
There was a problem hiding this comment.
Connection state is ephemeral and email input is unused.
isConnectedis local state that resets on page refresh - consider persisting to backend or context.handleSendConnectiondoesn't capture/use the email entered in the modal. The email input on lines 333-337 is uncontrolled.
🔎 Proposed fix for controlled email input
const [isConnected, setIsConnected] = useState(false);
+const [connectionEmail, setConnectionEmail] = useState('');
const actionMenuRef = useRef<HTMLDivElement>(null);
const handleSendConnection = () => {
+ // TODO: Send connection request to backend with connectionEmail
+ console.log('Sending connection to:', connectionEmail);
setIsConnected(true);
setIsSendConnectionModalOpen(false);
+ setConnectionEmail('');
};And update the input:
<input
type="email"
placeholder="Enter email address"
+ value={connectionEmail}
+ onChange={(e) => setConnectionEmail(e.target.value)}
className="w-full px-4 py-3 rounded-lg border border-gray-200 focus:outline-none focus:border-[#3A6D6C]"
/>Committable suggestion skipped: line range outside the PR's diff.
| {/* View Profile Button */} | ||
| <button className="w-full bg-[#C8C8C8] text-gray-800 py-2.5 rounded-full text-sm font-semibold hover:bg-[#b8b8b8] transition-colors shadow-inner"> | ||
| View Profile | ||
| </button> |
There was a problem hiding this comment.
"View Profile" button has no click handler.
The button renders but doesn't navigate or perform any action. Consider adding an onClick handler or removing it until functionality is implemented.
🤖 Prompt for AI Agents
In src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx around lines
255 to 258, the "View Profile" button is rendered without any onClick handler so
it performs no action; add an onClick prop that navigates to the pro's profile
(e.g., use react-router's useNavigate to push to `/pros/{id}` or call a provided
openProfile handler) or, if navigation isn't ready, disable/remove the button;
ensure you pass the correct pro id from props/state and keep the existing
styling.
…nts to the dashboard.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/pages/Dashboard/features/Tenants/components/TenantRequestsSection.tsx (1)
33-44: Consider consolidating status color utilities.A similar
getStatusColorpattern exists insrc/pages/Dashboard/features/Units/utils.ts. While the status values differ, extracting a generic status-to-color mapper could improve maintainability and reduce duplication across the codebase.💡 Suggested approach for a shared utility
Create a shared utility in a common location:
// utils/statusColors.ts export const createStatusColorMapper = (colorMap: Record<string, string>) => { return (status: string, defaultColor = 'bg-gray-500') => { return colorMap[status.toLowerCase()] || defaultColor; }; };Then use it in this component:
-const getStatusColor = (status: string) => { - switch (status.toLowerCase()) { - case 'normal': - return 'bg-[#7BD747]'; - case 'urgent': - return 'bg-red-500'; - case 'low': - return 'bg-yellow-500'; - default: - return 'bg-gray-500'; - } -}; +const getStatusColor = createStatusColorMapper({ + 'normal': 'bg-[#7BD747]', + 'urgent': 'bg-red-500', + 'low': 'bg-yellow-500' +});src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx (2)
58-69: Consider consolidating status color utilities.Similar to
TenantRequestsSection.tsx, this component defines its owngetStatusColorfunction. Multiple components across the codebase follow this pattern (e.g.,src/pages/Dashboard/features/Units/utils.ts). A shared utility with configurable color types (text vs. background) would improve maintainability.
119-122: Consider extracting status color logic.Line 120 duplicates the status-to-color mapping already defined in
getStatusColor. While the dot requires background colors and the text requires text colors, this duplication makes maintenance harder. Consider a helper that returns both values.💡 Suggested approach
const getStatusColors = (status: string) => { const colorMap = { 'Paid': { text: 'text-[#7BD747]', bg: 'bg-[#7BD747]' }, 'Void': { text: 'text-red-500', bg: 'bg-red-500' }, 'Pending': { text: 'text-orange-500', bg: 'bg-orange-500' } }; return colorMap[status] || { text: 'text-gray-500', bg: 'bg-gray-500' }; }; // Usage: const statusColors = getStatusColors(transaction.status); <div className={`w-2 h-2 rounded-full ${statusColors.bg}`}></div> <span className={`text-sm font-medium ${statusColors.text}`}>{transaction.status}</span>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/Dashboard/features/Tenants/components/TenantRequestsSection.tsx(2 hunks)src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx (2)
src/pages/Dashboard/features/Units/utils.ts (1)
getStatusColor(6-17)src/pages/basewebsite/resources/sections/resourceIcons.tsx (1)
Check(426-433)
src/pages/Dashboard/features/Tenants/components/TenantRequestsSection.tsx (1)
src/pages/Dashboard/features/Units/utils.ts (1)
getStatusColor(6-17)
🔇 Additional comments (5)
src/pages/Dashboard/features/Tenants/components/TenantRequestsSection.tsx (2)
1-1: LGTM! Clean removal of unused import.The removal of the unused
Usericon import is good housekeeping and keeps the dependencies minimal.
47-93: Well-structured UI refactoring.The grid-based layout with responsive spacing and visual styling provides a clean, maintainable structure. The use of Tailwind utilities is appropriate, and the component styling aligns well with the broader dashboard design system mentioned in the PR objectives.
src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx (3)
40-56: Well-implemented selection state management.The multi-select functionality is correctly implemented with proper state management and toggle logic. The handlers appropriately manage both individual and bulk selection operations.
74-99: Well-structured table header with proper accessibility.The header implementation with select-all functionality is well done. The conditional rendering of the
Checkicon and the guard conditiontransactions.length > 0prevents showing a checked state on an empty list.Note: The
ChevronUpicons suggest sortable columns, but sorting logic is not yet implemented. This is acceptable if sorting is planned for a future iteration.
72-158: Excellent UI restructuring with consistent design patterns.The grid-based layout successfully mirrors the ServicePro style mentioned in the comments. The implementation demonstrates good attention to detail with:
- Consistent column definitions between header and body
- Proper hover states and transitions
- Clean separation of concerns with dedicated columns
- Responsive design considerations
The integration of multi-select functionality fits naturally into the new layout.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx (3)
107-110: Date parsing may fail for display format.The
parseDatefunction still relies onnew Date(dateStr)for parsing "DD MMM YYYY" format dates (e.g., "01 Dec 2024"), which may not parse consistently across browsers. Consider implementing explicit date parsing as suggested in the previous review.Does the JavaScript Date constructor reliably parse "DD MMM YYYY" format across all major browsers?
469-471: "Save Filters" button has no functionality.This button renders with hover effects but has no
onClickhandler. Either implement filter persistence or remove the button until the functionality is ready.
488-492: Sort indicators shown but sorting not implemented.Columns display
ChevronUpsort icons andcursor-pointerstyling whenhasSort: true, but clicking them does nothing. Either implement the sorting functionality or remove the visual indicators until ready.
🧹 Nitpick comments (1)
src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx (1)
133-136: Consider extracting status dot color logic to avoid duplication.Line 134 hardcodes the status-to-color mapping for the status indicator dot, while Line 135 reuses the
getStatusColorfunction for text color. Consider creating a helper function for background colors or extendinggetStatusColorto support both use cases.🔎 Proposed refactor
+const getStatusBgColor = (status: string) => { + switch (status) { + case 'Paid': + return 'bg-[#7BD747]'; + case 'Void': + return 'bg-red-500'; + case 'Pending': + return 'bg-orange-500'; + default: + return 'bg-gray-500'; + } +}; {/* Status Column */} <div className="flex items-center gap-2"> - <div className={`w-2 h-2 rounded-full ${transaction.status === 'Paid' ? 'bg-[#7BD747]' : transaction.status === 'Pending' ? 'bg-orange-500' : 'bg-red-500'}`}></div> + <div className={`w-2 h-2 rounded-full ${getStatusBgColor(transaction.status)}`}></div> <span className={`text-sm font-medium ${getStatusColor(transaction.status)}`}>{transaction.status}</span> </div>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/App.tsx(12 hunks)src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx(1 hunks)src/pages/Dashboard/features/Tenants/components/TenantRequestsSection.tsx(2 hunks)src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/Dashboard/features/Tenants/components/TenantRequestsSection.tsx
- src/App.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx (1)
src/pages/Dashboard/features/Units/utils.ts (1)
getStatusColor(6-17)
src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx (1)
src/pages/basewebsite/resources/sections/resourceIcons.tsx (1)
Check(426-433)
🔇 Additional comments (2)
src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx (1)
71-83: Excellent edge case handling in getInitials helper.The implementation properly addresses all the edge cases mentioned in the previous review:
- Returns
'?'for empty or falsy names- Trims and filters whitespace correctly
- Handles single-word names by returning the first two characters
- Handles multi-word names by returning the first character of the first two words
This is a solid implementation that prevents the issues identified earlier.
src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx (1)
148-200: CSV export issues have been properly addressed.The implementation now correctly:
- Exports
filteredStatementsinstead of all statements (Line 171)- Properly escapes CSV fields using
escapeCsvFieldto handle quotes, commas, and newlines (Lines 157-166)- Revokes the object URL after download to prevent memory leaks (Line 196)
All concerns from the previous review have been resolved.
1 participant
Summary by CodeRabbit
New Features
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.