Fade transitions between pages #36
Conversation
WalkthroughAnimated transitions were introduced for both tab and route navigation using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant RouterContext
participant AnimationEngine
User->>UI: Navigate to new tab/route
UI->>RouterContext: Get current and previous location
UI->>AnimationEngine: Determine animation direction (left/right)
AnimationEngine->>UI: Animate transition (slide in/out)
UI->>User: Display animated new content
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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
🔭 Outside diff range comments (1)
packages/widget-react/src/lib/router/MemoryRouter.tsx (1)
47-49: Consider updating previousLocation in reset function.The
resetfunction clears the history but doesn't updatepreviousLocation, which could lead to stale data after a reset. Consider setting it tonullor the current location before resetting.const reset = useCallback((path: string, state?: object) => { + setPreviousLocation(history[history.length - 1]) setHistory([{ path, state }]) }, [])
🧹 Nitpick comments (3)
packages/widget-react/src/public/app/Routes.tsx (3)
78-83: Optimize route lookup for better performance.The
routes.find()call inside the transition function will execute for every animated route on every render. Consider memoizing this lookup or restructuring to avoid repeated searches.return ( <div style={{ position: "relative", height: "100%", overflow: "hidden" }}> {transitions((style, animatedPath) => { const route = routes.find((r) => r.path === animatedPath) - if (!route) return null + if (!route) { + console.warn(`Route not found for path: ${animatedPath}`) + return null + } if (!address && !route.renderWithoutAddress) return null
68-68: Consider additional scenarios for skipping animations.The
skipAnimationflag only checks for missingprevPath, but other scenarios like initial app load or programmatic navigation might also benefit from skipping animations.- skipAnimation: !prevPath, + skipAnimation: !prevPath || prevPath === path,
73-74: Address the FIXME comment about route blocking.The FIXME comment suggests uncertainty about blocking all routes when no address is connected. This could impact user experience and should be clarified based on the app's requirements.
Do you want me to help analyze the routing requirements and propose a more definitive approach for handling unauthenticated access to different routes?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/widget-react/src/components/form/hooks.ts(1 hunks)packages/widget-react/src/lib/router/MemoryRouter.tsx(2 hunks)packages/widget-react/src/lib/router/RouterContext.ts(2 hunks)packages/widget-react/src/pages/wallet/tabs/Home.module.css(1 hunks)packages/widget-react/src/pages/wallet/tabs/Home.tsx(2 hunks)packages/widget-react/src/public/app/Routes.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/widget-react/src/lib/router/MemoryRouter.tsx (1)
packages/widget-react/src/lib/router/RouterContext.ts (2)
HistoryEntry(3-6)RouterContext(16-16)
🔇 Additional comments (13)
packages/widget-react/src/pages/wallet/tabs/Home.module.css (1)
1-3: LGTM! CSS addition supports animation requirements.The new
.containerclass with 100% height is necessary for the absolute positioning of animated tab content.packages/widget-react/src/components/form/hooks.ts (1)
9-13: LGTM! Proper timeout handling with cleanup.The 250ms delay prevents focus from interfering with page transitions, and the timeout is correctly cleared on unmount.
packages/widget-react/src/lib/router/RouterContext.ts (2)
10-10: LGTM! Interface extension supports navigation history tracking.The
previousLocationproperty is properly typed as nullable to handle initial state.
27-30: LGTM! Hook follows established patterns.The
usePreviousPathhook correctly uses optional chaining to handle nullpreviousLocation.packages/widget-react/src/lib/router/MemoryRouter.tsx (3)
17-18: LGTM! Clear explanation for separate state management.The comment effectively explains why
historyarray cannot be relied upon for tracking the previous location.
21-45: LGTM! Correct timing for capturing previous location.Setting
previousLocationbefore updating history ensures the current location is captured before navigation changes.
52-52: LGTM! Context value properly includes previousLocation.The
previousLocationis correctly added to the context provider value.packages/widget-react/src/pages/wallet/tabs/Home.tsx (3)
13-17: LGTM! Clean tab configuration structure.The centralized
tabsarray improves maintainability and makes it easy to add new tabs.
27-42: LGTM! Well-implemented animation logic.The direction calculation and transition configuration properly handle:
- Directional sliding based on tab order
- Skip animation when navigating from non-tab routes
- Smooth spring physics with appropriate tension/friction values
67-78: LGTM! Proper implementation of animated tab content.The animated transitions correctly use:
- Absolute positioning to enable sliding animations
forceMountto keep tab content in DOM during transitions- Proper style spreading for animation properties
packages/widget-react/src/public/app/Routes.tsx (3)
19-32: LGTM! Excellent refactoring of route configuration.The centralized route configuration array is a significant improvement over the previous switch statement approach. The addition of
rerenderandrenderWithoutAddressflags provides clear control over component behavior.
63-69: Well-implemented spring transition configuration.The transition configuration provides smooth slide animations with appropriate timing. The opacity and transform combinations create a polished user experience.
41-42: Nested tab normalization is intentional and correctThe “/nfts” and “/activity” paths are virtual child‐routes of the Home component (path="/") and are normalized to “/” in Routes.tsx (lines 41–42) so that switching between the Home, NFTs, and Activity tabs doesn’t trigger a full page transition. This matches the
useEffectnavigation logic and preserves the intended animation direction. No changes required.
| // Compute transition direction | ||
| const currentIndex = routes.findIndex((r) => r.path === path) | ||
| const prevIndex = routes.findIndex((r) => r.path === prevPath) | ||
| const direction = currentIndex >= prevIndex ? 1 : -1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle edge cases when routes are not found in the array.
The findIndex method returns -1 when a route is not found, which could lead to unexpected animation directions. Consider adding validation to ensure both routes exist before calculating direction.
// Compute transition direction
const currentIndex = routes.findIndex((r) => r.path === path)
const prevIndex = routes.findIndex((r) => r.path === prevPath)
- const direction = currentIndex >= prevIndex ? 1 : -1
+ const direction = currentIndex === -1 || prevIndex === -1 ? 1 : (currentIndex >= prevIndex ? 1 : -1)📝 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.
| // Compute transition direction | |
| const currentIndex = routes.findIndex((r) => r.path === path) | |
| const prevIndex = routes.findIndex((r) => r.path === prevPath) | |
| const direction = currentIndex >= prevIndex ? 1 : -1 | |
| // Compute transition direction | |
| const currentIndex = routes.findIndex((r) => r.path === path) | |
| const prevIndex = routes.findIndex((r) => r.path === prevPath) | |
| const direction = currentIndex === -1 || prevIndex === -1 | |
| ? 1 | |
| : (currentIndex >= prevIndex ? 1 : -1) |
🤖 Prompt for AI Agents
In packages/widget-react/src/public/app/Routes.tsx around lines 58 to 61, the
code calculates transition direction using indices from routes.findIndex, but
does not handle cases where a route is not found (findIndex returns -1). To fix
this, add validation to check if both currentIndex and prevIndex are not -1
before computing direction. If either is -1, handle the case appropriately, such
as defaulting direction to 0 or skipping the animation logic.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/widget-react/src/pages/wallet/tabs/Home.tsx (1)
67-78: Consider accessibility implications of forceMount.The animated content implementation is functionally correct, but using
forceMounton all tab contents might cause accessibility issues as screen readers could announce all tab panels simultaneously.Consider implementing a more accessibility-friendly approach:
-<Tabs.Content forceMount key={item} value={item} asChild> +<Tabs.Content key={item} value={item} asChild> <animated.div style={{ ...style, position: "absolute", width: "100%" }}> {tab?.component} </animated.div> </Tabs.Content>And handle the animation lifecycle more explicitly to ensure only the active and transitioning tabs are mounted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/widget-react/src/pages/wallet/tabs/Home.tsx(2 hunks)packages/widget-react/src/public/app/Routes.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/widget-react/src/public/app/Routes.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/widget-react/src/pages/wallet/tabs/Home.tsx (3)
1-1: LGTM! Clean imports for animation functionality.The imports are well-organized and include all necessary dependencies for the animated tab transitions.
Also applies to: 4-5
13-17: LGTM! Excellent refactor to dynamic tab configuration.The tabs array provides a clean, maintainable structure for tab metadata. This approach makes it easy to add, remove, or modify tabs in the future.
33-42: LGTM! Well-configured animation with performance optimization.The transition configuration is well-thought-out:
- Uses transform and opacity for smooth performance
- Includes skipAnimation logic to avoid unnecessary animations
- Spring physics configuration provides natural-feeling transitions
| const direction = useMemo(() => { | ||
| const currentIndex = tabs.findIndex((t) => t.value === path) | ||
| const prevIndex = tabs.findIndex((t) => t.value === prevPath) | ||
|
|
||
| return currentIndex > prevIndex ? 1 : -1 | ||
| }, [path, prevPath]) |
There was a problem hiding this comment.
Handle edge case when tab indices are not found.
The direction calculation doesn't handle the case where findIndex returns -1 (when a path is not found in the tabs array). This could lead to incorrect animation direction.
const direction = useMemo(() => {
const currentIndex = tabs.findIndex((t) => t.value === path)
const prevIndex = tabs.findIndex((t) => t.value === prevPath)
+ // Handle case where indices are not found
+ if (currentIndex === -1 || prevIndex === -1) {
+ return 1 // Default to forward direction
+ }
+
return currentIndex > prevIndex ? 1 : -1
}, [path, prevPath])📝 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 direction = useMemo(() => { | |
| const currentIndex = tabs.findIndex((t) => t.value === path) | |
| const prevIndex = tabs.findIndex((t) => t.value === prevPath) | |
| return currentIndex > prevIndex ? 1 : -1 | |
| }, [path, prevPath]) | |
| const direction = useMemo(() => { | |
| const currentIndex = tabs.findIndex((t) => t.value === path) | |
| const prevIndex = tabs.findIndex((t) => t.value === prevPath) | |
| // Handle case where indices are not found | |
| if (currentIndex === -1 || prevIndex === -1) { | |
| return 1 // Default to forward direction | |
| } | |
| return currentIndex > prevIndex ? 1 : -1 | |
| }, [path, prevPath]) |
🤖 Prompt for AI Agents
In packages/widget-react/src/pages/wallet/tabs/Home.tsx around lines 26 to 31,
the direction calculation does not handle cases where findIndex returns -1 if a
path is not found in the tabs array. Update the useMemo logic to check if either
currentIndex or prevIndex is -1 and handle this edge case explicitly, for
example by returning a default direction or skipping the animation, to avoid
incorrect animation direction.
Summary by CodeRabbit
New Features
Refactor
Style
Bug Fixes