refactor: migrate menu commands from AppState.shared to @FocusedValue#610
Merged
refactor: migrate menu commands from AppState.shared to @FocusedValue#610
Conversation
Phase 1+2 of AppState migration: - Add 13 computed properties to MainContentCommandActions (per-window state) - Replace all appState.xxx reads in AppMenuCommands and PasteboardCommands with actions?.xxx reads from @focusedvalue - Remove appState parameter from AppMenuCommands and PasteboardCommands - Menu items now correctly reflect the active window's state in multi-window AppState.shared is kept as a thin bridge for non-menu consumers (SidebarView, MainContentView) that will be migrated separately.
- SidebarView: safeModeLevel read from coordinator instead of AppState.shared - SidebarContextMenu: currentDatabaseType read from coordinator.connection
…ToolbarState Migrated sidebar reads from AppState.shared to per-window coordinator. Added two properties to ConnectionToolbarState for future write migration. AppState.shared writes remain temporarily — removing them requires careful per-file editing to avoid breaking multi-line expressions.
- TableProApp: replace all 38 appState.xxx reads with actions?.xxx - Remove appState parameter from AppMenuCommands and PasteboardCommands - MainContentCommandActions: hasStructureChanges reads from toolbarState - MainContentCommandActions: toggleHistoryPanel writes to toolbarState - MainEditorContentView: isHistoryPanelVisible reads from toolbarState - MainContentView: hasStructureChanges reads from toolbarState
… remove dead writes
…nges - MainContentView: hasStructureChanges reads from toolbarState - MainEditorContentView: hasQueryText writes to toolbarState - QueryEditorView: hasQueryText derived from queryText binding directly - ConnectionToolbarState: add hasQueryText property - Zero appState.xxx reads remain in the codebase
- Remove 4 unused @Environment(AppState.self) declarations - Remove .environment(AppState.shared) injection from WindowGroup - Add hasQueryText to ConnectionToolbarState - QueryEditorView derives hasQueryText from queryText binding directly - Zero appState reads remain; 53 dead writes remain (harmless, for cleanup)
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
1 participant
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
Migrates menu command state from the global
AppState.sharedsingleton to per-window@FocusedValue(\.commandActions). This fixes multi-window state corruption where menu items reflected the last-active window instead of the currently-focused one.What changed:
MainContentCommandActions— 13 computed properties added deriving state from the per-window coordinator, connection, and bindingsAppMenuCommands— AllappState.xxxreads replaced withactions?.xxx ?? defaultPasteboardCommands— Same migrationWhat stays:
AppState.sharedclass and singleton preserved as a thin bridge for 6 remaining non-menu consumers (SidebarView, MainContentView).environment(AppState.shared)still injected into WindowGroupMulti-window fix: When two windows are open, switching focus now correctly updates all menu enabled/disabled states for the active window. Previously, menus reflected whichever window last wrote to
AppState.shared.Test plan