Repository navigation
feat(web): invite system frontend & app refactor - #15
Conversation
- Refactored App.tsx into TenantRoutes and AdminRoutes - Implemented InviteMemberDialog with Refine hooks - Added shadcn components (dialog, form, toast) - Beautified UI with gradients - Unified UserList component - Cleaned up lint errors (Strict Mode)
📝 WalkthroughWalkthroughAdds a Radix-based UI component library (forms, dialog, select, label, toast), an in-memory toast hook, tenant/admin route modules and layouts, a tenant auth provider, and an invite-member dialog with react-hook-form/zod; also updates app dependencies and App.tsx routing to use TenantRoutes/AdminRoutes and Toaster. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InviteDialog as InviteMemberDialog (UI)
participant FormLib as react-hook-form/Controller
participant API as useCustomMutation / authClient
participant ToastHook as toast / useToast
participant ToasterUI as Toaster / ToastProvider
User->>InviteDialog: Open dialog, submit email+role
InviteDialog->>FormLib: validate (zod) and get values
FormLib->>InviteDialog: validated values
InviteDialog->>API: POST /invitations (mutation)
API-->>InviteDialog: success / error
InviteDialog->>ToastHook: toast({ title, description, variant })
ToastHook->>ToasterUI: enqueue toast
ToasterUI->>User: render toast, auto-dismiss
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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
🤖 Fix all issues with AI agents
In @apps/web/src/hooks/use-toast.ts:
- Around line 169-180: The useEffect in useToast currently depends on state
causing setState to be re-subscribed on every state change; update the effect so
it only runs once on mount by changing the dependency array to [] (keep the
existing listener push and cleanup logic that finds and splices setState from
listeners so the listener is removed on unmount). Ensure you reference the
useToast function and the setState listener when making this change.
In @apps/web/src/routes/TenantRoutes.tsx:
- Around line 47-52: The Settings nav item points to /dashboard/settings but the
route is commented out; either remove the Settings entry from the navGroups used
by TenantLayout or restore/add a route for it by uncommenting or adding a Route
with path="settings" and element={<SettingsPage />} (or a lightweight
placeholder component like SettingsPlaceholder) inside the <Routes> block that
wraps <TenantLayout> so the nav link resolves to a valid component.
🧹 Nitpick comments (13)
apps/web/src/providers/tenant-auth-provider.ts (2)
49-63: Consider adding proper typing for the user object instead of usingas any.The
as anycast bypasses TypeScript's type checking. Consider defining an interface for the expected user shape fromauthClient.getSession()to maintain type safety.Suggested approach
// Define the expected user shape interface SessionUser { id: string; name?: string; image?: string; roles?: string[]; role?: string; organizationId?: string; } // Then in getIdentity: const user = data.user as SessionUser;
33-48: Consider adding error handling forgetSession()call.If
authClient.getSession()throws (e.g., network error, service unavailable), the error will propagate unhandled. Consider wrapping in try/catch and returningauthenticated: falseon failure.Defensive error handling
check: async () => { + try { const session = await authClient.getSession(); if (!session.data) { return { authenticated: false, redirectTo: "/login", }; } return { authenticated: true, }; + } catch { + return { + authenticated: false, + redirectTo: "/login", + }; + } },apps/web/src/routes/AdminRoutes.tsx (1)
51-51: Placeholder for settings route.This is fine for now, but consider adding a TODO comment or tracking this in an issue to ensure it's implemented before release.
Would you like me to open an issue to track the settings page implementation?
apps/web/src/routes/TenantRoutes.tsx (1)
13-22: Consider memoizing or extractingnavGroups.The
navGroupsarray is recreated on every render. Since it's static, extract it outside the component or wrap inuseMemoto avoid unnecessary object allocations.♻️ Suggested refactor
+const navGroups = [ + { + title: "", + items: [ + { label: 'Dashboard', href: '/dashboard', icon: LayoutDashboard }, + { label: 'Users', href: '/dashboard/users', icon: Users }, + { label: 'Settings', href: '/dashboard/settings', icon: Settings }, + ] + } +]; + export function TenantRoutes() { - const navGroups = [ - { - title: "", - items: [ - { label: 'Dashboard', href: '/dashboard', icon: LayoutDashboard }, - { label: 'Users', href: '/dashboard/users', icon: Users }, - { label: 'Settings', href: '/dashboard/settings', icon: Settings }, - ] - } - ]; - return (apps/web/src/pages/DashboardPage.tsx (1)
71-81: Trend icon direction is always "up" regardless oftrendUpvalue.The
TrendingUpicon is rendered unconditionally (line 77), but the styling switches between green and orange based ontrendUp. IftrendUpis false, you'd likely want aTrendingDownicon instead.Also, consider removing or converting the commented-out code on line 81 to a TODO if it's intentional.
🔧 Suggested fix for trend direction
+import { + Users, + Activity, + CreditCard, + TrendingUp, + TrendingDown, + Shield +} from 'lucide-react';) : ( <span className={`flex items-center font-medium ${stat.trendUp ? 'text-green-600' : 'text-orange-600'}`}> - <TrendingUp className="h-3 w-3 mr-1" /> + {stat.trendUp ? ( + <TrendingUp className="h-3 w-3 mr-1" /> + ) : ( + <TrendingDown className="h-3 w-3 mr-1" /> + )} {stat.trend} </span> )} - {/* <span className="text-slate-400 ml-2">vs last month</span> */}apps/web/src/modules/invitations/InviteMemberDialog.tsx (1)
60-88: Consider extracting the error type for reusability.The inline error type on line 78 is verbose. Consider extracting it to a shared type for consistency across mutation handlers.
🔧 Suggested improvement
// Could be added to a shared types file type ApiError = { response?: { data?: { message?: string } }; message?: string; };Then use it as:
- onError: (error: { response?: { data?: { message?: string } }; message?: string }) => { + onError: (error: ApiError) => {apps/web/src/hooks/use-toast.ts (1)
88-112: Side effects in reducer are acknowledged but could be refactored.The comment on lines 91-92 acknowledges side effects in the reducer. While this works, extracting
addToRemoveQueuecalls to a middleware or effect would improve testability and align with reducer best practices.apps/web/src/layouts/TenantLayout.tsx (5)
40-47: Unused props inSidebarContentcomponent.The
navigate,logout, andtitleprops are declared in the type but never used withinSidebarContent. Either remove them from the interface and call sites, or implement the intended functionality.🔧 Suggested fix - remove unused props
-const SidebarContent = ({ navGroups, location, user }: { +const SidebarContent = ({ navGroups, location, user }: { navGroups: NavGroup[], location: Location, user: { name?: string; email?: string; roles?: string[]; organizationName?: string } | null, - navigate: NavigateFunction, - logout: () => void, - title?: string }) => (And update call sites (lines 128-135 and 148-155):
<SidebarContent navGroups={navGroups} location={location} user={user} - navigate={navigate} - logout={logout} - title={title} />
107-121: Auth guard implementation is functional but has a brief null render.The guard correctly redirects unauthenticated users. However, between
isLoadingbecoming false and the redirect completing,return null(line 120) may cause a brief flash. Consider keeping the loading state visible until navigation completes.
182-187: External API dependency for avatar generation.The avatar image URL calls
api.dicebear.comon every render. Consider:
- Adding error handling if the service is unavailable
- Caching the URL or using a local fallback
The
AvatarFallbackprovides a good default, so this is low priority.
200-209: Role check uses string literal 'admin'.Consider using a constant or enum for role values to prevent typos and improve maintainability across the codebase.
// Could be in a shared constants file const ROLES = { ADMIN: 'admin', USER: 'user', // ... } as const;
96-103: Type assertion foruseAuthreturn value.The inline type assertion is verbose. Consider typing the
useAuthhook's return value at its definition or creating a dedicatedAuthStatetype.apps/web/src/components/ui/toast.tsx (1)
73-89: Non-standardtoast-closeattribute format.Line 83 uses
toast-close=""which is a non-standard HTML attribute. This is likely intended for CSS targeting (e.g.,[toast-close]selector). Consider usingdata-toast-closefor standards compliance, though this is a minor concern as the current approach works.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
apps/web/package.jsonapps/web/src/App.tsxapps/web/src/components/ui/dialog.tsxapps/web/src/components/ui/form.tsxapps/web/src/components/ui/label.tsxapps/web/src/components/ui/select.tsxapps/web/src/components/ui/toast.tsxapps/web/src/components/ui/toaster.tsxapps/web/src/hooks/use-toast.tsapps/web/src/layouts/TenantLayout.tsxapps/web/src/modules/invitations/InviteMemberDialog.tsxapps/web/src/modules/users/UserList.tsxapps/web/src/pages/DashboardPage.tsxapps/web/src/providers/tenant-auth-provider.tsapps/web/src/routes/AdminRoutes.tsxapps/web/src/routes/TenantRoutes.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
apps/web/src/modules/invitations/InviteMemberDialog.tsx (6)
apps/web/src/hooks/use-toast.ts (2)
useToast(189-189)toast(189-189)apps/web/src/components/ui/dialog.tsx (6)
Dialog(110-110)DialogTrigger(113-113)DialogContent(115-115)DialogHeader(116-116)DialogTitle(118-118)DialogFooter(117-117)apps/web/src/components/ui/button.tsx (1)
Button(58-58)apps/web/src/components/ui/form.tsx (6)
Form(170-170)FormField(176-176)FormItem(171-171)FormLabel(172-172)FormControl(173-173)FormMessage(175-175)apps/web/src/components/ui/input.tsx (1)
Input(22-22)apps/web/src/components/ui/select.tsx (5)
Select(149-149)SelectTrigger(152-152)SelectValue(151-151)SelectContent(153-153)SelectItem(155-155)
apps/web/src/components/ui/toaster.tsx (2)
apps/web/src/hooks/use-toast.ts (1)
useToast(189-189)apps/web/src/components/ui/toast.tsx (6)
ToastProvider(122-122)Toast(124-124)ToastTitle(125-125)ToastDescription(126-126)ToastClose(127-127)ToastViewport(123-123)
apps/web/src/App.tsx (3)
apps/web/src/routes/TenantRoutes.tsx (1)
TenantRoutes(12-56)apps/web/src/routes/AdminRoutes.tsx (1)
AdminRoutes(14-56)apps/web/src/components/ui/toaster.tsx (1)
Toaster(13-35)
apps/web/src/components/ui/label.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/ui/form.tsx (2)
apps/web/src/lib/utils.ts (1)
cn(4-6)apps/web/src/components/ui/label.tsx (1)
Label(24-24)
apps/web/src/components/ui/toast.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/modules/users/UserList.tsx (3)
apps/api/src/modules/users/user.schema.ts (1)
user(3-15)apps/web/src/modules/invitations/InviteMemberDialog.tsx (1)
InviteMemberDialog(47-153)apps/web/src/modules/users/Users.tsx (1)
Users(21-83)
apps/web/src/components/ui/dialog.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/layouts/TenantLayout.tsx (1)
apps/api/src/modules/users/user.schema.ts (1)
user(3-15)
apps/web/src/components/ui/select.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (29)
apps/web/src/components/ui/select.tsx (9)
1-7: LGTM!Imports are correctly structured. The
"use client"directive is appropriate since Radix UI Select relies on browser APIs for dropdown behavior and positioning.
9-13: LGTM!Clean aliasing of base Radix primitives for direct consumption.
15-33: LGTM!Well-structured trigger component with proper ref forwarding, focus/disabled state handling, and accessible icon integration via
asChildpattern.
35-68: LGTM!Scroll buttons are consistently implemented with proper ref forwarding and styling.
70-100: LGTM!Comprehensive content wrapper with portal-based rendering, viewport-aware max-height, and smooth animations tied to Radix's data attributes. The conditional popper positioning classes handle all four sides correctly.
102-112: LGTM!Simple and clean label component.
114-134: LGTM!Item component correctly positions the check indicator, handles focus/disabled states, and follows the established shadcn/ui pattern.
136-146: LGTM!Standard separator with negative margins for full-width visual effect.
148-159: LGTM!Complete and organized named exports enabling tree-shaking for consumers.
apps/web/src/components/ui/label.tsx (1)
1-24: Clean implementation following shadcn/ui conventions.The Label component correctly wraps Radix UI's Label primitive with
forwardRef, applies variant-based styling viacva, and merges class names using thecnutility. The pattern is consistent with other UI components in this PR.apps/web/src/routes/AdminRoutes.tsx (1)
14-56: Well-structured Refine routing configuration.The AdminRoutes component correctly wires up Refine with auth, data, and router providers. Resource definitions align with route paths, and the use of relative routes within
<Routes>is appropriate assuming this component is mounted at/admin/*.apps/web/src/components/ui/form.tsx (4)
1-17: Well-structured form component library following shadcn/ui patterns.The form module correctly integrates
react-hook-formwith Radix UI primitives and provides proper context-based field tracking. The accessibility attributes (aria-describedby,aria-invalid) are correctly applied.Minor note:
LabelPrimitiveis imported (line 3) but only used for typingFormLabel. Since you're already importing the wrappedLabelcomponent (line 15), you could potentially derive types from there instead, though this approach is also valid.
41-66: Robust hook with proper context validation.The
useFormFieldhook correctly validates that it's used within both<FormField>and<FormItem>contexts, providing clear error messages when misused. The composed IDs (formItemId,formDescriptionId,formMessageId) follow a consistent naming pattern for accessibility linking.
105-125: Proper accessibility implementation in FormControl.The component correctly uses Radix's
Slotto merge props onto children and applies appropriate ARIA attributes. Thearia-describedbycorrectly references both description and error message IDs when an error is present.
144-166: Clean conditional rendering in FormMessage.The component correctly prioritizes error messages over children and avoids rendering empty elements when there's no message to display.
apps/web/package.json (1)
14-35: Dependencies are properly configured with full React 19 support.The combination of
react-hook-form(7.71.0),@hookform/resolvers(5.2.2), andzod(4.3.5) is a well-established and industry-standard pattern for form validation. All packages explicitly support React 19.2.0, and the version constraints are mutually compatible. The Radix UI components (react-label,react-toast,react-select) integrate seamlessly with the existing Radix dependencies in the project.apps/web/src/components/ui/dialog.tsx (2)
1-52: Well-structured dialog component implementation.The implementation correctly follows the shadcn/ui pattern for Radix UI dialog components. Good use of
forwardReffor ref forwarding, proper accessibility withsr-onlytext on the close button, and consistentdisplayNameassignments for debugging.Minor: Line 22 has a double space in the className string (
bg-black/80 data-[state=open]), which is harmless but could be cleaned up.
54-120: LGTM!The layout components (
DialogHeader,DialogFooter) appropriately use simple function signatures since they don't need ref forwarding.DialogTitleandDialogDescriptioncorrectly useforwardRefto wrap the Radix primitives. Exports are complete.apps/web/src/components/ui/toaster.tsx (1)
1-35: LGTM!The Toaster component correctly implements the toast rendering pattern:
- Proper
"use client"directive for client-side rendering- Stable
idused as thekeyprop for list items- Conditional rendering of optional
titleanddescriptionToastViewportcorrectly positioned as a sibling to the toast listapps/web/src/App.tsx (1)
22-28: Clean modular routing structure.The refactored routing cleanly separates tenant and admin concerns. The
Toasterplacement (line 28) is correct—insideBrowserRouterfor router context access but outsideRoutesfor persistence across navigation.Note: Both
TenantRoutesandAdminRoutesinstantiate their own<Refine>components with different auth providers (tenantAuthProvidervsauthProvider). Ensure this dual-Refine architecture is intentional, as it means each route group manages its own auth/data context independently.apps/web/src/modules/users/UserList.tsx (2)
7-11: Good refactor:basePathprop enables reusability.The
UserListPropsinterface withbasePathallows this component to be used in both tenant (/dashboard/users) and admin (/admin/users) contexts without hardcoding paths.
30-40: LGTM!The
InviteMemberDialogintegration is clean—it manages its own open/close state and toast notifications. ThebasePathprop is correctly passed to theUserscomponent for constructing view/edit links.apps/web/src/pages/DashboardPage.tsx (1)
12-50: Hardcoded placeholder data is acceptable for initial implementation.The comment on line 12 correctly indicates this should be fetched from the API. The structure is well-defined and will translate cleanly to API data.
apps/web/src/modules/invitations/InviteMemberDialog.tsx (2)
47-58: Well-structured form setup with proper validation.Good use of react-hook-form with zod resolver for type-safe validation. The default values are sensible.
117-137: Role select implementation looks correct.The Select component properly integrates with react-hook-form using
onValueChangeanddefaultValuefrom the field. The role options align with the backend schema expectations.apps/web/src/hooks/use-toast.ts (1)
8-9: Verify intentional delay:TOAST_REMOVE_DELAYis ~16.7 minutes.
TOAST_REMOVE_DELAY = 1000000ms equals approximately 16.7 minutes. This seems unusually long for auto-removing dismissed toasts from the DOM. Typical values are 300-1000ms. If this is intentional (e.g., keeping toast in memory for undo actions), consider adding a comment explaining the rationale.apps/web/src/components/ui/toast.tsx (3)
1-9: Standard shadcn/ui toast implementation.Well-structured Radix UI toast primitives with proper imports and the "use client" directive for client-side rendering.
27-41: Toast variants are well-defined.Good use of
class-variance-authorityfor managing default and destructive variants with appropriate styling for animations and states.
115-129: Clean type and component exports.Proper TypeScript type exports alongside component exports provide good DX for consumers of this module.
- Fixed double closing brace in useEffect dependency array
- Added placeholder for /dashboard/settings to prevent 404
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/src/hooks/use-toast.ts (2)
54-70: Consider clearing pending timeouts when toasts are removed.When a toast is removed via
REMOVE_TOASTbefore its removal timeout fires, the timeout remains scheduled and will eventually dispatch for a non-existent toast. While harmless, it's wasteful. Consider clearing the timeout in theREMOVE_TOASTcase:♻️ Suggested improvement
case "REMOVE_TOAST": + if (action.toastId !== undefined) { + const timeout = toastTimeouts.get(action.toastId) + if (timeout) { + clearTimeout(timeout) + toastTimeouts.delete(action.toastId) + } + } else { + toastTimeouts.forEach((timeout) => clearTimeout(timeout)) + toastTimeouts.clear() + } if (action.toastId === undefined) { return { ...state, toasts: [], } }
80-86: Type safety:UPDATE_TOASTaction allows missingid.The
toastproperty inUPDATE_TOASTisPartial<ToasterToast>, meaningidcould be undefined. Ifupdate()is called without anid, the comparison on line 84 (t.id === action.toast.id) would compare againstundefinedand silently fail to update any toast.Consider requiring
idin the update action:♻️ Suggested fix
| { type: ActionType["UPDATE_TOAST"] - toast: Partial<ToasterToast> + toast: Partial<ToasterToast> & Pick<ToasterToast, "id"> }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/hooks/use-toast.tsapps/web/src/routes/TenantRoutes.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/routes/TenantRoutes.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/hooks/use-toast.ts (1)
apps/web/src/components/ui/toast.tsx (3)
ToastProps(120-120)ToastActionElement(121-121)Toast(124-124)
🔇 Additional comments (3)
apps/web/src/hooks/use-toast.ts (3)
8-9: Verify the unusually long toast removal delay.
TOAST_REMOVE_DELAYis set to 1,000,000ms (~16.7 minutes). Typical toast removal delays are 3-5 seconds. If the intent is to keep toasts visible until manually dismissed, consider usingInfinityor a more semantic constant name likeTOAST_REMOVE_DELAY_MANUAL. Otherwise, this may be a placeholder value that was never updated.
169-187: LGTM!The subscription pattern is correctly implemented. The cleanup properly removes the listener on unmount, and the empty dependency array ensures single subscription per component instance. The hook correctly synchronizes component state with the module-level
memoryState.
140-167: LGTM!The
toastfunction correctly generates unique IDs, wires up the auto-dismiss behavior viaonOpenChange, and returns a useful API withdismissandupdatemethods for external control.
1 participant
Summary by CodeRabbit
New Features
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.