Skip to content

Pin Settings sidebar visibility so the toolbar toggle stops teleporting#369

Merged
FuJacob merged 1 commit into
mainfrom
fix/settings-sidebar-pin-visibility
May 28, 2026
Merged

Pin Settings sidebar visibility so the toolbar toggle stops teleporting#369
FuJacob merged 1 commit into
mainfrom
fix/settings-sidebar-pin-visibility

Conversation

@FuJacob
Copy link
Copy Markdown
Owner

@FuJacob FuJacob commented May 28, 2026

The default NavigationSplitView toolbar shows a collapse-sidebar toggle that hops between the sidebar header and the detail header as the column visibility changes. In a Settings window the user has no reason to collapse the sidebar, so the safer fix is to pin visibility to .all and explicitly remove the toggle from both toolbars.

  • BUILD SUCCEEDED, swiftlint clean.

Greptile Summary

Pins the NavigationSplitView sidebar to always-visible in SettingsContainerView by holding columnVisibility at .all, removing the sidebar-toggle toolbar item from both columns, and adding an onChange guard that snaps visibility back if anything tries to collapse it.

  • Removes the floating toolbar toggle that previously teleported between the sidebar and detail headers as column visibility changed — a clean, targeted fix for the reported UX regression.
  • The onChange snap-back provides a defensive fallback, but the system View › Hide Sidebar menu command and its keyboard shortcut (⌘⌃S) are not suppressed, so users can still trigger a brief collapse animation before the snap-back fires.

Confidence Score: 4/5

Safe to merge; the change is narrowly scoped to the Settings window and has no effect on data or navigation correctness.

The toolbar-removal and visibility-pinning approach is correct and well-commented. The one remaining gap is cosmetic: the macOS sidebar keyboard shortcut can still trigger a visible collapse-and-snap animation since the shortcut is not intercepted. This doesn't affect functionality, only polish.

No files require special attention; the single changed file is self-contained and the logic is straightforward.

Important Files Changed

Filename Overview
Cotabby/UI/Settings/SettingsContainerView.swift Adds columnVisibility state pinned to .all, removes sidebarToggle from both toolbar slots, and adds an onChange guard to snap back if anything tries to collapse the sidebar. Well-scoped fix; one minor concern around keyboard shortcut collapse still triggering a visible animation before the snap-back fires.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SettingsContainerView appears] --> B[columnVisibility = .all]
    B --> C[NavigationSplitView renders sidebar + detail]
    C --> D{Sidebar toggle toolbar button exists?}
    D -- Before PR --> E[Toggle shown in sidebar or detail header - jumps around]
    D -- After PR --> F[.toolbar removing .sidebarToggle applied to both columns]
    F --> G[Toggle removed from title bar]
    G --> H{Anything tries to collapse sidebar?}
    H -- No --> I[Sidebar stays visible ✓]
    H -- Yes e.g. ⌘⌃S keyboard shortcut --> J[columnVisibility changes]
    J --> K[onChange fires]
    K --> L[Snap back to .all]
    L --> I
Loading

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "Pin Settings sidebar visibility so the t..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@FuJacob FuJacob merged commit 6a95272 into main May 28, 2026
4 checks passed
@FuJacob FuJacob deleted the fix/settings-sidebar-pin-visibility branch May 28, 2026 10:44
Comment on lines +55 to +61
.onChange(of: columnVisibility) { _, newValue in
// Snap back to `.all` if something tries to collapse the sidebar. Cheaper than wiring
// a custom binding and reads as the same intent: the sidebar is never optional here.
if newValue != .all {
columnVisibility = .all
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 onChange snap-back doesn't suppress the sidebar-collapse animation

.toolbar(removing: .sidebarToggle) removes the UI button, but the system View › Hide Sidebar menu item and its default keyboard shortcut (⌘⌃S) remain active. When a user presses that shortcut, NavigationSplitView starts its collapse animation before onChange fires and resets columnVisibility to .all, producing a visible flash/flicker. The toggle button fix is solid; consider also disabling the sidebar menu command — for example by overriding NSMenuItem for toggleSidebar: in the hosting window, or by intercepting it with a .commands modifier — to make the experience fully seamless.

Fix in Codex Fix in Claude Code

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