Add OpenRig-inspired topology navigation cleanup - #42
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR refactors the app to drive UI state from URL search parameters instead of component-local React state. It introduces a centralized view-location module with href builders ( ChangesURL-driven routing and state refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/app/(main)/(tabs)/_layout.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. app/app/(main)/(tabs)/topology/index.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. app/app/(main)/_layout.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/app/`(main)/_layout.tsx:
- Around line 52-53: Prevent reuse of a previously selected project's endpoint
when the URL points to a different project by only falling back to
selectedProjectEndpoint if the URL project matches the currently selected
project; change the endpoint computation (variable endpoint) to prefer
effectiveProject.serviceEndpoint, but if effectiveProject is missing only use
selectedProjectEndpoint when urlProjectPath === selectedProject?.projectPath
(otherwise leave endpoint undefined/null) so writes cannot land under the wrong
effectiveProjectPath.
In `@app/app/`(main)/(tabs)/_layout.tsx:
- Line 16: The freezeOnBlur flag is currently enabled for web due to an inverted
Platform.OS check; update the assignment (the object/property where freezeOnBlur
is set) to enable freezeOnBlur only on native platforms by changing the
condition to Platform.OS !== "web" (or explicitly Platform.OS === "ios" ||
Platform.OS === "android") so that freezeOnBlur is false on web and true on
native.
In `@app/app/`(main)/(tabs)/topology/index.tsx:
- Around line 451-453: The endpoint fallback to selectedProjectEndpoint can
point to a different project; change the binding so endpoint is strictly the
URL-resolved project's endpoint (use const endpoint = project?.serviceEndpoint;)
and remove the ?? selectedProjectEndpoint fallback; then audit code that
consumes endpoint (and any fetch/render logic) to handle undefined endpoints
gracefully or explicitly use selectedProjectEndpoint only when project ===
selectedProject. Reference: project, projects, urlProjectPath,
selectedProjectEndpoint, desktopStateFamily, desktopState.
In `@app/components/AppShell.tsx`:
- Around line 24-31: The two useEffect hooks wrongly include sidebarOpen in
their dependency arrays which causes the drawer to immediately close when
opened; fix by removing sidebarOpen from both deps: make the "on load" effect
run only on mount (change the deps to [] and keep the body using isMobile &&
sidebarOpen to close on initial load), and make the "on navigation" effect run
only when pathname changes (set its deps to [pathname, setSidebarOpen] or
[pathname] if you prefer and add a // eslint-disable-next-line
react-hooks/exhaustive-deps lint comment); update the effects that reference
useEffect, isMobile, sidebarOpen, setSidebarOpen, and pathname accordingly.
In `@app/components/ProjectSidebar.tsx`:
- Around line 528-531: The effect in usePrePaintEffect closes the picker
immediately because showPicker is included in the dependency array; remove
showPicker from the deps so the effect only runs when effectiveProjectPath or
selectedProjectPath change, and keep the unconditional setShowPicker(false)
inside the effect; add a single-line lint suppression for
react-hooks/exhaustive-deps to acknowledge the intentional omission (referencing
usePrePaintEffect, showPicker, setShowPicker, effectiveProjectPath,
selectedProjectPath).
In `@app/lib/view-location.ts`:
- Around line 55-64: mergeViewParams currently uses the nullish coalescing
operator (??) so passing explicit nulls in next (e.g. { mode: null }) falls back
to the current search and cannot clear params; update the merge logic to treat
only undefined as "no override" and allow null to pass through: for project,
mode, lens, section, document change each expression from "next.X ?? fallback"
to "next.X !== undefined ? next.X : fallback" (for project keep using
projectPathFromSearch(current.project) as the fallback and for the others keep
cleanSearchValue(current.<field>) as the fallback) so explicit nulls clear state
while undefined still merges from current.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 058c8ea7-8000-4279-b0ef-a8f7e26b2c7c
📒 Files selected for processing (32)
app/app/(main)/(tabs)/(dashboard)/agent/[sessionId]/chat.tsxapp/app/(main)/(tabs)/(dashboard)/index.tsxapp/app/(main)/(tabs)/(dashboard)/service/[serviceId].tsxapp/app/(main)/(tabs)/(inbox)/notifications/_layout.tsxapp/app/(main)/(tabs)/(inbox)/notifications/agent/[sessionId]/chat.tsxapp/app/(main)/(tabs)/(inbox)/notifications/index.tsxapp/app/(main)/(tabs)/(inbox)/notifications/service/[serviceId].tsxapp/app/(main)/(tabs)/_layout.tsxapp/app/(main)/(tabs)/library/_layout.tsxapp/app/(main)/(tabs)/library/agent/[sessionId]/chat.tsxapp/app/(main)/(tabs)/library/index.tsxapp/app/(main)/(tabs)/library/service/[serviceId].tsxapp/app/(main)/(tabs)/project/_layout.tsxapp/app/(main)/(tabs)/project/agent/[sessionId]/chat.tsxapp/app/(main)/(tabs)/project/index.tsxapp/app/(main)/(tabs)/project/service/[serviceId].tsxapp/app/(main)/(tabs)/topology/_layout.tsxapp/app/(main)/(tabs)/topology/agent/[sessionId]/chat.tsxapp/app/(main)/(tabs)/topology/index.tsxapp/app/(main)/(tabs)/topology/service/[serviceId].tsxapp/app/(main)/_layout.tsxapp/app/_layout.tsxapp/app/index.tsxapp/components/AppShell.tsxapp/components/MobileTabBar.tsxapp/components/ProjectSidebar.tsxapp/components/screens/AgentChatScreen.tsxapp/components/screens/ServiceDetailScreen.tsxapp/index.jsapp/lib/main-tabs.tsapp/lib/view-location.test.tsapp/lib/view-location.ts
💤 Files with no reviewable changes (2)
- app/app/index.tsx
- app/app/_layout.tsx
This was referenced Jun 28, 2026
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
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.
Summary
Verification
Summary by CodeRabbit
New Features
Improvements