Skip to content

fix(shell): settings always keeps the sidebar visible and never becomes a project's start page - #1725

Merged
arul28 merged 2 commits into
mainfrom
perf/settings-sidebar
Oct 17, 2026
Merged

arul28 merged 2 commits into
mainfrom
perf/settings-sidebar

Conversation

@arul28

@arul28 arul28 commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Why

User-reported bug #1. On Settings › This computer › Diagnostics there was no visible way out: the project sidebar could be collapsed on Settings, and the only way back was the collapsed-sidebar toggle at the top-left, which nobody notices. Machine-scoped settings pages were also written as the project's remembered route (ade:project-route:<bindingKey>), so the project reopened on Diagnostics.

Correctness fix, no perf claim.

What changed

  1. Settings holds the sidebar open. ProjectSidebar.tsx computes hidden = hiddenByPreference && !settingsActive; the saved show/hide preference is not written. ProjectSidebarToggle.tsx is disabled on Settings (aria-disabled, dimmed, tooltip "Settings keeps the sidebar open."), with locked passed from AppShell through TopBar. Mod+B is a no-op on Settings (useProjectSidebarShortcuts.ts, sidebarLocked).
  2. Collapsed cue. When the sidebar is collapsed, the toggle gets data-collapsed="true", which sets its icon to var(--color-accent) (index.css, next to the ade-shell-control ghost rules), and renders a 6px bg-accent dot at its top-right (absolute right-[3px] top-[3px] h-1.5 w-1.5 rounded-full). No animation, so no reduced-motion handling is needed.
  3. Machine pages are never a project's route. settingsManifest.ts gains settingsTabForRoute (same precedence as SettingsPage: a #hash wins over ?tab=; unknown falls back to the default tab) and isMachineScopedSettingsRoute (the tab is in isMachineSettingsTab). They are used by:
    • projectRouteStorage.ts: writeStoredProjectRoute ignores machine routes; readStoredProjectRoute restores a stored machine route to /work and rewrites it.
    • App.tsx serializeStoredProjectRoute: returns null for machine routes, so the in-memory routesBySurfaceKey does not keep them either.
    • AppShell.tsx serializeLocationRoute: returns null for machine routes (its own writer, keyed by the raw project root, which the cog reads).
    • settingsReturnRoute.ts: the cog's in-session "last settings section" skips machine routes.
  4. Tests updated. Two ProjectSidebar.test.tsx cases stored or remembered appearance / providers, both machine pages. They now use chat / secrets so they still test remembered-route behaviour.
  5. Docs. docs/features/onboarding-and-settings/README.md: new subsection "Settings holds the project sidebar open".

Machine-scoped routes excluded (the machines group in SETTINGS_TABS, settingsManifest.ts): General (general), Appearance (appearance), Providers (agents), Apple devices (apple), Lanes (lanes-git), Integrations (integrations), Diagnostics (storage), Archive (archive). MACHINE_SETTINGS_TAB_IDS holds the same set. Account pages (Account, Chat, Notifications, MCP servers, Usage) and the Project page (Secrets) stay remembered, as before.

Evidence

Correctness fix, no perf claim.

  • New test apps/desktop/src/renderer/components/app/projectRouteStorage.test.ts: "never remembers a machine-scoped settings page, and restores a stale one to the default".
    • With projectRouteStorage.ts reverted to origin/main (the other parts of this PR applied): FAIL … expected '/settings?tab=chat#diagnostics' to be '/lanes'.
    • With this PR: PASS.
  • cd apps/desktop && npx vitest run on projectRouteStorage.test.ts, projectSidebar/ProjectSidebar.test.tsx, settings/settingsManifest.test.ts, TopBar.test.tsx, projectSidebar/useProjectSidebarShortcuts.test.tsx, AppShell.aiStatus.test.tsx, AppShell.laneRefresh.test.tsx: 7 files, 141 tests, all passed.
  • npm --prefix apps/desktop run typecheck: exit 0.
  • npx eslint on the 11 touched TS files: 0 errors, 5 warnings, all in TopBar.tsx in code this PR does not touch (unused windowId, four react-hooks/exhaustive-deps).
  • Before / after (UI, not run in the app):
    • Before: Settings could collapse the sidebar; the toggle looked the same collapsed or not; a project reopened on Diagnostics.
    • After: on Settings the sidebar is visible and its toggle is dimmed and disabled, Mod+B does nothing, and leaving Settings restores the saved preference. Elsewhere, a collapsed sidebar's toggle is accent-coloured with a dot. A project reopens on its last non-machine page, or Work.

Not checked

  • Not launched in the real app (coordinator verifies). The accent and dot colours are not checked by eye in light or dark themes.
  • The exact multi-tab sequence behind "every project tab reopened on Diagnostics" was not reproduced. The fix covers every route-write path I found (the App effect and the AppShell writer), and the test pins the storage layer.
  • Hosted web client (webMode) and remote project tabs are not exercised.
  • Windows: renderer-only, no platform branch, not run on Windows.
  • Full vitest suite not run (scoped files only).

🤖 Generated with Claude Code

ADE   Open in ADE  ·  perf/settings-sidebar branch  ·  PR #1725


Note

Low Risk
Renderer-only navigation and localStorage route-memory changes with targeted tests; no auth or data-layer impact.

Overview
Fixes a navigation trap on Settings → This computer pages by changing how the project sidebar and route memory behave on Settings.

Settings always shows the project sidebar without overwriting the user’s saved collapse preference: the sidebar is forced visible on /settings, the top-bar toggle is disabled (with an explanatory tooltip), and Mod+B no longer hides it. When the sidebar is collapsed elsewhere, the toggle is highlighted with accent styling and a dot so it’s easier to spot.

Machine-scoped settings routes are no longer remembered as a project’s restore target. New helpers in settingsManifest (settingsTabForRoute, isMachineScopedSettingsRoute) classify “This computer” tabs; all project-route writers/readers skip or migrate those URLs to /work, while account/project settings sections (e.g. Chat, Secrets) still restore as before. Session “last settings section” for the cog follows the same rule.

Tests cover storage behavior; onboarding/settings docs describe the new rules.

Reviewed by Cursor Bugbot for commit 519e113. Configure here.

…es a project's start page

Settings pages could collapse the project sidebar, so a user who opened
Diagnostics (a This computer page) had no visible way out except the
collapsed toggle. Machine-scoped settings pages were also saved as the
project's remembered route, so the project kept reopening on them.

- Settings holds the sidebar open: the toggle is disabled there and Mod+B
  does nothing. The saved show/hide preference is not changed, so leaving
  Settings restores the sidebar the way the user had it.
- A collapsed sidebar's toggle is drawn in the accent colour with a dot.
- Machine-scoped settings routes (the "This computer" group) are never
  stored as a project's route or remembered as its last settings section.
  A stored one restores to /work.

Correctness fix, no perf claim.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 17, 2026

Copy link
Copy Markdown

Deployment failed for project ade with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/arul28s-projects?upgradeToPro=build-rate-limit

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 19 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: arul28/ADE/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 93ffed90-93cd-4a4c-99ab-805f75d50097
📥 Commits

Reviewing files that changed from the base of the PR and between 83786e3 and 519e113.

⛔ Files ignored due to path filters: 1
  • docs/features/onboarding-and-settings/README.md is excluded by !docs/**
📒 Files selected for processing: 12
  • apps/desktop/src/renderer/components/app/App.tsx
  • apps/desktop/src/renderer/components/app/AppShell.tsx
  • apps/desktop/src/renderer/components/app/TopBar.tsx
  • apps/desktop/src/renderer/components/app/projectRouteStorage.test.ts
  • apps/desktop/src/renderer/components/app/projectRouteStorage.ts
  • apps/desktop/src/renderer/components/app/projectSidebar/ProjectSidebar.test.tsx
  • apps/desktop/src/renderer/components/app/projectSidebar/ProjectSidebar.tsx
  • apps/desktop/src/renderer/components/app/projectSidebar/ProjectSidebarToggle.tsx
  • apps/desktop/src/renderer/components/app/projectSidebar/settingsReturnRoute.ts
  • apps/desktop/src/renderer/components/app/projectSidebar/useProjectSidebarShortcuts.ts
  • apps/desktop/src/renderer/components/settings/settingsManifest.ts
  • apps/desktop/src/renderer/index.css
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 720c72d5-4ce6-4d0b-99bb-79fdcdc7373e)

…ings, where it does nothing

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
ade Ignored Ignored Preview Oct 17, 2026 9:14am UTC

@cursor

cursor Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: c31a0369-0027-4c27-a980-3cb74dbe3e39)

@arul28
arul28 merged commit cc2d5ef into main Oct 17, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant