Scaffold redesigned Settings: sidebar container behind feature flag#356
Merged
Conversation
0a7dc0d to
fb8c68b
Compare
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
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
First PR in the Settings sidebar-redesign stack. Adds the new container view, sidebar list, pane scaffold, placeholder panes, and a feature flag in
SettingsCoordinator. Default routes to the legacySettingsView; flippingdefaults write com.jacobfu.tabby cotabbySettingsRedesignEnabled -bool YES(or thecotabbySettingsRedesignEnabledkey) routes to the new container. Subsequent PRs replace placeholders with real panes; the final PR in the stack flips the flag default.Validation
xcodegen generate+xcodebuild -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' build CODE_SIGNING_ALLOWED=NO→
** BUILD SUCCEEDED **swiftlint lint --quiet→ no violations.Manual: with the flag flipped, open Settings and confirm the sidebar renders with eight rows under the planned sections, selection persists across reopens, and the title bar updates to "Settings — ". With the flag off, behavior is unchanged.
Linked issues
(none filed)
Risk / rollout notes
CotabbySettingsWindowV2is intentional so the redesigned layout starts from its own default frame rather than inheriting a smaller saved frame from the legacy window..id(selection)is applied preemptively on the detail pane to work around the documented macOS 14 NavigationSplitView selection bug where the first sidebar tap doesn't always trigger a detail re-render.SettingsCategoryenum includes bothengineAndModel(parent overview pane) andappleIntelligence/openSource(sub-rows). The parent pane will own the engine picker when PR 3 lands so the user has a single place to switch engines.xcodegen generatewas rerun and the resultingCotabby.xcodeproj/project.pbxprojis committed.Greptile Summary
This PR scaffolds the redesigned Settings window behind a
UserDefaultsfeature flag inSettingsCoordinator. The legacySettingsViewremains the default; flippingcotabbySettingsRedesignEnabledroutes to the newSettingsContainerView, aNavigationSplitViewshell with a categorized sidebar and placeholder detail panes that subsequent PRs will fill in.isRedesignEnabledbranch that selects the right hosting controller, initial frame, min size, and autosave name (CotabbySettingsWindowV2) per variant, leaving existing window-reuse and delegate teardown logic untouched.NavigationSplitViewwith@AppStorage-persisted selection,syncWindowTitlecalled unconditionally inonAppearand on every selection change, and.id(selection)as the documented macOS 14 split-view workaround.Confidence Score: 5/5
Safe to merge. The redesign is fully gated behind a feature flag that defaults to off, so all existing users continue on the legacy settings path until a future PR flips the default.
All changes are additive and behind a UserDefaults flag that is off by default. The legacy window path is unchanged. The new container view, sidebar, pane scaffold, and placeholder panes are all scaffolding that only activates for developers who opt in via defaults write. No existing behavior is affected.
SettingsContainerView.swift — the @AppStorage/@State two-variable sync pattern is worth simplifying before real panes land, but it has no user-visible impact while all panes are placeholders.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant SettingsCoordinator participant NSWindow participant SettingsContainerView participant SettingsSidebarView participant AppStorage User->>SettingsCoordinator: showSettings() SettingsCoordinator->>SettingsCoordinator: isRedesignEnabled? alt redesign flag ON SettingsCoordinator->>NSWindow: create(frame: 960x680, autosave: V2) SettingsCoordinator->>SettingsContainerView: NSHostingController else redesign flag OFF SettingsCoordinator->>NSWindow: create(frame: 700x620, autosave: legacy) SettingsCoordinator->>NSWindow: SettingsView (legacy) end NSWindow->>User: makeKeyAndOrderFront User->>SettingsContainerView: onAppear AppStorage-->>SettingsContainerView: storedCategoryRawValue SettingsContainerView->>SettingsContainerView: "selection = stored ?? .general" SettingsContainerView->>NSWindow: syncWindowTitle(selection) User->>SettingsSidebarView: tap row SettingsSidebarView->>SettingsContainerView: $selection binding updated SettingsContainerView->>AppStorage: persist rawValue SettingsContainerView->>NSWindow: syncWindowTitle(newValue) SettingsContainerView->>SettingsContainerView: detailPane re-renders (.id workaround)Reviews (2): Last reviewed commit: "Address Greptile feedback: initial title..." | Re-trigger Greptile