feat: add route-keyed page transitions with Framer Motion#100
feat: add route-keyed page transitions with Framer Motion#100Kavlin-Kaur wants to merge 1 commit intoyashvikram30:mainfrom
Conversation
|
@Kavlin-Kaur is attempting to deploy a commit to the yash vikram's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe layout structure of the application was refactored. The Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant RootLayout
participant ClientLayout
participant AuthProvider
participant ThemeProvider
participant AnimatePresence
participant Toaster
Browser->>RootLayout: Loads app
RootLayout->>ClientLayout: Renders children
ClientLayout->>AuthProvider: Wraps content
AuthProvider->>ThemeProvider: Wraps content
ThemeProvider->>AnimatePresence: Handles route transitions
AnimatePresence->>Toaster: Renders notifications
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
app/clientLayout.tsx (1)
31-31: Confirm whether query/hash changes should also animatekey={pathname} ignores search params and hash. If you want transitions on query/hash changes, derive a key from both.
Optional change:
-import { usePathname } from "next/navigation" +import { usePathname, useSearchParams } from "next/navigation" -const pathname = usePathname() +const pathname = usePathname() +const search = useSearchParams() +const routeKey = `${pathname}?${search.toString()}`And:
- key={pathname} + key={routeKey}If you only want path changes to animate (current behavior), keep as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/clientLayout.tsx(1 hunks)app/layout.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/layout.tsx (1)
app/clientLayout.tsx (1)
ClientLayout(17-47)
🔇 Additional comments (6)
app/clientLayout.tsx (4)
29-36: Route-keyed transitions correctly implementedUsing AnimatePresence with mode="wait" and initial={false} plus exit/enter opacity variants is a solid setup for smooth page fades.
25-25: Verify ThemeProvider configuration is intentionalattribute="class" with enableSystem={false} and defaultTheme="light" ignores OS preference. Confirm this is product-intended.
7-7: Import aligns with usageusePathname import is correct for the route-keyed motion div.
12-12: Toaster placement looks goodKeeping Toaster within ThemeProvider (but outside the animated container) prevents remount on route changes and keeps it themed.
app/layout.tsx (2)
6-6: Global CSS import at the root is correctImporting "./globals.css" here ensures it’s applied once at the app root, per Next.js App Router guidance.
21-28: HTML skeleton separation is cleanShifting and into RootLayout and rendering as a sibling to is a solid separation of concerns. suppressHydrationWarning on is appropriate with client-side theming.
If you adopt applying the theme class on (next-themes default), this config should remain stable; confirm there’s no duplicate theme class handling elsewhere.
| </html> | ||
| <AuthProvider> | ||
| <ThemeProvider attribute="class" defaultTheme="light" enableSystem={false} disableTransitionOnChange> | ||
| <div className={`${inter.className} flex min-h-screen flex-col bg-background`}> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the Inter font at the level to style portal content (e.g., Toaster)
Placing ${inter.className} on an inner div won’t affect portal-based UI (react-hot-toast renders to document.body), causing font inconsistency. Apply the font to in RootLayout and drop it from this div.
Follow-up diffs:
In app/layout.tsx (outside the shown changed lines, add import and font init):
import { Inter } from "next/font/google"
const inter = Inter({ subsets: ["latin"] })Then within the changed block:
- <body>
+ <body className={`${inter.className} min-h-screen bg-background antialiased`}>In app/clientLayout.tsx (remove font from the inner container and the Inter usage here):
-import { Inter } from "next/font/google"
-const inter = Inter({ subsets: ["latin"] })
...
- <div className={`${inter.className} flex min-h-screen flex-col bg-background`}>
+ <div className="flex min-h-screen flex-col bg-background">This ensures consistent typography across the app, including toast notifications.
I can push a patch that applies these changes across both files if you prefer.
🤖 Prompt for AI Agents
In app/clientLayout.tsx at line 26, remove the `${inter.className}` from the
div's className to avoid applying the Inter font only to an inner container.
Instead, import and initialize the Inter font in app/layout.tsx and apply
`${inter.className}` to the <body> element there. This change ensures consistent
font styling across portal-based UI like react-hot-toast. Also, remove any Inter
font usage from app/clientLayout.tsx accordingly.
|
@yashvikram30 can u please assign me this and merge it |
1 participant
## Description
Please include a summary of the changes and any related issue.
Fixes # (issue)
## Type of change
- [ ] Bug fix
- [ ] New feature
- [ ] Documentation update
- [ ] Other (please describe):
## Checklist
- [ ] I have read the contributing guidelines
- [ ] My changes follow the code style of this project
- [ ] I have added tests if needed
- [ ] I have updated documentation where necessary
Summary by CodeRabbit