Skip to content

fix: auto-scroll sidebar to follow workspace selection - #2425

Closed
sugarshin wants to merge 2 commits into
manaflow-ai:mainfrom
sugarshin:fix/sidebar-scroll
Closed

sugarshin wants to merge 2 commits into
manaflow-ai:mainfrom
sugarshin:fix/sidebar-scroll

Conversation

@sugarshin

@sugarshin sugarshin commented Mar 31, 2026 •

Copy link
Copy Markdown

Summary

  • Wrap the sidebar ScrollView in ScrollViewReader and add .onChange(of: tabManager.selectedTabId) to scroll to the active tab
  • When switching workspaces with Ctrl+Cmd+[ / Ctrl+Cmd+], the sidebar now auto-scrolls to keep the selected tab visible when sessions exceed the screen height
  • Uses anchor: nil for minimal scrolling — no scroll when the tab is already visible

Testing

  • Built and launched tagged debug app (reload.sh --tag fix-sidebar-scroll --launch)
  • Created 20+ workspaces to exceed sidebar height
  • Verified Ctrl+Cmd+] scrolls sidebar down to follow selection
  • Verified Ctrl+Cmd+[ scrolls sidebar up to follow selection
  • Verified no unnecessary scrolling when selected tab is already visible

Demo Video

screen2026-03-31.21.01.17.mov

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed
  • I requested bot reviews after my latest commit (copy/paste block above or equivalent)
  • All code review bot comments are resolved
  • All human review comments are resolved

Summary by cubic

Sidebar now auto-scrolls to keep the selected tab visible when switching workspaces with Ctrl+Cmd+[ / Ctrl+Cmd+], even when the list exceeds screen height. Implemented with ScrollViewReader and the two-argument .onChange to scroll to tabManager.selectedTabId with a short ease-out animation (anchor: nil); adds a DEBUG log for scroll events.

Written for commit 86bf756. Summary will update on new commits.

Summary by CodeRabbit

  • New Features
    • Sidebar now automatically scrolls the selected tab into view with a short ease-out animation and includes debug logging for selection changes.
  • Refactor
    • Sidebar view hierarchy and scroll handling reorganized to ensure reliable programmatic scrolling and consistent overlays/background behavior.

When switching workspaces with Ctrl+Cmd+[ / Ctrl+Cmd+], the sidebar now
scrolls to keep the selected tab visible when sessions exceed the screen height.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sugarshin

Copy link
Copy Markdown
Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@vercel

vercel Bot commented Mar 31, 2026

Copy link
Copy Markdown

@sugarshin is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@cubic-dev-ai

cubic-dev-ai Bot commented Mar 31, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@sugarshin I have started the AI code review. It will take a few minutes to complete.

@coderabbitai

coderabbitai Bot commented Mar 31, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The VerticalTabsSidebar's sidebar ScrollView is now wrapped in a ScrollViewReader; an onChange handler listens for tabManager.selectedTabId and animates scrollProxy.scrollTo(newId, anchor: nil) with .easeOut(duration: 0.15) when a new tab is selected.

Changes

Cohort / File(s) Summary
Sidebar Auto-Scroll Enhancement
Sources/ContentView.swift
Wrapped sidebar ScrollView in ScrollViewReader, moved related background/overlay modifiers into the ScrollViewReader scope, and added an .onChange(of: tabManager.selectedTabId) that animates scrollProxy.scrollTo(newId, anchor: nil) with .easeOut(duration: 0.15) when selection changes (DEBUG-only logging of selected ID prefix).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐇 I hopped through code with eager paws,

Found tabs that hid behind their claws.
With a nudge and gentle scroll so slight,
The chosen tab now jumps in sight.
Hop—no more searching late at night!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding auto-scroll functionality to the sidebar when workspace selection changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description is comprehensive and complete. It includes all required sections: Summary (clear and detailed), Testing (method and verification steps), Demo Video (direct link provided), Review Trigger (included), and Checklist (partially completed with 3 of 6 items checked).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@coderabbitai

coderabbitai Bot commented Mar 31, 2026

Copy link
Copy Markdown

@sugarshin Sure, I'll review the changes in this PR now!


🧠 Learnings used
Learnt from: Znboston
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-26T19:00:12.434Z
Learning: Repo: manaflow-ai/cmux — Sources/TabManager.swift — In `moveWorkspaceOutOfGroup(_:toSidebarIndex:)`, do NOT call `syncUngroupedSidebarOrder()` after inserting into `sidebarOrder`. That helper derives order from the `tabs` array insertion index, which is unrelated to the user's drag-drop target position. `sidebarOrder` is the source of truth for sidebar rendering; the raw insertion at the resolved sidebar index is correct and sufficient. Calling `syncUngroupedSidebarOrder()` would overwrite the intended drop position with stale `tabs` order, causing a snap-back regression.

Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:38.465Z
Learning: Sources/GhosttyTerminalView.swift — In GhosttySurfaceScrollView.setVisibleInUI(_:), preserve scroll across workspace switches by saving savedScrollRow only when surfaceView.scrollbar.offset > 0 (user scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; otherwise rely on synchronizeScrollView() to keep bottom-pinned sessions following new output.

Learnt from: andrekat
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-29T11:05:01.662Z
Learning: In manaflow-ai/cmux PR `#1773`: `setActiveProfileName(_:)` on `TabManager` (Sources/TabManager.swift) must call `updateWindowTitleForSelectedTab()` explicitly — the save and delete paths do not go through `selectedTabId.didSet`, so without the explicit call the AppKit window title is left stale with the old profile prefix. This was confirmed fixed in commit be9eaa26.

Learnt from: Znboston
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-27T17:03:24.720Z
Learning: Repo: manaflow-ai/cmux — Sources/TabManager.swift — `reorderWorkspace(...)` must reject grouped tabs (early return). Flat-array reorders are not allowed for workspaces inside a group; use the group-aware reorder APIs instead.

Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/Panels/BrowserPanel.swift, BrowserPanel.updateWorkspaceId(_:) is a dedicated method (var workspaceId) that updates both BrowserPanel.workspaceId and pickerMessageHandler?.updateWorkspaceId(_:) atomically. It is called from BrowserPanel.reattachToWorkspace(_:) when a panel moves between workspaces. This ensures BrowserPickerMessageHandler always posts notifications with the current workspaceId, not a stale one from panel initialization.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/ContentView.swift`:
- Around line 8831-8836: Add a debug log inside a DEBUG guard to record the
tab-selection scroll path: in the onChange handler observing
tabManager.selectedTabId (the block that calls scrollProxy.scrollTo(newId,
anchor: nil)), wrap a dlog(...) call in `#if` DEBUG / `#endif` and log the newId
(and any minimal context like "tab selected" or tabManager) before performing
the withAnimation scroll so the event is emitted to the unified debug log;
reference tabManager.selectedTabId, the onChange closure, scrollProxy.scrollTo
and the dlog free function when making the change.
🪄 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

Run ID: 4d2b4d49-d437-4697-8a74-1be2bee99860

📥 Commits

Reviewing files that changed from the base of the PR and between e2e5f87 and 8fe2b6d.

📒 Files selected for processing (1)
  • Sources/ContentView.swift

Comment thread Sources/ContentView.swift Outdated
@greptile-apps

greptile-apps Bot commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds auto-scroll behavior to the sidebar so the selected workspace stays visible when cycling through workspaces with Ctrl+Cmd+[/]. The implementation wraps the existing ScrollView in a ScrollViewReader and attaches an onChange(of: tabManager.selectedTabId) handler that calls scrollProxy.scrollTo(newId, anchor: nil) with a short ease-out animation.

Key points:

  • The approach is architecturally sound: ForEach(tabs, id: \\.id) already exposes each tab's UUID to the scroll proxy, so no explicit .id(tab.id) modifier is required.
  • anchor: nil correctly produces "minimal-scroll" semantics — no movement when the target is already visible.
  • The onChange callback fires only on changes (not initial render), avoiding spurious scrolls at launch.
  • Two minor style issues: the single-argument onChange closure form is deprecated on macOS 14+ (the project minimum), and ScrollView is not indented inside the ScrollViewReader block.

Confidence Score: 5/5

Safe to merge — only P2 style issues remain; the scrolling logic is correct and well-tested.

No P0 or P1 issues found. The ScrollViewReader/onChange approach is correct: ForEach(tabs, id: .id) exposes view IDs to the scroll proxy, anchor: nil provides minimal-scroll semantics, and the animation is straightforward. Remaining findings are a deprecated single-arg onChange closure and an indentation inconsistency — neither affects runtime behavior.

No files require special attention.

Important Files Changed

Filename Overview
Sources/ContentView.swift Wraps the sidebar ScrollView in a ScrollViewReader and adds an onChange handler to auto-scroll to the selected tab ID on workspace changes; logic is correct, two minor style issues (deprecated single-arg onChange, missing indentation).

Sequence Diagram

sequenceDiagram
    participant User
    participant KeyHandler
    participant TabManager
    participant VerticalTabsSidebar
    participant ScrollViewReader
    participant ScrollView

    User->>KeyHandler: Ctrl+Cmd+] / Ctrl+Cmd+[
    KeyHandler->>TabManager: selectNextTab() / selectPreviousTab()
    TabManager->>TabManager: selectedTabId = newId
    TabManager-->>VerticalTabsSidebar: onChange(selectedTabId)
    VerticalTabsSidebar->>ScrollViewReader: scrollProxy.scrollTo(newId, anchor: nil)
    ScrollViewReader->>ScrollView: animate scroll (easeOut 0.15s)
    ScrollView-->>User: Selected tab visible in sidebar
Loading

Reviews (1): Last reviewed commit: "fix: auto-scroll sidebar to follow works..." | Re-trigger Greptile

Comment thread Sources/ContentView.swift Outdated
Comment thread Sources/ContentView.swift
- Use two-argument onChange API (deprecated single-arg form on macOS 14+)
- Add dlog for sidebar scroll event in DEBUG builds

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sugarshin
sugarshin force-pushed the fix/sidebar-scroll branch from 3b82e4f to 86bf756 Compare March 31, 2026 12:20
@sugarshin sugarshin closed this May 25, 2026
@sugarshin
sugarshin deleted the fix/sidebar-scroll branch May 25, 2026 01:11
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