🤖 feat: implement proper URL routing with react-router #1281
Conversation
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
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ammar-agent
force-pushed
the
routing-64w3
branch
from
781e8f8 to
926f38d
Compare
Replace hash-based URL handling with proper URL routing using react-router-dom. ## Changes **New URL Structure:** - `/` → Welcome screen (no project selected) - `/project?path=<projectPath>` → ProjectPage (workspace creation) - `/workspace/:workspaceId` → AIView (active workspace) **Architecture:** - Add react-router-dom with MemoryRouter for Electron compatibility - Create RouterContext to provide navigation functions and parse current route - Derive selectedWorkspace and pendingNewWorkspaceProject from URL state - Sync router state to browser URL via history.replaceState() **Backward Compatibility:** - Legacy hash URLs (`#workspace=abc`, `#/path/to/project`) are converted to proper routes - localStorage workspace is restored when loading at root URL - Existing navigation APIs (setSelectedWorkspace, beginWorkspaceCreation) continue to work **Files Changed:** - package.json: Add react-router-dom dependency - RouterContext.tsx: New - router provider and navigation context - useRouterUrlSync.ts: New - URL parsing and sync logic - WorkspaceContext.tsx: Derive state from router instead of internal state - AppLoader.tsx: Wrap app in RouterProvider - App.tsx: Remove old hash syncing code --- _Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking: `high`_
The parseInitialUrl function now correctly treats .html paths (from file:// URLs in packaged Electron) as root URLs, allowing localStorage workspace restoration to work in production builds.
In Electron with file:// protocol, updating the browser URL to /workspace/abc would cause page.reload() to fail with ERR_FILE_NOT_FOUND. The MemoryRouter state is sufficient for in-app navigation; only browser/server mode needs the URL bar to reflect current state.
In Storybook, the pathname is /iframe.html with query params for story selection. This should still restore the selected workspace from localStorage.
Dead code from initial implementation: - workspaceUrl() - only used internally - projectUrl() - only used internally - useAppNavigate() - never imported; RouterContext has same logic
ammar-agent
force-pushed
the
routing-64w3
branch
from
926f38d to
3535536
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Replace hash-based URL handling with proper URL routing using react-router-dom.
URL Structure
//project?path=/path/to/project/workspace/:workspaceIdKey Changes
New Files
src/browser/contexts/RouterContext.tsx- Router provider wrapping MemoryRouter with navigation contextsrc/browser/hooks/useRouterUrlSync.ts- URL parsing and browser URL synchronizationModified Files
WorkspaceContext.tsx- State derived from URL instead of internal useStateAppLoader.tsx- Wrapped in RouterProviderApp.tsx- Removed old hash syncing codeArchitecture
Uses MemoryRouter (not BrowserRouter) because Electron's
file://protocol in production doesn't support HTML5 pushState. The router state syncs to the browser URL viahistory.replaceState()so the URL bar shows proper routes.Backward Compatibility
#workspace=abc,#/path/to/project) are converted to proper routes on loadsetSelectedWorkspace,beginWorkspaceCreation) continue to workTesting
Generated with
mux• Model:anthropic:claude-opus-4-5• Thinking:high