Open
Conversation
Implements a mobile-like stack-based navigation system to address issue #445. Key changes: 1. **`useNavigationHistory` hook (`app/hooks/use-navigation-history.ts`):** * Manages a custom navigation stack (`historyStack`). * Provides `push` and `pop` methods to control navigation. * `push` adds a new entry to the stack and navigates using `react-router`, storing the `historyStack` in the browser's `location.state`. * `pop` removes the top entry, navigates to the previous entry, and updates the `historyStack` in `location.state`. * A `useEffect` hook syncs the internal `historyStack` with `location.state.historyStack`, enabling correct behavior for browser back/forward buttons and manual URL changes. 2. **Updated Navigation Components (`app/lib/transitions/view-transition.tsx`):** * `LinkWithViewTransition` now uses `useNavigationHistory().push` to navigate, ensuring new routes are added to the custom stack. * `useNavigateWithViewTransition` now returns a function that uses `useNavigationHistory().push`. * View transition logic (`useViewTransitionEffect`) remains compatible, reading transition details from `location.state`. 3. **Custom Back Button Handling (`app/hooks/use-custom-back-button.ts`):** * Introduces `useCustomBackButton` hook, which is initialized in `app/root.tsx`. * Provides a `goBack()` function that calls `useNavigationHistory().pop()`. This can be used by UI elements (e.g., header back arrows). * Relies on `useNavigationHistory`'s `useEffect` to handle actual browser `popstate` events (back/forward buttons). This new system ensures that navigation, especially using the back button (browser or in-app), behaves like a stack, as is common in mobile applications. View transitions are preserved. The scenario described in issue #445 (Home -> Settings -> Appearance, close Appearance, then back button leads to Home) should now work correctly if closing a page is implemented using the `pop()` mechanism.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Updates to Preview Branch (feature/mobile-like-routing) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
gudnuf
reviewed
Jun 4, 2025
Contributor
gudnuf
left a comment
gudnuf
left a comment
There was a problem hiding this comment.
Seems more complicated than it needs to be, were you just wanting to show me the result of the ai's work or do you like it how it is?
| // If not, we might need to explicitly call `pop()` here under certain conditions, | ||
| // for example, if the new location (after browser back) matches what `pop()` would navigate to. | ||
|
|
||
| // For now, this effect will primarily be a placeholder to remind us |
Contributor
There was a problem hiding this comment.
Have you tested this? Seems like this lots of comments is saying it might now work as implemented
| }; | ||
| } | ||
|
|
||
| export const transitionStyles = ` |
Contributor
There was a problem hiding this comment.
these should be added to transitions.css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implemented by Jules
Implements a mobile-like stack-based navigation system to address issue #445.
Key changes:
useNavigationHistoryhook (app/hooks/use-navigation-history.ts):historyStack).pushandpopmethods to control navigation.pushadds a new entry to the stack and navigates usingreact-router, storing thehistoryStackin the browser'slocation.state.popremoves the top entry, navigates to the previous entry, and updates thehistoryStackinlocation.state.useEffecthook syncs the internalhistoryStackwithlocation.state.historyStack, enabling correct behavior for browser back/forward buttons and manual URL changes.Updated Navigation Components (
app/lib/transitions/view-transition.tsx):LinkWithViewTransitionnow usesuseNavigationHistory().pushto navigate, ensuring new routes are added to the custom stack.useNavigateWithViewTransitionnow returns a function that usesuseNavigationHistory().push.useViewTransitionEffect) remains compatible, reading transition details fromlocation.state.Custom Back Button Handling (
app/hooks/use-custom-back-button.ts):useCustomBackButtonhook, which is initialized inapp/root.tsx.goBack()function that callsuseNavigationHistory().pop(). This can be used by UI elements (e.g., header back arrows).useNavigationHistory'suseEffectto handle actual browserpopstateevents (back/forward buttons).This new system ensures that navigation, especially using the back button (browser or in-app), behaves like a stack, as is common in mobile applications. View transitions are preserved. The scenario described in issue #445 (Home -> Settings -> Appearance, close Appearance, then back button leads to Home) should now work correctly if closing a page is implemented using the
pop()mechanism.