-
Notifications
You must be signed in to change notification settings - Fork 44
feat: add "Investors" nav item to sidebar with appropriate roles and … #153
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: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new Investors dashboard page accessible via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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: 0
🧹 Nitpick comments (2)
src/components/layout/sidebar/constants/items.constant.ts (1)
27-33: New Investors nav item looks good, but consider extracting role constants.The navigation entry is well-structured and follows the existing pattern. However, role strings like "ADMIN", "PAYOUT_PROVIDER", and "GRANTEE" are repeated across multiple nav items (lines 17, 24, 31, 38, 45).
As per coding guidelines, values referenced in multiple callsites should use an enum or const object with UPPERCASE_WITH_UNDERSCORE naming.
🔎 Suggested refactor to extract role constants
Create a roles constant at the top of the file or in a separate constants file:
+export const USER_ROLES = { + ADMIN: "ADMIN", + PAYOUT_PROVIDER: "PAYOUT_PROVIDER", + GRANTEE: "GRANTEE", +} as const; + export const navItems: NavItem[] = [ { title: "Dashboard", url: "/dashboard", icon: Home, - roles: ["PAYOUT_PROVIDER", "GRANTEE"], + roles: [USER_ROLES.PAYOUT_PROVIDER, USER_ROLES.GRANTEE], group: "Platform", }, // Apply to all other nav items...This improves maintainability and follows the project's coding guidelines for repeated string constants.
Based on coding guidelines: enum/const object members should be written UPPERCASE_WITH_UNDERSCORE, and properties referenced in multiple callsites should use an enum or const object.
src/components/modules/investors/ui/pages/Investors.tsx (1)
42-42: Consider data source for metrics in future iterations.The hardcoded "0" values are fine for an MVP, but you'll eventually want to fetch these from an API or accept them as props with proper TypeScript typing.
Also applies to: 57-57, 72-72, 87-87
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/dashboard/investors/page.tsxsrc/components/layout/sidebar/constants/items.constant.tssrc/components/modules/investors/ui/pages/Investors.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend-rule.mdc)
**/*.{js,jsx,ts,tsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Everything must be 100% responsive (using all the sizes of tailwind) and compatible with dark/light shadcn's mode.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Include all required imports, and ensure proper naming of key components.
**/*.{js,jsx,ts,tsx}: enum/const object members should be written UPPERCASE_WITH_UNDERSCORE.
Gate flag-dependent code on a check that verifies the flag's values are valid and expected.
If a custom property for a person or event is at any point referenced in two or more files or two or more callsites in the same file, use an enum or const object, as above in feature flags.
Files:
src/app/dashboard/investors/page.tsxsrc/components/modules/investors/ui/pages/Investors.tsxsrc/components/layout/sidebar/constants/items.constant.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend-rule.mdc)
You can never use anys.
If using TypeScript, use an enum to store flag names.
Files:
src/app/dashboard/investors/page.tsxsrc/components/modules/investors/ui/pages/Investors.tsxsrc/components/layout/sidebar/constants/items.constant.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
Never hallucinate an API key. Instead, always use the API key populated in the .env file.
Files:
src/app/dashboard/investors/page.tsxsrc/components/modules/investors/ui/pages/Investors.tsxsrc/components/layout/sidebar/constants/items.constant.ts
🧬 Code graph analysis (1)
src/components/modules/investors/ui/pages/Investors.tsx (1)
src/components/ui/card.tsx (5)
Card(77-77)CardHeader(78-78)CardTitle(80-80)CardContent(82-82)CardDescription(81-81)
🔇 Additional comments (6)
src/app/dashboard/investors/page.tsx (1)
1-5: LGTM! Clean Next.js page implementation.The page component correctly imports and renders the Investors component, following Next.js 15 App Router conventions. The implementation is minimal and appropriate for a route wrapper.
src/components/layout/sidebar/constants/items.constant.ts (1)
1-1: LGTM! Icon import added correctly.The TrendingUp icon is appropriately chosen for the Investors navigation item and properly imported.
src/components/modules/investors/ui/pages/Investors.tsx (4)
1-19: LGTM! Proper client component setup with clean imports.The "use client" directive is correctly placed, and all necessary components and icons are imported. The imports are well-organized and all used in the component.
20-30: LGTM! Clean component structure with accessible heading hierarchy.The component uses const arrow function syntax as per guidelines. The header section has proper semantic HTML with h1 and descriptive text that works well with screen readers.
22-177: Excellent responsive design and dark mode compatibility.The layout implementation demonstrates several strengths:
- Responsive grid with appropriate breakpoints:
md:grid-cols-2 lg:grid-cols-4andmd:grid-cols-3- Semantic color tokens (
bg-background,text-muted-foreground,text-primary) ensure compatibility with shadcn's dark/light mode- Clean component structure with Card components
- Good visual hierarchy with consistent spacing
The placeholder metric values (0s) and empty state messages are appropriate for an initial implementation.
181-181: LGTM! Proper default export.The component is correctly exported for use by the page route.
…icon
Pull Request | GrantChain
1. Issue Link
2. Brief Description of the Issue
3. Type of Change
Mark with an
xall the checkboxes that apply (like[x]).4. Changes Made
5. Evidence Before Solution
Loom Video - Before Solution
6. Evidence After Solution
Loom Video - After Solution
7. Important Notes
If you don't use this template, you'd be ignored
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.