-
Notifications
You must be signed in to change notification settings - Fork 76
feat(web): implement invitations page with search, filters (closes #191) #209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(web): implement invitations page with search, filters (closes #191) #209
Conversation
…tellar-Rent#191) Add search and status filtering with created/property sorting. Add pagination for mobile/desktop and a responsive mobile menu drawer. Resolve useUserRole merge conflicts and add loading state.
📝 WalkthroughWalkthroughThe pull request relocates RoleGuard from hooks to a dedicated component directory and refactors it with unified loading state and computed access logic. The invitations page is completely rewritten to include mobile-responsive navigation, real-time search/filtering, sorting, and pagination. Corresponding imports are updated across dashboards, and obsolete hooks are removed. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/app/invitations/page.tsx`:
- Around line 300-305: The JSX references an undefined symbol mobileInvitations
causing build/runtime errors; fix by declaring it before use (e.g., inside the
component scope, add a line like const mobileInvitations = invitations ?? []; or
derive it from an existing invitations array (const mobileInvitations =
(invitations || []).filter(...) if you need mobile-specific filtering) so the
mobile empty-state check and rendering in page.tsx can safely reference
mobileInvitations.
- Around line 45-53: Reset pagination when filters/search/sort change: after you
compute totalPages and desktopTotalPages (the values derived from the useMemo
filtered list), add a useEffect that watches searchQuery, statusFilter,
createdSort, propertySort (and optionally totalPages/desktopTotalPages) and call
setCurrentPage(1) and setDesktopPage(1) or clamp current values to
Math.min(currentPage, totalPages) / Math.min(desktopPage, desktopTotalPages).
This ensures currentPage/desktopPage (and their setters
setCurrentPage/setDesktopPage) are reset or clamped whenever the
filter/search/sort state changes so the pagination slice never points past
available pages.
🧹 Nitpick comments (1)
apps/web/src/app/invitations/page.tsx (1)
67-122: Extract a shared filter/sort helper to keep mobile & desktop logic in sync.
The twouseMemoblocks are identical, which risks divergence over time. Consider a single helper to reduce duplication.♻️ Suggested refactor
+ const filterAndSortInvitations = (invitations: Invitation[]) => { + const normalizedQuery = searchQuery.trim().toLowerCase(); + let filtered = invitations.filter((invite) => { + if (statusFilter !== 'all' && invite.status.toLowerCase() !== statusFilter) { + return false; + } + if (!normalizedQuery) return true; + return ( + invite.property.toLowerCase().includes(normalizedQuery) || + invite.owner.toLowerCase().includes(normalizedQuery) || + invite.status.toLowerCase().includes(normalizedQuery) || + invite.invitation.toLowerCase().includes(normalizedQuery) + ); + }); + + filtered = [...filtered].sort((left, right) => { + const propertyCompare = left.property.localeCompare(right.property); + if (propertyCompare !== 0) { + return propertySort === 'a-z' ? propertyCompare : -propertyCompare; + } + + const leftDate = new Date(left.created).getTime(); + const rightDate = new Date(right.created).getTime(); + return createdSort === 'newest' ? rightDate - leftDate : leftDate - rightDate; + }); + + return filtered; + }; + - const filteredDesktopInvitations = useMemo(() => { - const normalizedQuery = searchQuery.trim().toLowerCase(); - let filtered = DESKTOP_INVITATIONS.filter((invite) => { - if (statusFilter !== 'all' && invite.status.toLowerCase() !== statusFilter) { - return false; - } - if (!normalizedQuery) return true; - return ( - invite.property.toLowerCase().includes(normalizedQuery) || - invite.owner.toLowerCase().includes(normalizedQuery) || - invite.status.toLowerCase().includes(normalizedQuery) || - invite.invitation.toLowerCase().includes(normalizedQuery) - ); - }); - - filtered = [...filtered].sort((left, right) => { - const propertyCompare = left.property.localeCompare(right.property); - if (propertyCompare !== 0) { - return propertySort === 'a-z' ? propertyCompare : -propertyCompare; - } - - const leftDate = new Date(left.created).getTime(); - const rightDate = new Date(right.created).getTime(); - return createdSort === 'newest' ? rightDate - leftDate : leftDate - rightDate; - }); - - return filtered; - }, [createdSort, propertySort, searchQuery, statusFilter]); + const filteredDesktopInvitations = useMemo( + () => filterAndSortInvitations(DESKTOP_INVITATIONS), + [createdSort, propertySort, searchQuery, statusFilter] + ); - const filteredMobileInvitations = useMemo(() => { - const normalizedQuery = searchQuery.trim().toLowerCase(); - let filtered = MOBILE_INVITATIONS.filter((invite) => { - if (statusFilter !== 'all' && invite.status.toLowerCase() !== statusFilter) { - return false; - } - if (!normalizedQuery) return true; - return ( - invite.property.toLowerCase().includes(normalizedQuery) || - invite.owner.toLowerCase().includes(normalizedQuery) || - invite.status.toLowerCase().includes(normalizedQuery) || - invite.invitation.toLowerCase().includes(normalizedQuery) - ); - }); - - filtered = [...filtered].sort((left, right) => { - const propertyCompare = left.property.localeCompare(right.property); - if (propertyCompare !== 0) { - return propertySort === 'a-z' ? propertyCompare : -propertyCompare; - } - - const leftDate = new Date(left.created).getTime(); - const rightDate = new Date(right.created).getTime(); - return createdSort === 'newest' ? rightDate - leftDate : leftDate - rightDate; - }); - - return filtered; - }, [createdSort, propertySort, searchQuery, statusFilter]); + const filteredMobileInvitations = useMemo( + () => filterAndSortInvitations(MOBILE_INVITATIONS), + [createdSort, propertySort, searchQuery, statusFilter] + );
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/app/invitations/page.tsx`:
- Around line 300-304: The Tailwind arbitrary color class in the mobile
empty-state div uses invalid backticks (bg-[`#071429`]) so the background color
doesn't apply; update the class to use valid syntax (bg-[`#071429`]) in the JSX
element that renders the "No invitations sent yet" container (the div inside
filteredMobileInvitations.length === 0 branch) so Tailwind recognizes the color,
or alternatively replace it with an equivalent utility or inline style if
preferred.
♻️ Duplicate comments (1)
apps/web/src/app/invitations/page.tsx (1)
45-135: Reset pagination when filters/search/sort change.
Page indices can drift out of range after filtering/sorting, producing empty screens. Please reset or clamp on dependency changes.✅ Suggested fix
const [createdSort, setCreatedSort] = useState('newest'); const [propertySort, setPropertySort] = useState('a-z'); const mobileRowsPerPage = 5; const desktopRowsPerPage = 10; + + useEffect(() => { + setCurrentPage(1); + setDesktopPage(1); + }, [searchQuery, statusFilter, createdSort, propertySort]);
🧹 Nitpick comments (1)
apps/web/src/app/invitations/page.tsx (1)
67-122: Consider extracting shared filter/sort logic.
The mobile/desktop filtering blocks are identical—this is a good candidate for a helper to avoid divergence.Example sketch:
const filterAndSortInvitations = (items: Invitation[]) => { const normalizedQuery = searchQuery.trim().toLowerCase(); let filtered = items.filter((invite) => { /* same logic */ }); return [...filtered].sort((left, right) => { /* same sort */ }); }; const filteredDesktopInvitations = useMemo( () => filterAndSortInvitations(DESKTOP_INVITATIONS), [createdSort, propertySort, searchQuery, statusFilter] ); const filteredMobileInvitations = useMemo( () => filterAndSortInvitations(MOBILE_INVITATIONS), [createdSort, propertySort, searchQuery, statusFilter] );
885cb35 to
47a2097
Compare
respp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I appreciate your work on both Issue #191 and #201, please avoid combining multiple issues into a single PR in the future to keep the review process cleaner.
Currently, there is an integration conflict with RoleGuard: the existing code still references the old version in hooks/auth/, meaning your new component in components/guards/ isn't actually being used; please migrate all references to the new one and delete the legacy file.
Additionally, the invitations page needs refactoring to address a syntax error on line 303 of page.tsx, the removal of unnecessary empty constants (MOBILE_INVITATIONS/DESKTOP_INVITATIONS), and the cleanup of duplicated filtering logic between mobile and desktop.
Finally, please break down the 475-line component into smaller sub-components and delete the legacy useUserRole.ts mock to ensure we are using the new .tsx implementation consistently.
|
Hey there, you can take a look at the latest commit. I have fixed all you have pointed out. Thanks for your time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/dashboard/host-dashboard/page.tsx (1)
2-8: ImportBreadcrumbto avoid a build error.
<Breadcrumb />is used (Line 204) but not imported, which will fail compilation.🐛 Suggested fix
import NetworkStatus from '@/components/NetworkStatus'; import BookingHistory from '@/components/dashboard/BookingHistory'; import NotificationSystem from '@/components/dashboard/NotificationSystem'; import ProfileManagement from '@/components/dashboard/ProfileManagement'; import PropertyManagement from '@/components/dashboard/PropertyManagement'; +import Breadcrumb from '@/components/ui/breadcrumb'; import { RoleGuard } from '@/components/guards/RoleGuard';
🤖 Fix all issues with AI agents
In `@apps/web/src/app/invitations/page.tsx`:
- Around line 284-295: The SearchBar input is missing an accessible label;
update the SearchBar component to provide one by adding a visible or
programmatic label tied to the input (e.g., add a <label> with htmlFor and give
the input a unique id, or add aria-label/aria-labelledby) so screen readers can
announce the field; ensure the label text conveys "Search invitations" (or
similar) and keep the existing props/value/onChange handlers intact.
🧹 Nitpick comments (1)
apps/web/src/app/invitations/page.tsx (1)
24-24: Clamp page indices when the data set changes.
Handlers reset pages, but ifinvitationsrefresh asynchronously,mobilePage/desktopPagecan drift past the new totals and render empty results. Clamping on total change keeps pagination valid.♻️ Suggested fix
-import { useMemo, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react'; @@ const currentDesktopInvitations = filteredInvitations.slice( (desktopPage - 1) * DESKTOP_ROWS_PER_PAGE, desktopPage * DESKTOP_ROWS_PER_PAGE ); + + useEffect(() => { + setMobilePage((prev) => Math.min(prev, mobileTotalPages)); + setDesktopPage((prev) => Math.min(prev, desktopTotalPages)); + }, [mobileTotalPages, desktopTotalPages]);Also applies to: 101-116
| const SearchBar = ({ value, onChange }: { value: string; onChange: (value: string) => void }) => ( | ||
| <div className="relative w-full max-w-md"> | ||
| <span className="absolute left-3 top-1/2 -translate-y-1/2 text-muted-foreground"> | ||
| <Search className="h-4 w-4" /> | ||
| </span> | ||
| <input | ||
| type="text" | ||
| placeholder="Search an invitation..." | ||
| className="w-full rounded-full border border-gray-800 bg-background py-2 pl-9 pr-3 text-white placeholder:text-gray-400 focus:outline-none focus:ring-2 focus:ring-primary/60" | ||
| value={value} | ||
| onChange={(event) => onChange(event.target.value)} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an accessible label to the search input.
Without a label, screen readers may announce this as an unlabeled field.
🐛 Suggested fix
<input
type="text"
+ aria-label="Search invitations"
placeholder="Search an invitation..."
className="w-full rounded-full border border-gray-800 bg-background py-2 pl-9 pr-3 text-white placeholder:text-gray-400 focus:outline-none focus:ring-2 focus:ring-primary/60"
value={value}
onChange={(event) => onChange(event.target.value)}
/>📝 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 SearchBar = ({ value, onChange }: { value: string; onChange: (value: string) => void }) => ( | |
| <div className="relative w-full max-w-md"> | |
| <span className="absolute left-3 top-1/2 -translate-y-1/2 text-muted-foreground"> | |
| <Search className="h-4 w-4" /> | |
| </span> | |
| <input | |
| type="text" | |
| placeholder="Search an invitation..." | |
| className="w-full rounded-full border border-gray-800 bg-background py-2 pl-9 pr-3 text-white placeholder:text-gray-400 focus:outline-none focus:ring-2 focus:ring-primary/60" | |
| value={value} | |
| onChange={(event) => onChange(event.target.value)} | |
| /> | |
| const SearchBar = ({ value, onChange }: { value: string; onChange: (value: string) => void }) => ( | |
| <div className="relative w-full max-w-md"> | |
| <span className="absolute left-3 top-1/2 -translate-y-1/2 text-muted-foreground"> | |
| <Search className="h-4 w-4" /> | |
| </span> | |
| <input | |
| type="text" | |
| aria-label="Search invitations" | |
| placeholder="Search an invitation..." | |
| className="w-full rounded-full border border-gray-800 bg-background py-2 pl-9 pr-3 text-white placeholder:text-gray-400 focus:outline-none focus:ring-2 focus:ring-primary/60" | |
| value={value} | |
| onChange={(event) => onChange(event.target.value)} | |
| /> |
🤖 Prompt for AI Agents
In `@apps/web/src/app/invitations/page.tsx` around lines 284 - 295, The SearchBar
input is missing an accessible label; update the SearchBar component to provide
one by adding a visible or programmatic label tied to the input (e.g., add a
<label> with htmlFor and give the input a unique id, or add
aria-label/aria-labelledby) so screen readers can announce the field; ensure the
label text conveys "Search invitations" (or similar) and keep the existing
props/value/onChange handlers intact.
feat(web): implement invitations page with search, filters (closes #191)
feat(web): implement roleguard component (closes #201)
Add search and status filtering with created/property sorting. Add pagination for mobile/desktop and a responsive mobile menu drawer. Resolve useUserRole merge conflicts and add loading state.
Pull Request | StellarRent
📝 Summary
Implement invitations page with search, filters, and pagination.
Add comprehensive functionality to the invitations page, including search by property, owner, status, or invitation, status filtering (all, pending, accepted, declined, expired), sorting by created date and property name, pagination for mobile and desktop views, and a responsive mobile menu drawer. Also, resolve merge conflicts in useUserRole hook by consolidating code and adding loading state.
🔗 Related Issues
Closes #191 (Replace with the actual issue number).
🔄 Changes Made
Provide a general description of the changes. Include any relevant background information or context to help reviewers understand the purpose of this PR.
Added the invitations/page.tsx placeholder page based on spec provided in the figma file.
🖼️ Current Output
Provide visual evidence of the changes:
🧪 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
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.