improvement: mobile experience #1010
Conversation
|
Warning Rate limit exceeded@ameer2468 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughIntroduces a new MobileTab component with an org selector and quick links, integrates it into the dashboard layout, restructures DashboardInner layout and nav positioning, tweaks mobile navbar padding/removal of open button, and adds a mobile CSS rule to offset the Intercom launcher. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant MT as MobileTab
participant OM as OrgsMenu
participant DC as DashboardContext
participant SA as updateActiveOrganization (server)
participant R as Router
U->>MT: Tap Orgs pill
MT->>OM: Toggle menu (open)
U->>OM: Select organization
OM->>DC: setActiveOrganization(id)
DC->>SA: Invoke server action with org id
SA-->>DC: Confirm active org updated
OM->>MT: Close menu
MT->>R: navigate("/dashboard/caps")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 (4)
apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (2)
83-89: Review the fixed navbar positioning strategy.The navbar is now fixed on mobile (using
fixedclass withtop-0) but relative on larger screens (md:relative). This dual positioning approach combined with the margin offset (mt-[126px]on the parent andmt-[60px]on the navbar) creates a complex layout that may be fragile and harder to maintain.Consider using CSS custom properties or a single positioning strategy to manage the layout more consistently.
176-182: Remove placeholder CSS class.The
custom-scrollclass is referenced on Line 177 but doesn't appear to be defined in the provided CSS. This seems to be a placeholder that should either be implemented or removed.Apply this diff to remove the unused class:
- "flex overscroll-contain flex-col flex-1 flex-grow p-5 h-full border border-b-0 custom-scroll bg-gray-2 border-gray-3 lg:rounded-tl-2xl lg:p-8" + "flex overscroll-contain flex-col flex-1 flex-grow p-5 h-full border border-b-0 bg-gray-2 border-gray-3 lg:rounded-tl-2xl lg:p-8"apps/web/app/(org)/dashboard/_components/MobileTab.tsx (2)
126-129: Consider UX improvement for organization switching.When switching organizations, the user is automatically redirected to
/dashboard/caps. This might be disruptive if the user was on a different page and just wanted to switch context. Consider preserving the current route or making the redirect optional.Would you like me to implement a solution that preserves the user's current location within the dashboard when switching organizations?
33-43: Consider adding active state indicators to navigation icons.The navigation icons (spaces, caps, settings) don't show which section is currently active. Adding visual indicators would improve user orientation.
Would you like me to implement active state styling for the navigation icons based on the current route?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(org)/dashboard/_components/DashboardInner.tsx(3 hunks)apps/web/app/(org)/dashboard/_components/MobileTab.tsx(1 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Mobile.tsx(2 hunks)apps/web/app/(org)/dashboard/layout.tsx(2 hunks)apps/web/app/globals.css(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration
Files:
apps/web/app/(org)/dashboard/_components/MobileTab.tsxapps/web/app/(org)/dashboard/_components/Navbar/Mobile.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/_components/DashboardInner.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps/web,packages/ui}/**/*.{ts,tsx}: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'
Files:
apps/web/app/(org)/dashboard/_components/MobileTab.tsxapps/web/app/(org)/dashboard/_components/Navbar/Mobile.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/_components/DashboardInner.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/web/app/(org)/dashboard/_components/MobileTab.tsxapps/web/app/(org)/dashboard/_components/Navbar/Mobile.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/_components/DashboardInner.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/web/app/(org)/dashboard/_components/MobileTab.tsxapps/web/app/(org)/dashboard/_components/Navbar/Mobile.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/_components/DashboardInner.tsx
🧬 Code graph analysis (2)
apps/web/app/(org)/dashboard/_components/MobileTab.tsx (3)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (3)
LayersIcon(19-19)CapIcon(14-14)CogIcon(16-16)apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(42-42)apps/web/app/(org)/dashboard/_components/Navbar/server.ts (1)
updateActiveOrganization(15-39)
apps/web/app/(org)/dashboard/layout.tsx (2)
apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsx (1)
DesktopNav(13-90)apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (1)
DashboardInner(51-194)
🪛 GitHub Check: Typecheck
apps/web/app/(org)/dashboard/_components/MobileTab.tsx
[failure] 109-109:
Type 'MutableRefObject' is not assignable to type 'LegacyRef | undefined'.
[failure] 60-60:
Parameter 'p' implicitly has an 'any' type.
[failure] 60-60:
Argument of type '(p: any) => boolean' is not assignable to parameter of type 'boolean'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (5)
apps/web/app/globals.css (1)
863-867: LGTM! Proper Intercom launcher repositioning for mobile navigation.The media query correctly adjusts the Intercom launcher position to avoid overlap with the new mobile bottom navigation (90px from bottom instead of default). The use of
!importantis justified here to override third-party inline styles.apps/web/app/(org)/dashboard/_components/Navbar/Mobile.tsx (1)
57-57: LGTM! Consistent mobile header padding.The addition of horizontal padding (
px-4) to the right-side items container creates visual balance with the left-side logo container, improving the mobile header appearance.apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (1)
307-307: Good fix for React list key warnings.Converting numeric indices to strings for the key prop is the correct approach. This prevents React warnings about non-string keys in production builds.
apps/web/app/(org)/dashboard/layout.tsx (1)
82-91: Solid responsive grid layout implementation.The grid structure with responsive column spans (
col-span-2on mobile,md:col-span-1on desktop) properly handles the show/hide behavior of the desktop navigation. The addition ofMobileTabas a separate grid item ensures clean separation of concerns.apps/web/app/(org)/dashboard/_components/MobileTab.tsx (1)
28-44: Well-structured mobile navigation component.The component provides good mobile UX with:
- Sticky bottom positioning for thumb-friendly navigation
- Clear visual hierarchy with the organization selector
- Quick access to commonly used features
- Proper z-index layering (
z-50) to stay above contentThe animation and click-away behavior for the organization dropdown are properly implemented.
This PR:
Summary by CodeRabbit