Skip to content

refactor: migrate 5 settings notifications to AppEvents#1118

Merged
datlechin merged 3 commits intomainfrom
refactor/phase5-settings-events
May 8, 2026
Merged

refactor: migrate 5 settings notifications to AppEvents#1118
datlechin merged 3 commits intomainfrom
refactor/phase5-settings-events

Conversation

@datlechin
Copy link
Copy Markdown
Member

Summary

Phase 5 tail. Five settings notifications moved off NotificationCenter onto typed AppEvents publishers (Void payload):

  • editorSettingsChanged
  • dataGridSettingsChanged
  • aiSettingsChanged
  • terminalSettingsChanged
  • accessibilityTextSizeChanged

7 post sites and 7 observer sites migrated. SettingsNotifications.swift and the AppSettingsManager.notifyChange helper deleted.

Changed

  • 1 post: ThemeEngine.swift accessibilityTextSize forwarder
  • 5 posts: AppSettingsManager didSet hooks and accessibility observer
  • 5 observers: TerminalSessionState, DataGridCoordinator, DataGridCellRegistry, SQLEditorCoordinator (editor + ai), SQLEditorView
  • AppEvents.swift gains 5 publishers

NSObjectProtocol observers replaced with AnyCancellable. Manual removeObserver in deinit removed where the observer was the only reason to keep the override.

Test plan

  • Build clean
  • swiftlint clean (verified, 0 violations on touched files)
  • Toggle editor settings (line numbers, vim, font), confirm SQLEditor reacts
  • Toggle data grid row height, confirm grid re-tiles
  • Toggle terminal font, confirm terminal updates
  • Toggle AI provider, confirm inline suggestions source switches
  • System Settings > Accessibility text size change, confirm editor font reloads

Punch list status

Bucket Migrated Remaining
Tier 1 (high traffic) 2 / 8 6
Tier 2 (single observer) 0 / 10 10
Total domain notifs 7 / 35 28

Tier 3 command-style notifications stay on NotificationCenter until the command-bus pass.

@datlechin datlechin merged commit 2239b80 into main May 8, 2026
2 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