chore: refactor the surfaces routes to reflect the new functionality#4078
chore: refactor the surfaces routes to reflect the new functionality#4078arikorn wants to merge 2 commits intobitfocus:mainfrom
Conversation
The main page is more than the "configured surfaces" table, so "configured surfaces" is not really the correct name for it.. Perhaps more importantly, this refactor also supports the new features in PR# 4062, so that the surfaces group-toggle shows active when the remote page is selected. (It will probably create a minor merge-conflict with it, due to the Sidebar.tsx overlap. Also note: both PRs delete the redundant "configured surfaces" sidebar item.)
(Makes it more like the "Connections" and other pages.)
📝 WalkthroughWalkthroughThe PR restructures the surfaces routing architecture by moving routes from the Changes
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
webui/src/routes/_app/surfaces_/remote/$connectionId.tsx (1)
1-7: Declarative navigation is a nice improvement!Switching from the imperative
navigate()call to the<Navigate>component is cleaner and more React-idiomatic. The route structure withsurfaces_correctly maps to/surfaces/remoteas confirmed by the parent route file.One small observation: the import on line 4 (
'~/Resources/Error') doesn't include the.jsextension, while the similar file$itemId.tsxuses'~/Resources/Error.js'. It would be nice to keep these consistent across the codebase when you have a moment, but it's certainly not blocking!
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a5e1899-117d-4b50-ad01-1f52320bf1c7
📒 Files selected for processing (24)
webui/src/Layout/Sidebar.tsxwebui/src/Surfaces/EditPanel.tsxwebui/src/Surfaces/Instances/AddSurfaceInstancePanel.tsxwebui/src/Surfaces/Instances/SurfaceInstanceEdit/SurfaceInstanceEditPanel.tsxwebui/src/Surfaces/Instances/SurfaceInstanceList/SurfaceInstanceList.tsxwebui/src/Surfaces/KnownSurfacesTable.tsxwebui/src/Surfaces/MainSurfacesPage.tsxwebui/src/Surfaces/SurfaceSettingsPanel.tsxwebui/src/UserConfig/index.tsxwebui/src/routeTree.gen.tswebui/src/routes/_app/surfaces.tsxwebui/src/routes/_app/surfaces/$.tsxwebui/src/routes/_app/surfaces/$itemId.tsxwebui/src/routes/_app/surfaces/configured.$.tsxwebui/src/routes/_app/surfaces/configured.tsxwebui/src/routes/_app/surfaces/index.tsxwebui/src/routes/_app/surfaces/integrations/$instanceId.tsxwebui/src/routes/_app/surfaces/integrations/add.tsxwebui/src/routes/_app/surfaces/integrations/index.tsxwebui/src/routes/_app/surfaces_/remote.tsxwebui/src/routes/_app/surfaces_/remote/$connectionId.tsxwebui/src/routes/_app/surfaces_/remote/discover.tsxwebui/src/routes/_app/surfaces_/remote/index.tsxwebui/src/scss/_surfaces.scss
💤 Files with no reviewable changes (1)
- webui/src/routes/_app/surfaces/$.tsx
The main Surfaces page is now more than the "configured surfaces" table, so "configured surfaces" is not really the correct name for it..
This refactor also supports the new features in PR #4062, so that the surfaces group-toggle shows active when the remote page is selected. (It will probably create a minor merge-conflict with it, due to the Sidebar.tsx overlap. Also note: both PRs delete the redundant "configured surfaces" sidebar item.)
Bonus: apply some (local) rabbit suggestions to make code a bit more robust.
The second commit here improves wording and help on the Surfaces page. (Makes it more like the "Connections" and other pages.)
Summary by CodeRabbit
Navigation & Routing
UI Updates