refactor(editor): enhance popup management and initialization order

- Updated EditorManager to initialize PopupManager in the constructor, ensuring safe access to popups during menu operations. - Refactored MenuOrchestrator to utilize PopupID constants for popup management, improving clarity and maintainability. - Enhanced PopupManager to register popups with structured definitions, allowing for better organization and future expansion. Benefits: - Streamlines the initialization process, preventing potential crashes due to uninitialized components. - Improves the organization of popup management, leading to a more intuitive user experience and easier maintenance.
2025-10-15 13:29:21 -04:00
parent a45e16e04a
commit 651be0fdca
6 changed files with 267 additions and 78 deletions
--- a/src/app/editor/editor_manager.cc
+++ b/src/app/editor/editor_manager.cc
@@ -144,28 +144,46 @@ EditorManager::EditorManager()
  // - EditorCardRegistry: Card-based editor UI management
  // - ShortcutConfigurator: Keyboard shortcut registration
  // - WindowDelegate: Window layout operations
+  // - PopupManager: Modal popup/dialog management
  //
  // EditorManager retains:
  // - Session storage (sessions_) and current pointers (current_rom_, etc.)
  // - Main update loop (iterates sessions, calls editor updates)
  // - Asset loading (Initialize/Load on all editors)
  // - Session ID tracking (current_session_id_)
+  //
+  // INITIALIZATION ORDER (CRITICAL):
+  // 1. PopupManager - MUST be first, MenuOrchestrator/UICoordinator take ref to it
+  // 2. SessionCoordinator - Independent, can be early
+  // 3. MenuOrchestrator - Depends on PopupManager, SessionCoordinator
+  // 4. UICoordinator - Depends on PopupManager, SessionCoordinator
+  // 5. ShortcutConfigurator - Created in Initialize(), depends on all above
+  //
+  // If this order is violated, you will get SIGSEGV crashes when menu callbacks
+  // try to call popup_manager_.Show() with an uninitialized PopupManager!
  // ============================================================================
  
-  // SessionCoordinator: Manages session lifecycle and session-specific UI
+  // STEP 1: Initialize PopupManager FIRST
+  popup_manager_ = std::make_unique<PopupManager>(this);
+  popup_manager_->Initialize();  // Registers all popups with PopupID constants
+  
+  // STEP 2: Initialize SessionCoordinator (independent of popups)
  session_coordinator_ = std::make_unique<SessionCoordinator>(
      static_cast<void*>(&sessions_), &card_registry_, &toast_manager_);
  
-  // MenuOrchestrator: Builds all menus and routes actions to appropriate managers
+  // STEP 3: Initialize MenuOrchestrator (depends on popup_manager_, session_coordinator_)
  menu_orchestrator_ = std::make_unique<MenuOrchestrator>(
      this, menu_builder_, rom_file_manager_, project_manager_,
      editor_registry_, *session_coordinator_, toast_manager_, *popup_manager_);
  
-  // UICoordinator: Coordinates all UI drawing (dialogs, popups, welcome screen)
+  // STEP 4: Initialize UICoordinator (depends on popup_manager_, session_coordinator_)
  ui_coordinator_ = std::make_unique<UICoordinator>(
      this, rom_file_manager_, project_manager_, editor_registry_,
      *session_coordinator_, window_delegate_, toast_manager_, *popup_manager_,
      shortcut_manager_);
+  
+  // STEP 5: ShortcutConfigurator created later in Initialize() method
+  // It depends on all above coordinators being available
 }

 EditorManager::~EditorManager() = default;
@@ -227,9 +245,8 @@ void EditorManager::Initialize(gfx::IRenderer* renderer,
    PRINT_IF_ERROR(OpenRomOrProject(filename));
  }

-  // Initialize popup manager
-  popup_manager_ = std::make_unique<PopupManager>(this);
-  popup_manager_->Initialize();
+  // Note: PopupManager is now initialized in constructor before MenuOrchestrator
+  // This ensures all menu callbacks can safely call popup_manager_.Show()

  // Register emulator cards early (emulator Initialize might not be called)
  // Using EditorCardRegistry directly
--- a/src/app/editor/system/menu_orchestrator.cc
+++ b/src/app/editor/system/menu_orchestrator.cc
@@ -199,18 +199,18 @@ void MenuOrchestrator::AddViewMenuItems() {
            [this]() { OnShowEmulator(); }, "Ctrl+Shift+E")
      .Separator();
  
+  // Settings and UI
+  menu_builder_
+      .Item("Display Settings", ICON_MD_DISPLAY_SETTINGS,
+            [this]() { OnShowDisplaySettings(); })
+      .Separator();
+  
  // Additional UI Elements
  menu_builder_
      .Item("Card Browser", ICON_MD_DASHBOARD,
            [this]() { OnShowCardBrowser(); }, "Ctrl+Shift+B")
      .Item("Welcome Screen", ICON_MD_HOME,
-            [this]() { OnShowWelcomeScreen(); })
-      .Separator();
-  
-  // Display Settings
-  menu_builder_
-      .Item("Display Settings", ICON_MD_DISPLAY_SETTINGS,
-            [this]() { OnShowDisplaySettings(); });
+            [this]() { OnShowWelcomeScreen(); });
 }

 void MenuOrchestrator::BuildToolsMenu() {
@@ -401,8 +401,7 @@ void MenuOrchestrator::OnSaveRom() {
 }

 void MenuOrchestrator::OnSaveRomAs() {
-  // TODO: Show save dialog and delegate to rom_manager_
-  toast_manager_.Show("Save ROM As", ToastType::kInfo);
+  popup_manager_.Show(PopupID::kSaveAs);
 }

 void MenuOrchestrator::OnCreateProject() {
@@ -552,12 +551,7 @@ void MenuOrchestrator::OnShowEditorSelection() {
 }

 void MenuOrchestrator::OnShowDisplaySettings() {
-  // Delegate to UICoordinator for display settings popup
-  if (editor_manager_) {
-    if (auto* ui = editor_manager_->ui_coordinator()) {
-      ui->ShowDisplaySettings();
-    }
-  }
+  popup_manager_.Show(PopupID::kDisplaySettings);
 }

 void MenuOrchestrator::OnShowHexEditor() {
@@ -785,44 +779,44 @@ void MenuOrchestrator::OnShowNetworkStatus() {

 // Help menu actions
 void MenuOrchestrator::OnShowAbout() {
-  popup_manager_.Show("About");
+  popup_manager_.Show(PopupID::kAbout);
 }

 void MenuOrchestrator::OnShowGettingStarted() {
-  popup_manager_.Show("Getting Started");
+  popup_manager_.Show(PopupID::kGettingStarted);
 }

 void MenuOrchestrator::OnShowAsarIntegration() {
-  popup_manager_.Show("Asar Integration");
+  popup_manager_.Show(PopupID::kAsarIntegration);
 }

 void MenuOrchestrator::OnShowBuildInstructions() {
-  popup_manager_.Show("Build Instructions");
+  popup_manager_.Show(PopupID::kBuildInstructions);
 }

 void MenuOrchestrator::OnShowCLIUsage() {
-  popup_manager_.Show("CLI Usage");
+  popup_manager_.Show(PopupID::kCLIUsage);
 }

 void MenuOrchestrator::OnShowTroubleshooting() {
-  popup_manager_.Show("Troubleshooting");
+  popup_manager_.Show(PopupID::kTroubleshooting);
 }

 void MenuOrchestrator::OnShowContributing() {
-  popup_manager_.Show("Contributing");
+  popup_manager_.Show(PopupID::kContributing);
 }

 void MenuOrchestrator::OnShowWhatsNew() {
-  popup_manager_.Show("Whats New v03");
+  popup_manager_.Show(PopupID::kWhatsNew);
 }

 void MenuOrchestrator::OnShowSupportedFeatures() {
-  popup_manager_.Show("Supported Features");
+  popup_manager_.Show(PopupID::kSupportedFeatures);
 }

 // Additional File menu actions
 void MenuOrchestrator::OnShowRomInfo() {
-  popup_manager_.Show("ROM Information");
+  popup_manager_.Show(PopupID::kRomInfo);
 }

 void MenuOrchestrator::OnCreateBackup() {
--- a/src/app/editor/system/menu_orchestrator.h
+++ b/src/app/editor/system/menu_orchestrator.h
@@ -86,7 +86,7 @@ class MenuOrchestrator {
  // Editor-specific menu actions
  void OnSwitchToEditor(EditorType editor_type);
  void OnShowEditorSelection();
-  void OnShowDisplaySettings();
+  void OnShowDisplaySettings();  // Display settings popup
  void OnShowHexEditor();
  void OnShowEmulator();
  void OnShowCardBrowser();
--- a/src/app/editor/system/popup_manager.cc
+++ b/src/app/editor/system/popup_manager.cc
@@ -16,31 +16,104 @@ PopupManager::PopupManager(EditorManager* editor_manager)
    : editor_manager_(editor_manager), status_(absl::OkStatus()) {}

 void PopupManager::Initialize() {
-  // Register all popups
-  popups_["About"] = {"About", false, [this]() { DrawAboutPopup(); }};
-  popups_["ROM Information"] = {"ROM Information", false, [this]() { DrawRomInfoPopup(); }};
-  popups_["Save As.."] = {"Save As..", false, [this]() { DrawSaveAsPopup(); }};
-  popups_["New Project"] = {"New Project", false, [this]() { DrawNewProjectPopup(); }};
-  popups_["Supported Features"] = {"Supported Features", false, [this]() { DrawSupportedFeaturesPopup(); }};
-  popups_["Open a ROM"] = {"Open a ROM", false, [this]() { DrawOpenRomHelpPopup(); }};
-  popups_["Manage Project"] = {"Manage Project", false, [this]() { DrawManageProjectPopup(); }};
+  // ============================================================================
+  // POPUP REGISTRATION
+  // ============================================================================
+  // All popups must be registered here BEFORE any menu callbacks can trigger them.
+  // This method is called in EditorManager constructor BEFORE MenuOrchestrator
+  // and UICoordinator are created, ensuring safe initialization order.
+  //
+  // Popup Registration Format:
+  // popups_[PopupID::kConstant] = {
+  //   .name = PopupID::kConstant,
+  //   .type = PopupType::kXxx,
+  //   .is_visible = false,
+  //   .allow_resize = false/true,
+  //   .draw_function = [this]() { DrawXxxPopup(); }
+  // };
+  // ============================================================================
  
-  // v0.3 Help Documentation popups
-  popups_["Getting Started"] = {"Getting Started", false, [this]() { DrawGettingStartedPopup(); }};
-  popups_["Asar Integration"] = {"Asar Integration", false, [this]() { DrawAsarIntegrationPopup(); }};
-  popups_["Build Instructions"] = {"Build Instructions", false, [this]() { DrawBuildInstructionsPopup(); }};
-  popups_["CLI Usage"] = {"CLI Usage", false, [this]() { DrawCLIUsagePopup(); }};
-  popups_["Troubleshooting"] = {"Troubleshooting", false, [this]() { DrawTroubleshootingPopup(); }};
-  popups_["Contributing"] = {"Contributing", false, [this]() { DrawContributingPopup(); }};
-  popups_["Whats New v03"] = {"What's New in v0.3", false, [this]() { DrawWhatsNewPopup(); }};
+  // File Operations
+  popups_[PopupID::kSaveAs] = {
+    PopupID::kSaveAs, PopupType::kFileOperation, false, false,
+    [this]() { DrawSaveAsPopup(); }
+  };
+  popups_[PopupID::kNewProject] = {
+    PopupID::kNewProject, PopupType::kFileOperation, false, false,
+    [this]() { DrawNewProjectPopup(); }
+  };
+  popups_[PopupID::kManageProject] = {
+    PopupID::kManageProject, PopupType::kFileOperation, false, false,
+    [this]() { DrawManageProjectPopup(); }
+  };
  
-  // Workspace-related popups
-  popups_["Workspace Help"] = {"Workspace Help", false, [this]() { DrawWorkspaceHelpPopup(); }};
-  popups_["Session Limit Warning"] = {"Session Limit Warning", false, [this]() { DrawSessionLimitWarningPopup(); }};
-  popups_["Layout Reset Confirm"] = {"Reset Layout Confirmation", false, [this]() { DrawLayoutResetConfirmPopup(); }};
+  // Information
+  popups_[PopupID::kAbout] = {
+    PopupID::kAbout, PopupType::kInfo, false, false,
+    [this]() { DrawAboutPopup(); }
+  };
+  popups_[PopupID::kRomInfo] = {
+    PopupID::kRomInfo, PopupType::kInfo, false, false,
+    [this]() { DrawRomInfoPopup(); }
+  };
+  popups_[PopupID::kSupportedFeatures] = {
+    PopupID::kSupportedFeatures, PopupType::kInfo, false, false,
+    [this]() { DrawSupportedFeaturesPopup(); }
+  };
+  popups_[PopupID::kOpenRomHelp] = {
+    PopupID::kOpenRomHelp, PopupType::kHelp, false, false,
+    [this]() { DrawOpenRomHelpPopup(); }
+  };
  
-  // Settings popups (accessible without ROM)
-  popups_["Display Settings"] = {"Display Settings", false, [this]() { DrawDisplaySettingsPopup(); }};
+  // Help Documentation
+  popups_[PopupID::kGettingStarted] = {
+    PopupID::kGettingStarted, PopupType::kHelp, false, false,
+    [this]() { DrawGettingStartedPopup(); }
+  };
+  popups_[PopupID::kAsarIntegration] = {
+    PopupID::kAsarIntegration, PopupType::kHelp, false, false,
+    [this]() { DrawAsarIntegrationPopup(); }
+  };
+  popups_[PopupID::kBuildInstructions] = {
+    PopupID::kBuildInstructions, PopupType::kHelp, false, false,
+    [this]() { DrawBuildInstructionsPopup(); }
+  };
+  popups_[PopupID::kCLIUsage] = {
+    PopupID::kCLIUsage, PopupType::kHelp, false, false,
+    [this]() { DrawCLIUsagePopup(); }
+  };
+  popups_[PopupID::kTroubleshooting] = {
+    PopupID::kTroubleshooting, PopupType::kHelp, false, false,
+    [this]() { DrawTroubleshootingPopup(); }
+  };
+  popups_[PopupID::kContributing] = {
+    PopupID::kContributing, PopupType::kHelp, false, false,
+    [this]() { DrawContributingPopup(); }
+  };
+  popups_[PopupID::kWhatsNew] = {
+    PopupID::kWhatsNew, PopupType::kHelp, false, false,
+    [this]() { DrawWhatsNewPopup(); }
+  };
+  
+  // Settings
+  popups_[PopupID::kDisplaySettings] = {
+    PopupID::kDisplaySettings, PopupType::kSettings, false, true,  // Resizable
+    [this]() { DrawDisplaySettingsPopup(); }
+  };
+  
+  // Workspace
+  popups_[PopupID::kWorkspaceHelp] = {
+    PopupID::kWorkspaceHelp, PopupType::kHelp, false, false,
+    [this]() { DrawWorkspaceHelpPopup(); }
+  };
+  popups_[PopupID::kSessionLimitWarning] = {
+    PopupID::kSessionLimitWarning, PopupType::kWarning, false, false,
+    [this]() { DrawSessionLimitWarningPopup(); }
+  };
+  popups_[PopupID::kLayoutResetConfirm] = {
+    PopupID::kLayoutResetConfirm, PopupType::kConfirmation, false, false,
+    [this]() { DrawLayoutResetConfirmPopup(); }
+  };
 }

 void PopupManager::DrawPopups() {
@@ -52,11 +125,9 @@ void PopupManager::DrawPopups() {
    if (params.is_visible) {
      OpenPopup(name.c_str());
      
-      // Special handling for Display Settings popup to make it resizable
-      ImGuiWindowFlags popup_flags = ImGuiWindowFlags_AlwaysAutoResize;
-      if (name == "Display Settings") {
-        popup_flags = ImGuiWindowFlags_None; // Allow resizing for display settings
-      }
+      // Use allow_resize flag from popup definition
+      ImGuiWindowFlags popup_flags = params.allow_resize ? 
+          ImGuiWindowFlags_None : ImGuiWindowFlags_AlwaysAutoResize;
      
      if (BeginPopupModal(name.c_str(), nullptr, popup_flags)) {
        params.draw_function();
@@ -67,14 +138,31 @@ void PopupManager::DrawPopups() {
 }

 void PopupManager::Show(const char* name) {
-  auto it = popups_.find(name);
+  if (!name) {
+    return;  // Safety check for null pointer
+  }
+  
+  std::string name_str(name);
+  auto it = popups_.find(name_str);
  if (it != popups_.end()) {
    it->second.is_visible = true;
+  } else {
+    // Log warning for unregistered popup
+    printf("[PopupManager] Warning: Popup '%s' not registered. Available popups: ", name);
+    for (const auto& [key, _] : popups_) {
+      printf("'%s' ", key.c_str());
+    }
+    printf("\n");
  }
 }

 void PopupManager::Hide(const char* name) {
-  auto it = popups_.find(name);
+  if (!name) {
+    return;  // Safety check for null pointer
+  }
+  
+  std::string name_str(name);
+  auto it = popups_.find(name_str);
  if (it != popups_.end()) {
    it->second.is_visible = false;
    CloseCurrentPopup();
@@ -82,7 +170,12 @@ void PopupManager::Hide(const char* name) {
 }

 bool PopupManager::IsVisible(const char* name) const {
-  auto it = popups_.find(name);
+  if (!name) {
+    return false;  // Safety check for null pointer
+  }
+  
+  std::string name_str(name);
+  auto it = popups_.find(name_str);
  if (it != popups_.end()) {
    return it->second.is_visible;
  }
@@ -189,7 +282,7 @@ void PopupManager::DrawSaveAsPopup() {
      auto status = editor_manager_->SaveRomAs(final_filename);
      if (status.ok()) {
        save_as_filename = "";
-        Hide("Save As..");
+        Hide(PopupID::kSaveAs);
      }
    }
  }
@@ -198,7 +291,7 @@ void PopupManager::DrawSaveAsPopup() {
  if (Button(absl::StrFormat("%s Cancel", ICON_MD_CANCEL).c_str(),
             gui::kDefaultModalSize)) {
    save_as_filename = "";
-    Hide("Save As..");
+    Hide(PopupID::kSaveAs);
  }
 }

@@ -265,7 +358,7 @@ void PopupManager::DrawNewProjectPopup() {
        rom_filename = "";
        labels_filename = "";
        code_folder = "";
-        Hide("New Project");
+        Hide(PopupID::kNewProject);
      }
    }
  }
@@ -278,7 +371,7 @@ void PopupManager::DrawNewProjectPopup() {
    rom_filename = "";
    labels_filename = "";
    code_folder = "";
-    Hide("New Project");
+    Hide(PopupID::kNewProject);
  }
 }

--- a/src/app/editor/system/popup_manager.h
+++ b/src/app/editor/system/popup_manager.h
@@ -13,12 +13,84 @@ namespace editor {
 // Forward declaration
 class EditorManager;

+/**
+ * @enum PopupType
+ * @brief Type classification for popups to enable future filtering and organization
+ */
+enum class PopupType {
+  kInfo,           // Information display (About, ROM Info, etc.)
+  kHelp,           // Help documentation (Getting Started, etc.)
+  kSettings,       // Settings dialogs (Display Settings, etc.)
+  kFileOperation,  // File operations (Save As, New Project, etc.)
+  kConfirmation,   // Confirmation dialogs (Layout Reset, etc.)
+  kWarning,        // Warning messages (Session Limit, etc.)
+  kEditor          // Editor-specific dialogs
+};
+
+/**
+ * @struct PopupDefinition
+ * @brief Complete definition of a popup including metadata
+ */
+struct PopupDefinition {
+  const char* id;                    // Unique constant identifier
+  const char* display_name;          // Human-readable name for UI
+  PopupType type;                    // Type classification
+  bool allow_resize;                 // Whether popup can be resized
+  std::function<void()> draw_function;  // Drawing callback (set at runtime)
+};
+
+/**
+ * @struct PopupParams
+ * @brief Runtime state for a registered popup
+ */
 struct PopupParams {
  std::string name;
+  PopupType type;
  bool is_visible = false;
+  bool allow_resize = false;
  std::function<void()> draw_function;
 };

+/**
+ * @namespace PopupID
+ * @brief String constants for all popup identifiers to prevent typos
+ */
+namespace PopupID {
+  // File Operations
+  constexpr const char* kSaveAs = "Save As..";
+  constexpr const char* kNewProject = "New Project";
+  constexpr const char* kManageProject = "Manage Project";
+  
+  // Information
+  constexpr const char* kAbout = "About";
+  constexpr const char* kRomInfo = "ROM Information";
+  constexpr const char* kSupportedFeatures = "Supported Features";
+  constexpr const char* kStatus = "Status";
+  
+  // Help Documentation
+  constexpr const char* kGettingStarted = "Getting Started";
+  constexpr const char* kAsarIntegration = "Asar Integration";
+  constexpr const char* kBuildInstructions = "Build Instructions";
+  constexpr const char* kCLIUsage = "CLI Usage";
+  constexpr const char* kTroubleshooting = "Troubleshooting";
+  constexpr const char* kContributing = "Contributing";
+  constexpr const char* kWhatsNew = "Whats New v03";
+  constexpr const char* kOpenRomHelp = "Open a ROM";
+  
+  // Settings
+  constexpr const char* kDisplaySettings = "Display Settings";
+  
+  // Workspace
+  constexpr const char* kWorkspaceHelp = "Workspace Help";
+  constexpr const char* kSessionLimitWarning = "Session Limit Warning";
+  constexpr const char* kLayoutResetConfirm = "Reset Layout Confirmation";
+  
+  // Future expansion
+  constexpr const char* kQuickExport = "Quick Export";
+  constexpr const char* kAssetImport = "Asset Import";
+  constexpr const char* kScriptGenerator = "Script Generator";
+}
+
 // ImGui popup manager.
 class PopupManager {
 public:
--- a/src/app/editor/ui/ui_coordinator.cc
+++ b/src/app/editor/ui/ui_coordinator.cc
@@ -104,6 +104,7 @@ void UICoordinator::DrawAllUI() {
  // Draw all UI components in order
  DrawMenuBarExtras();
  DrawContextSensitiveCardControl();
+  DrawCommandPalette();          // NEW: Moved from EditorManager
  DrawSessionSwitcher();
  DrawSessionManager();
  DrawSessionRenameDialog();
@@ -119,15 +120,15 @@ void UICoordinator::DrawAllUI() {
 void UICoordinator::DrawMenuBarExtras() {
  // Get current ROM from EditorManager (RomFileManager doesn't track "current")
  auto* current_rom = editor_manager_->GetCurrentRom();
+  
+  // Calculate version width for right alignment
  std::string version_text = absl::StrFormat("v%s", editor_manager_->version().c_str());
  float version_width = ImGui::CalcTextSize(version_text.c_str()).x;
-  float session_rom_area_width = 280.0f;
-
-  ImGui::SameLine(ImGui::GetWindowWidth() - version_width - 10 - session_rom_area_width);

  // Session indicator with Material Design styling
  if (session_coordinator_.HasMultipleSessions()) {
-    std::string session_button_text = absl::StrFormat("%s%zu", ICON_MD_TAB, 
+    ImGui::SameLine();
+    std::string session_button_text = absl::StrFormat("%s %zu", ICON_MD_TAB, 
                                                     session_coordinator_.GetActiveSessionCount());
    
    // Material Design button styling
@@ -142,21 +143,32 @@ void UICoordinator::DrawMenuBarExtras() {
    ImGui::PopStyleColor(3);
    
    if (ImGui::IsItemHovered()) {
-      ImGui::SetTooltip("Sessions: %zu active\nClick to switch",
-                        session_coordinator_.GetActiveSessionCount());
+      ImGui::SetTooltip("Switch Sessions (Ctrl+Tab)");
    }
-    ImGui::SameLine();
  }

  // ROM information display with Material Design card styling
+  ImGui::SameLine();
  if (current_rom && current_rom->is_loaded()) {
    ImGui::PushStyleColor(ImGuiCol_Text, gui::GetTextSecondaryVec4());
-    ImGui::Text("%s %s", ICON_MD_INSERT_DRIVE_FILE, current_rom->title().c_str());
+    std::string rom_title = current_rom->title();
+    if (current_rom->dirty()) {
+      ImGui::Text("%s %s*", ICON_MD_CIRCLE, rom_title.c_str());
+      if (ImGui::IsItemHovered()) {
+        ImGui::SetTooltip("Unsaved changes");
+      }
+    } else {
+      ImGui::Text("%s %s", ICON_MD_INSERT_DRIVE_FILE, rom_title.c_str());
+    }
+    ImGui::PopStyleColor();
+  } else {
+    ImGui::PushStyleColor(ImGuiCol_Text, gui::GetTextDisabledVec4());
+    ImGui::Text("%s No ROM", ICON_MD_WARNING);
    ImGui::PopStyleColor();
-    ImGui::SameLine();
  }

-  // Version info with subtle styling
+  // Version info aligned to far right
+  ImGui::SameLine(ImGui::GetWindowWidth() - version_width - 15.0f);
  ImGui::PushStyleColor(ImGuiCol_Text, gui::GetTextDisabledVec4());
  ImGui::Text("%s", version_text.c_str());
  ImGui::PopStyleColor();
@@ -319,8 +331,9 @@ void UICoordinator::HidePopup(const std::string& popup_name) {
 }

 void UICoordinator::ShowDisplaySettings() {
-  show_display_settings_ = true;
-  ShowPopup("Display Settings");
+  // Display Settings is now a popup managed by PopupManager
+  // Delegate directly to PopupManager instead of UICoordinator
+  popup_manager_.Show(PopupID::kDisplaySettings);
 }

 void UICoordinator::HideCurrentEditorCards() {